-
Notifications
You must be signed in to change notification settings - Fork 3
Support Infinity, NaN, and -0 #57
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
Conversation
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.
See line comments below for some details, but in general, this seems to be introducing a whole bunch of new logic that probably should not be new. Instead, we ought to copy or move text from ECMA-402 and the keep-trailing-zeros proposal to ECMA-262. That proposal is already effecting the same sort of parsing that's needed here by modifying ToIntlMathematicalValue.
We don't really want to define a new number parser here, as that runs a high risk of introducing a difference with the exisitng JS number parsers, so we ought to rely on pre-existing definitions as far as possible.
Co-authored-by: Eemeli Aro <[email protected]>
Co-authored-by: Eemeli Aro <[email protected]>
I've taken a look -- indeed, we can reuse some 402 stuff, in particular the string parsing work already done in the StringIntlMV syntax-directed operation. I've used that now; the diff is now smaller. Can you take another look? I guess we should be able to refer to keep-trailing-zero's preservation of precision, too. To make things clean, I'd like to refer to a keep-trailing-zero biblio. Can you generate one? |
Also, copy the definition of the `StringIntlMV` from 402.
Also, remove `CountSignificantDigits` AO, which is no longer needed.
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.
This looks good to merge; we'll still need to iterate on it, but that doesn't need to all happen in this PR.
I still think that the structure I proposed in #57 (comment) would be clearer, but that's just an editorial concern.
This PR extends the Amount constructor to allow Infinity, -Infinity, NaN, and -0, both as Numbers and as Strings. We stipulate that when these kinds of values are given, the precision is undefined. We also render them in
.toString()
; options are ignored in these cases (so the attempt to render, say, Infinity with 4 significant digits just returns"Infinity"
).