-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: cross-link util.TextDecoder and intl.md #14486
Conversation
doc/api/intl.md
Outdated
| [WHATWG URL Parser][] | partial (no IDN support) | full | full | full | ||
| [`require('buffer').transcode()`][] | none (function does not exist) | full | full | full | ||
| [REPL][] | partial (inaccurate line editing) | full | full | full | ||
| [`util.TextDecoder`][] | REPLACEME | REPLACEME | partial (`'utf8'`, `'utf-16be'`, `'utf-16le'`) | full (except `'iso-8859-16'`) |
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.
Hah, the TextDecoder
addition actually broke the no-icu build. I'll fix that. Edit: #14489
For documentation purposes, this should do fine after no-icu is fixed.
| [`require('util').TextDecoder`][] | none (class does not exist) | partial/full (depends on OS) | partial (Unicode-only) | full
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.
Done.
doc/api/intl.md
Outdated
@@ -16,6 +16,7 @@ programs. Some of them are: | |||
- The [WHATWG URL parser][]'s [internationalized domain names][] (IDNs) support | |||
- [`require('buffer').transcode()`][] | |||
- More accurate [REPL][] line editing | |||
- [`util.TextDecoder`][] |
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.
require('util').TextDecoder
?
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.
I am not sure. Is the require('buffer').transcode()
due to the note: "Note that this is a property on the buffer
module returned by require('buffer')
, not on the Buffer
global or a Buffer
instance"? If not, I will add require()
for consistency.
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.
Changed. Let me know to revert if this is reconsidered.
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.
That's fine then.
@@ -184,26 +186,27 @@ to be helpful: | |||
- [Test262][]: ECMAScript's official conformance test suite includes a section | |||
dedicated to ECMA-402. | |||
|
|||
[btest402]: https://github.com/srl295/btest402 |
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.
I like the original order better, which does not take into account capitalization and any special characters when sorting...
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.
It seems this is the dominating style in the docs after #12726
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.
Okay…
@@ -560,7 +560,7 @@ string += decoder.decode(); // end-of-stream | |||
Per the [WHATWG Encoding Standard][], the encodings supported by the | |||
`TextDecoder` API are outlined in the tables below. For each encoding, | |||
one or more aliases may be used. Support for some encodings is enabled | |||
only when Node.js is using the full ICU data. | |||
only when Node.js is using the full ICU data (see [Internationalization][]). |
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.
Explicitly say util.TextDecoder
is undefined
when ICU is not enabled during build.
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.
Done. Correct me re place or wording if this is needed.
Also, note about builds without ICU and ASCII-sort some bottom references.
Landed in 9623ce5 |
Also, note about builds without ICU and ASCII-sort some bottom references. PR-URL: #14486 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Also, note about builds without ICU and ASCII-sort some bottom references. PR-URL: #14486 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
Affected core subsystem(s)
doc, intl, util
Also: note about builds without ICU and ASCII-sort some bottom references.
I am not sure what to write in two table cells, so I've left(see #14486 (comment))REPLACEME
there. Please, let me know how to correct them.cc @jasnell