Repository navigation
Conversation
Regenerate src/third-party/UnicodeProperties.ts from PerBothner/unicode-properties a58885f (out-ts), keeping the xterm.js local changes (import path, Buffer/atob decoder, infoToWidth return type). tiny-inflate.ts is unchanged, and the unicode-trie.ts byteOffset fix from upstream is left to xtermjs#6080. This brings in from upstream: - Unicode 18.0.0 data tables - GB11: only join ZWJ x ExtPic after ExtPic Extend* (PerBothner/unicode-properties#2) - GB9c: Indic_Conjunct_Break support (PerBothner/unicode-properties#3) Also use the now exported GRAPHEME_BREAK_SAW_Regional_Pair constant, update the docs that mention the Unicode version, add unit tests for the provider, and fix a typo (U+F3CB for U+1F3CB) in the integration test that the old GB11 behavior hid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #6241, which patched the generated
UnicodeProperties.tsdirectly and was closed. As @PerBothner asked there, the fixes went to the generator first, and this PR re-imports the generated files.The files are regenerated from PerBothner/unicode-properties at commit a58885f6811d8db9b0729ddf23a1f786494608da (
npm install; makewith the Unicode 18.0.0 data files from https://www.unicode.org/Public/18.0.0/). That commit includes:unicode-triereads the trie header at the view'sbyteOffset. This is the same change as the open Read the unicode trie header at the view's byteOffset #6080 (for addon-unicode-graphemes: trie header is read via DataView(data.buffer) ignoring byteOffset, so a pooled base64 decode silently corrupts widths #6079), so this PR leavesunicode-trie.tsalone and that fix to Read the unicode trie header at the view's byteOffset #6080.ZWJ × ExtPicafter anExtPic Extend*prefix.UNICODE_VERSION18.0.0.Indic_Conjunct_Breakdata and rule GB9c, in its Unicode 18 form\p{InCB=Linker} \p{InCB=Extend}* × \p{InCB=Consonant}.Changes
src/third-party/UnicodeProperties.ts: replaced without-ts/uc-properties.ts. Three local differences that the vendored file already had are kept:./unicode-trie(upstream uses./unicode-trie/index.mjs);_dec()decoder, which usesBufferin node andatobotherwise (upstream useswindow.atob);0 | 1 |2return type ofinfoToWidth.Apart from these, the file is identical to the generator output.
src/third-party/unicode-trie.ts: not changed. The regeneratedout-ts/unicode-trie.tsdiffers from it only by thebyteOffsetfix, which is Read the unicode trie header at the view's byteOffset #6080.src/third-party/tiny-inflate.ts:out-ts/tiny-inflate.tsis byte-identical to the vendored copy, so the file is not changed.UnicodeGraphemeProvider.ts: uses the newly exportedUC.GRAPHEME_BREAK_SAW_Regional_Pairconstant in place of the literal32.README and the comment in
UnicodeGraphemesAddon.ts: now say the data is Unicode 18. The README also gets the correct path of the generated file (src/third-party/) and lists the two helper files that come from the sameout-tsdirectory.test/UnicodeGraphemesAddon.test.ts: fixes a typo. The "woman lifting weights, medium dark" case used\u{F3CB}(a private-use character) where\u{1F3CB}was meant. With the old GB11 behaviour the ZWJ after it still joined the following U+2640, so the expected width of 7 came out right by accident. With correct GB11 the typo gives 8. With\u{1F3CB}, which is what the comment describes, the width is 7 again.New
src/UnicodeGraphemeProvider.test.ts: mocha unit tests for the provider's cluster boundaries and widths. They cover GB11, GB9c, and two Unicode 16+ data changes: U+0B53 is now Extend, and U+2630 is now East_Asian_Width=W.Data changes
The old and new tables were decoded for every code point from U+0000 to U+10FFFF, with the old one read through the fixed trie reader. On the grapheme-break and width bits, 2444 code points differ. The largest groups are:
emoji-data.txtno longer lists many non-emoji symbols as Extended_Pictographic, for example U+2388, U+2605 and U+1F000..U+1F02B.The InCB bits (
INCB_MASK = 0xC0) are new.Version string: not renamed, question for maintainers
The provider is still registered as
'15'and'15-graphemes', and the addon still setsunicode.activeVersion = '15-graphemes'. These strings are what users pass toterm.unicode.activeVersion. The demo (demo/client/client.ts),test/playwright/TestUtils.tsand the addon benchmark all set'15-graphemes', and@xterm/addon-unicode-graphemesis published on npm (0.4.0, plus 0.5.0 betas). Renaming them would break existing callers, so this PR leaves them alone.Should they become
'18'/'18-graphemes', perhaps with the'15'names kept as aliases? If so, that could go in this PR or in a follow-up. The remaining "15" mentions are all tied to those strings: the playwright test titlewcwidth V15 emoji testand its expectedversionslist, and thev15: 20 cellslabel indemo/client/components/window/testWindow.ts.Testing
GraphemeBreakTest-18.0.0.txt(853 lines) was run through the realUnicodeGraphemeProvider.charPropertiesjoin chain, outside the repo:Buffer.fromin nodeatob(as in a browser)atob(or in node with #6080 applied)The two master rows differ because of the
byteOffsetbug that #6080 fixes: in node, the pooledBufferleaves the trie header at a non-zero offset.80 of the 87 remaining failures involve CR, LF or Control, which the generator folds into
Otheron purpose. The other 7 start with an InCB=Linker, for example÷ 094D × 0915 ÷. They come from how the provider builds the state for the first code point, an xterm-side issue that is left for a separate PR.The addon tests were run, along with the repo's build, unit and lint commands (macOS, Node 22):
npm run build && npm run esbuild: no errors.npm run test-unit -- addons/addon-unicode-graphemes/out-esbuild/*.test.js: 8 passing. With master'sthird-party/files and the new test file, 7 of the 8 fail. With master'sUnicodeProperties.tsplus only the trie fix, 6 of the 8 fail. The 8 also pass on this branch as is, without the trie fix.npm run test-unit: 2411 passing.npm run lint: clean.npm run lint-api: clean.npm run test-integration -- --suite=addon-unicode-graphemes --project=Chromium: 1 passed. Before the typo fix above, it failed on the "woman lifting weights" assertion.npm run test-integration -- --suite=core --project=Chromium: 419 passed, 59 skipped. The core tests load this addon by default.Not run: the Firefox and WebKit integration projects.