Skip to content
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

Infinity should be treated as plural #108

Merged
merged 1 commit into from
Jan 21, 2020

Conversation

nterray
Copy link
Contributor

@nterray nterray commented Jan 21, 2020

In plural.js, parseInt()[0] is used to check if the given input is a
number. However the radix argument for parseInt() is omitted which can
produce unexpected behaviors; this is discouraged by the MDN[1].

Anyways if we are using Infinity (for example with $ngettext(singular, plural, Infinity)) then the expected translation should be plural instead of
singular.

[0] https://tc39.es/ecma262/#sec-parseint-string-radix
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt

In plural.js, `parseInt()`[0] is used to check if the given input is a
number. However the radix argument for `parseInt()` is omitted which can
produce unexpected behaviors; this is discouraged by the MDN[1].

Anyways if we are using Infinity (for example with `$ngettext(singular, plural,
Infinity)`) then the expected translation should be plural instead of
singular.

[0] https://tc39.es/ecma262/#sec-parseint-string-radix
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt
@kemar
Copy link
Contributor

kemar commented Jan 21, 2020

The radix of parseInt is assumed to be base 10 when unspecified since ECMAScript 5 Strict Mode.

Quoting the ES 2020 spec you referenced:

If radix is undefined or 0, it is assumed to be 10

But I like the explicit conversion with Number() :)

Thanx @nterray

@kemar kemar merged commit 70699ea into Polyconseil:master Jan 21, 2020
@kemar
Copy link
Contributor

kemar commented Jan 21, 2020

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.

2 participants