-
Notifications
You must be signed in to change notification settings - Fork 30k
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
[v6.x backport] doc: add documentation on ICU #15353
Conversation
doc/api/intl.md
Outdated
console.log(english.format(january)); | ||
// Prints "January" | ||
console.log(spanish.format(january)); | ||
// Prints "January" on small-icu |
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.
Default v6.11.3 prints M01
for me (Windows 7 x64).
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.
Hmm, it's M01 for me on master as well but January on v6.x. Can you check the output after changing your LC_ALL
environment variable to en_US
?
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.
Output:
> set LC_ALL=en_US
> set LC_ALL
LC_ALL=en_US
> node.6.11.3.v8-5.1.exe
> const january = new Date(9e8);
> const english = new Intl.DateTimeFormat('en', { month: 'long' });
> const spanish = new Intl.DateTimeFormat('es', { month: 'long' });
> console.log(english.format(january));
January
> // Prints "January"
> console.log(spanish.format(january));
M01
> // Prints "January" on small-icu
> // Should print "enero"
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.
FWIW, I use the standalone binary from https://nodejs.org/download/release/latest-v6.x/win-x64/
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.
Ah you are on Windows. That's different...
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.
So maybe we can state "January" or "M01"
?
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.
@vsemozhetbyt Sounds like a good idea.
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.
With code output recheck.
PR-URL: nodejs#13916 Refs: nodejs#13644 (comment) Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
abbf810
to
d7293be
Compare
@vsemozhetbyt Done, PTAL! |
landed in 90fcccd |
PR-URL: #13916
Refs: #13644 (comment)
Reviewed-By: Vse Mozhet Byt vsemozhetbyt@gmail.com
Checklist
Affected core subsystem(s)
doc