Skip to content

tests: Add missing test in String #1686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 17, 2021
Merged

tests: Add missing test in String #1686

merged 1 commit into from
Feb 17, 2021

Conversation

saulecabrera
Copy link
Contributor

@saulecabrera saulecabrera commented Feb 15, 2021

Adds missing tests for the following:

  • codePointAt
  • concat
  • fromCharCodes

@dcodeIO I also found out that the documentation lists String#fromCodePoints as a static member, but there's no actual definition for it. Should this be defined or removed from the docs instead? If it's a documentation thing, I can submit a PR to fix it.

  • I've read the contributing guidelines

@SebastianSpeitel
Copy link

If we are already at fromCharCodes, maybe we could look further into the signature to allow arrays of different element sizes.
I already prepared 2 options:
SebastianSpeitel@fcf2fd9
SebastianSpeitel@08a4f4d

@saulecabrera
Copy link
Contributor Author

If we are already at fromCharCodes, maybe we could look further into the signature to allow arrays of different element sizes.
I already prepared 2 options:
SebastianSpeitel@fcf2fd9
SebastianSpeitel@08a4f4d

I'm probably missing something here, but could you elaborate on what would be the advantage of doing this? In SebastianSpeitel@08a4f4d you're still checking if the value of the units is an integer, so is it just for the semantics of the function?

@SebastianSpeitel
Copy link

It allows you to use all the different integer types not just i32, where the first 2 bytes are discarded anyways.

@dcodeIO dcodeIO requested a review from MaxGraey February 17, 2021 13:55
Add tests for the following:

- `codePointAt`
- `concat`
- `fromCharCodes`
@dcodeIO dcodeIO merged commit 369c6fc into AssemblyScript:master Feb 17, 2021
@dcodeIO
Copy link
Member

dcodeIO commented Feb 17, 2021

Thank you! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants