Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(NODE-5648): add Long.fromStringStrict() #675
feat(NODE-5648): add Long.fromStringStrict() #675
Changes from 18 commits
c0f5350
272e4f1
4483ac4
a87dade
76801e8
a001590
c4818d9
59f3911
4c7d5b5
8939fe9
667168e
88fb767
e616e88
70c6162
1d261c5
60b2272
9f308aa
63beeea
d212f7e
c98b513
152db8f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we checking the radix here? This changes the behavior of Long.fromString - something we want to avoid in this PR. The logic was previously:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does change the behavior of
Long.fromString
, but its another bug fix, (see updated release notes).We previously did not allow valid inputs to radices, because we interpreted them as 'not a number,' but in
radix >= 24
, 'nan' is a valid number (since 'n' and 'a' are in the character set for numbers).Example:
Long.fromString('NaN', 27); // new Long(17060)
The original
Long.fromString
would returnLong.ZERO
on '+/- Infinity' and 'NaN' cases, and then check for radix later. This behavior is still preserved, other than in the case that the user is not actually inputting 'not a number' or an infinite value, but a valid non-infinite Long written in a high radix.