-
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
Add version info: CLDR, Unicode, Timezone #9237
Comments
Yeah, I guess using a getter should work for that, similar to how the builtin modules are defined in the REPL (src)? We might customize (So, yes, I’m okay with using |
I'm +1 for this. Using a getter as @addaleax suggests is the right approach here.. and definite +1 to using |
Aren't those processed at compile time? |
No, they are in the data file. You can actually upgrade the tz version without recompiling, for example. By "data file" this includes linked-in data, but that still has a loading process which includes looking for overrides on disk, intialization, etc. I don't want "all users" to pay for this if we can avoid it. Checking @addaleax @jasnell thanks for the help with the getter idea. |
I committed a WIP so far. The Sample output:
|
@srl295 You can do it if you see advantages to it, but generally there should be no need to cache things on the C++ side; You can insert code into the getter to remove the getter itself and replace it with the actual value, so for a second access of the property no Node.js code actually gets executed (the bit of code linked above does that, for example). |
@addaleax thanks! It's fun to add bound functions! (And I get to contribute actual JavaScript, imagine that…)
Anyway- TODO's:
|
how about an acutal PR also. |
Oh and
|
* Adds process.versions.cldr, .tz, and .unicode * Changes how process.versions.icu is loaded * Lazy loads the process.versions.* values for these * add an exception to util.js to cause 'node -p process.versions' to still work * update process.version docs Fixes: nodejs#9237
@sam-github pointed out that 4fb27d4 was landed with bad formatting. |
^^ FYI @jasnell @rvagg @thealphanerd 😢 |
@srl295 bad formatting as in missing metadata or code formatting? |
Missing metadata : PR and reviewed-by. On Friday, October 28, 2016, Rod Vagg [email protected] wrote:
|
* Adds process.versions.cldr, .tz, and .unicode * Changes how process.versions.icu is loaded * Lazy loads the process.versions.* values for these * add an exception to util.js to cause 'node -p process.versions' to still work * update process.version docs PR-URL: #9266 Fixes: #9237 Reviewed-By: James M Snell <[email protected]>
@srl295 should this be backported? |
ping @srl295 |
Yes would be good to backport
|
@srl295 this will need a manual backport then, would you be able to help? |
Yes, but busy for a couple weeks. AFAIK It was rewritten some since I wrote
it, so backport ing the improvements would be good rather than just as
originally landed.
|
@srl295 we can backport whatever the latest version is |
It could be handy to have some additional ICU version information available, besides just the
icu
version itself. since the ICU features have more and more applicability to various parts of Node.These could go into
process.versions
, or not.Note that for the CLDR and TimeZone version, we actually need to read data files to get the answer. So, I'd hesitate to just stuff constants into
process.versions
. Is there a way to lazily-initialize a constant?Unicode Data Version
This gives information about which Unicode Version is included. This would affect which characters are interpreted by regexes, etc.
Implementation:
u_getUnicodeVersion()
(doesn't actually read any data files to get the result)CLDR Version
This is the version of the CLDR data files used for ICU's implementation.
Time Zone Data Version
This is the version of the iana tz database.
cc: @nodejs/intl
The text was updated successfully, but these errors were encountered: