Number: Add padding zero to leading decimal #518
Conversation
|
Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA). 📝 Please visit http://contribute.jquery.org/CLA/ to sign. After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know. If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check. |
|
Hmm, I signed the CLA... three times! The first time I used "Rob Garrison", but that didn't match the git |
|
Looks like the issue here is that the CLA checker expects names to have at least one space (like "Rob Garrison"), but you've used "Mottie" as your git author name. Amending the commit to change the author should fix it, since it then matches your first signature. We can also take care of that when landing your patch. |
| QUnit.test( "should parse non-padded decimals", function( assert ) { | ||
| assert.equal( parse( ".14159", properties( "0.0", en ) ), 0.14159 ); | ||
| assert.equal( parse( ".752", properties( "0.0", en ) ), 0.752 ); | ||
| }); |
There was a problem hiding this comment.
Can you add tests for locales that use:
- different decimal point (like Portuguese)?
- different orientation (like Arabic)?
PS: You could format the number in these locales to know what the output would be and remove the 0. :)
There was a problem hiding this comment.
LOL I would if I knew what the other formats looked like. Should I just copy them from other tests?
There was a problem hiding this comment.
You can Globalize("ar").formatNumber(0.14159); 😄 (considering you have loaded data for this locale)
There was a problem hiding this comment.
Ok, something weird is going on... when I use the above code, I get this result ٠٫١٤٢... I removed the ٠ assuming it's the zero.
Then I plug it into Globalize( "ar" ).numberParser()( "٫١٤٢" ) and the result becomes 0.142. I have no idea why it is rounding the number.
If I use Globalize( "pt" ).numberParser()( ",14159" ), the result is 0.14159 as expected.
There was a problem hiding this comment.
Use maximumFractionDigits option when formatting to increase the default, which is 3. For example:
Globalize("ar").formatNumber(0.14159, {maximumFractionDigits: 10})
// > '٠٫١٤١٥٩'Try using ٫١٤١٥٩.
|
Yeah, I updated my config user.name and did the forced update per instructions, then just pushed a regular commit. Still no luck 😞 |
|
@Mottie, thanks for your patch. I've added one comment above about tests. Other than that, it looks great! Thanks! |
|
I still don't know why it's saying I haven't signed the CLA. Maybe I'm in the database more than once, and it's only catching the first instance? |
|
@Mottie, excellent. Thanks for including the tests. Don't worry about the CLA check for this PR, because I can squash using the name from your last commit, which is in the CLA database as @jzaefferer mentioned. |
|
Closed by 07fac0f and published in 1.1.0-rc.3. |
Fixes #443
Ref #353
Ref #454
Ref #517
I know this isn't an ideal solution, but adding a padding zero is way easier than trying to modify the regular expression, then adapting the code that depends on the matches.
New unit tests were added, and all passing.