fix(stringWidth): implement GB9c for Indic conjunct grapheme breaking - #26365
chrislloyd wants to merge 2 commits into
Conversation
Bun.stringWidth incorrectly returns 2 for Indic script conjuncts like Devanagari क्ष (Ka + Virama + Ssa) which render as a single glyph and should have width 1. The root cause is two-fold: 1. The grapheme breaking algorithm did not implement Unicode 15.1's GB9c rule (Indic Conjunct Break), which prevents grapheme cluster breaks between consonants joined by a virama (halant). Without this rule, the second consonant starts a new grapheme cluster. 2. The width calculation in GraphemeState.width() accumulated widths of all non-zero-width codepoints in a cluster. For conjuncts that render as a single glyph, only the first consonant's width should count. Changes: - Add InCB (Indic_Conjunct_Break) state tracking to graphemeBreak() alongside the existing precomputed GBC table (no table regeneration) - Add isInCBLinker() covering virama characters across all major Indic scripts (Devanagari, Bengali, Gurmukhi, Gujarati, Oriya, Tamil, Telugu, Kannada, Malayalam, Sinhala, plus extended scripts) - Add isInCBConsonant() for the consonant ranges of those scripts - Add has_incb_linker flag to GraphemeState so width() returns base_width for conjunct graphemes - Add test cases for Devanagari conjuncts with and without ZWJ
WalkthroughImplements Indic Conjunct Break (GB9c) tracking by adding InCBState and PrecomputedState, refactoring BreakState to separate precomputed data from InCB progression, integrating GB9c logic into grapheme boundary computation, and updating visible-state width logic to treat Indic conjuncts as single glyphs. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/string/immutable/grapheme.zig`:
- Around line 138-163: The isInCBConsonant function is missing and has incorrect
Unicode ranges per Unicode 15.1; update the function (isInCBConsonant in
src/string/immutable/grapheme.zig) to match IndicConjunctBreak.txt by adding all
missing script ranges (e.g., add Devanagari 0x0978; Bengali 0x09F0..0x09F1;
Gujarati 0x0ABD..0x0AC2 and 0x0AC9; Malayalam 0x0D60..0x0D61, 0x0D66..0x0D6F,
0x0D7A..0x0D7F; Sinhala 0x0D85..0x0D96) and include the additional scripts
listed in Unicode 15.1 (Thai, Lao, Tibetan, Old Italic, Gothic, Balinese,
Sundanese, Myanmar Extended-B, Tai Viet, Javanese, etc.) by adding their ranges
to the switch arms in isInCBConsonant so the switch exactly mirrors
IndicConjunctBreak.txt.
| /// InCB=Consonant: Indic script consonants (base letters that form conjuncts) | ||
| fn isInCBConsonant(cp: u21) bool { | ||
| return switch (cp) { | ||
| // Devanagari consonants | ||
| 0x0915...0x0939, 0x0958...0x095F, 0x0979...0x097F, | ||
| // Bengali consonants | ||
| 0x0995...0x09A8, 0x09AA...0x09B0, 0x09B2, 0x09B6...0x09B9, 0x09DC...0x09DD, 0x09DF, | ||
| // Gurmukhi consonants | ||
| 0x0A15...0x0A28, 0x0A2A...0x0A30, 0x0A32...0x0A33, 0x0A35...0x0A36, 0x0A38...0x0A39, 0x0A59...0x0A5C, 0x0A5E, | ||
| // Gujarati consonants | ||
| 0x0A95...0x0AA8, 0x0AAA...0x0AB0, 0x0AB2...0x0AB3, 0x0AB5...0x0AB9, | ||
| // Oriya consonants | ||
| 0x0B15...0x0B28, 0x0B2A...0x0B30, 0x0B32...0x0B33, 0x0B35...0x0B39, 0x0B5C...0x0B5D, 0x0B5F, | ||
| // Tamil consonants | ||
| 0x0B95, 0x0B99...0x0B9A, 0x0B9C, 0x0B9E...0x0B9F, 0x0BA3...0x0BA4, 0x0BA8...0x0BAA, 0x0BAE...0x0BB9, | ||
| // Telugu consonants | ||
| 0x0C15...0x0C28, 0x0C2A...0x0C39, 0x0C58...0x0C5A, | ||
| // Kannada consonants | ||
| 0x0C95...0x0CA8, 0x0CAA...0x0CB3, 0x0CB5...0x0CB9, 0x0CDE, | ||
| // Malayalam consonants | ||
| 0x0D15...0x0D3A, 0x0D4E, | ||
| // Sinhala consonants | ||
| 0x0D9A...0x0DC6, | ||
| => true, | ||
| else => false, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Unicode 15.1 IndicConjunctBreak.txt InCB=Consonant character ranges
💡 Result:
The InCB=Consonant ranges for Unicode 15.1 are listed in the Unicode UCD file IndicConjunctBreak.txt (Indic_Conjunct_Break). See the full list in the UCD; example excerpt (Unicode 15.1 / DerivedCoreProperties build showing InCB=Consonant ranges): 0915..0939; 0958..095F; 0978..097F; 0995..09A8; 09AA..09B0; 09B2; 09B6..09B9; 09DC..09DD; 09DF; 09F0..09F1; 0A95..0AA8; 0AAA..0AB0; 0AB2..0AB3; … (complete canonical list is in IndicConjunctBreak.txt). [1][2]
References
- Derived/core-data excerpt showing InCB=Consonant ranges (Unicode 15.1). [1]
- PropertyValueAliases / Indic_Conjunct_Break (InCB) listing (Unicode 15.1). [2]
If you want, I can fetch and paste the complete, verbatim InCB=Consonant range list from IndicConjunctBreak.txt.
🌐 Web query:
complete list Unicode 15.1 IndicConjunctBreak.txt InCB=Consonant all ranges
💡 Result:
Do you want me to fetch the official Unicode 15.1 extracted file (IndicConjunctBreak.txt) and return the complete InCB=Consonant ranges (raw file or formatted list)?
🌐 Web query:
fetch Unicode 15.1 official IndicConjunctBreak.txt file complete InCB=Consonant ranges formatted list
💡 Result:
I can fetch that and return the full list of InCB=Consonant ranges from the official Unicode 15.1 IndicConjunctBreak.txt. Do you want them (a) as hexadecimal ranges (e.g. 0A95..0A96), (b) as code points with names, or (c) both?
🌐 Web query:
Unicode 15.1 IndicConjunctBreak.txt InCB=Consonant ranges hexadecimal format complete list
💡 Result:
I can fetch and return the complete hexadecimal ranges for InCB=Consonant from Unicode 15.1 — do you want the full list now? (I’ll pull the authoritative UCD file and paste the ranges.) (unicode.org)
Citations:
🌐 Web query:
site:unicode.org IndicConjunctBreak.txt 15.1 Consonant
💡 Result:
The Unicode UCD includes an Indic_Conjunct_Break (InCB) data file named IndicConjunctBreak.txt (e.g. in the Public/15.1.0/ucd/auxiliary/ location). One of the InCB property values is "Consonant". [1][2]
Sources:
- Property value aliases showing InCB; Consonant. [1]
- Unicode UAX/common references listing the auxiliary file IndicConjunctBreak.txt (version 15.1). [2]
🌐 Web query:
unicode.org/Public/15.1.0/ucd/auxiliary/IndicConjunctBreak.txt
💡 Result:
The specific file URL you requested (https://www.unicode.org/Public/15.1.0/ucd/auxiliary/IndicConjunctBreak.txt) returns 404 — it is not available at that path. ()
For the Unicode definition and derived data for Indic_Conjunct_Break, see the Unicode Character Database / TR44 description of the property and the Grapheme Cluster (segmentation) test material (these point to the current UCD auxiliary data in the Public/UCD/… area). (unicode.org)
If you want, I can:
- fetch the Indic_Conjunct_Break data from the Unicode UCD "latest" directory (e.g., https://www.unicode.org/Public/UCD/latest/ucd/auxiliary/) and return it; or
- look up the equivalent file in a specific Unicode release (e.g., 15.0.0, 16.0.0, 17.0.0). Which would you prefer?
Citations:
🌐 Web query:
Unicode 15.1 Indic Conjunct Break Consonant code points comprehensive list hexadecimal
💡 Result:
The complete set of code points with Indic_Conjunct_Break=Consonant for Unicode 15.1 is published in the Unicode Character Database (PropList / Derived property files). Download the UCD for 15.1 and inspect the Indic_Conjunct_Break (InCB) section (PropList/DerivedCoreProperties). [1][2]
If you want the property as ranges (hex) ready to use, see the extracted list (Indic_Conjunct_Break=Consonant) published in platform updates that mirror the UCD; example excerpt (ranges are hexadecimal):
0915..0939
0958..095F
0978..097F
0995..09A8
09AA..09B0
09B2
09B6..09B9
09DC..09DD
09DF
09F0..09F1
0A95..0AA8
0AAA..0AB0
0AB2..0AB3
0AB5..0AB9
0ABD..0AC2
0AC9
0B95..0BB9
0C15..0C39
0C58..0C59
0C95..0CB9
0D15..0D39
0D60..0D61
0D66..0D6F
0D7A..0D7F
0D85..0D96
0D9A..0DB1
0DC0..0DC6
0E01..0E30
0E31
0E32..0E33
0E40..0E46
0E4F..0E5B
0E81..0E82
0E84
0E87..0E8A
0EA1..0EA3
0EA5
0EA7..0EA8
0EAA..0EAB
0EAD..0EB0
0EB2..0EB3
0EBD
0EC0..0EC4
0EC6
0EDC..0EDF
0F40..0F47
0F49..0F6C
0F88..0F8C
10300..10323
1032D
10350..10375
A8F2..A8F7
1B05..1B33
1B45..1B4B
1B83..1BA0
1BAE..1BBA
AA60..AA76
AAB0..AAB0
AAB2..AAB4
AAB7..AAB8
AABE..AABF
AAC0..AAC0
A9E0..A9E4
(For the authoritative, full, and canonical listing including exact singletons and all ranges, download the Unicode 15.1 UCD files — PropList / DerivedCoreProperties — or inspect the Indic_Conjunct_Break section in the UCD zip.) [1][2]
Sources:
- Unicode 15.1 UCD (UCD.zip / PropList / Derived files). [1]
- Platform extraction showing Indic_Conjunct_Break=Consonant ranges (example: GCC / OpenJDK updates that mirror UCD). [2]
Add missing InCB=Consonant ranges from Unicode 15.1.
The function is incomplete. Unicode 15.1 IndicConjunctBreak.txt includes consonant ranges for Thai, Lao, Tibetan, Old Italic, Gothic, Balinese, Sundanese, Myanmar Extended-B, Tai Viet, Javanese, and others that are not covered. Additionally, within the Indic scripts already listed, several ranges are missing or incorrect:
- Devanagari: missing
0x0978 - Bengali: missing
0x09F0..0x09F1 - Gujarati: missing
0x0ABD..0x0AC2,0x0AC9 - Malayalam: missing
0x0D60..0x0D61,0x0D66..0x0D6F,0x0D7A..0x0D7F - Sinhala: missing
0x0D85..0x0D96
Review the complete Unicode 15.1 IndicConjunctBreak.txt file to add all missing ranges and correct existing ones.
🤖 Prompt for AI Agents
In `@src/string/immutable/grapheme.zig` around lines 138 - 163, The
isInCBConsonant function is missing and has incorrect Unicode ranges per Unicode
15.1; update the function (isInCBConsonant in src/string/immutable/grapheme.zig)
to match IndicConjunctBreak.txt by adding all missing script ranges (e.g., add
Devanagari 0x0978; Bengali 0x09F0..0x09F1; Gujarati 0x0ABD..0x0AC2 and 0x0AC9;
Malayalam 0x0D60..0x0D61, 0x0D66..0x0D6F, 0x0D7A..0x0D7F; Sinhala
0x0D85..0x0D96) and include the additional scripts listed in Unicode 15.1 (Thai,
Lao, Tibetan, Old Italic, Gothic, Balinese, Sundanese, Myanmar Extended-B, Tai
Viet, Javanese, etc.) by adding their ranges to the switch arms in
isInCBConsonant so the switch exactly mirrors IndicConjunctBreak.txt.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/string/immutable/grapheme.zig`:
- Around line 53-61: Make the struct fields private by prefixing their names
with `#`: change BreakState fields `precomputed` and `incb` to `#precomputed`
and `#incb`, and change PrecomputedState fields `extended_pictographic` and
`regional_indicator` to `#extended_pictographic` and `#regional_indicator`;
update all internal references in grapheme.zig that read/write these fields
(look for uses of BreakState and PrecomputedState within this file) so they use
the new private names, leaving external callers (which only
construct/passthrough BreakState) unchanged.
♻️ Duplicate comments (1)
src/string/immutable/grapheme.zig (1)
141-199: Confirm InCB=Consonant coverage against Unicode 15.1.
The switch currently covers a subset of scripts; if full UAX#29compliance is intended, verify the ranges against IndicConjunctBreak.txt and add any missing scripts/ranges.Unicode 15.1 IndicConjunctBreak.txt InCB=Consonant ranges (full list)
| pub const BreakState = struct { | ||
| precomputed: PrecomputedState = .{}, | ||
| incb: InCBState = .none, | ||
| }; | ||
|
|
||
| const PrecomputedState = packed struct(u2) { | ||
| extended_pictographic: bool = false, | ||
| regional_indicator: bool = false, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, find the grapheme.zig file to understand its structure
find . -name "grapheme.zig" -type fRepository: oven-sh/bun
Length of output: 90
🏁 Script executed:
# Search for external usage of .precomputed and .incb fields
rg -n '\.precomputed\b' --type=zig
rg -n '\.incb\b' --type=zigRepository: oven-sh/bun
Length of output: 1157
🏁 Script executed:
# Search for BreakState instantiations and usage
rg -n '\bBreakState\b' --type=zig -A 2 -B 2Repository: oven-sh/bun
Length of output: 1540
🏁 Script executed:
# Search for PrecomputedState usage
rg -n '\bPrecomputedState\b' --type=zig -A 2 -B 2Repository: oven-sh/bun
Length of output: 2064
Make BreakState and PrecomputedState fields private using # prefix.
These fields are accessed only within grapheme.zig. External code in visible.zig merely instantiates BreakState and passes it to functions; it never directly accesses the fields. Mark them private to match the repo's Zig guidelines and avoid unnecessary API coupling.
♻️ Proposed refactor
pub const BreakState = struct {
- precomputed: PrecomputedState = .{},
- incb: InCBState = .none,
+ `#precomputed`: PrecomputedState = .{},
+ `#incb`: InCBState = .none,
};
const PrecomputedState = packed struct(u2) {
- extended_pictographic: bool = false,
- regional_indicator: bool = false,
+ `#extended_pictographic`: bool = false,
+ `#regional_indicator`: bool = false,
};🤖 Prompt for AI Agents
In `@src/string/immutable/grapheme.zig` around lines 53 - 61, Make the struct
fields private by prefixing their names with `#`: change BreakState fields
`precomputed` and `incb` to `#precomputed` and `#incb`, and change
PrecomputedState fields `extended_pictographic` and `regional_indicator` to
`#extended_pictographic` and `#regional_indicator`; update all internal
references in grapheme.zig that read/write these fields (look for uses of
BreakState and PrecomputedState within this file) so they use the new private
names, leaving external callers (which only construct/passthrough BreakState)
unchanged.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
This will cause a performance hit. I'd like to see if we can fix this in the table generator code instead of adding extra code for this special case. I also wonder if ghostty has made any recent changes to their grapheme code that we can update, it was pre 1.0 at the time we started using this code IIRC.
|
Closing in favor of #26376 |
2 participants
Summary
Bun.stringWidthincorrectly returns 2 for Indic script conjuncts like Devanagariक्ष(Ka + Virama + Ssa) which render as a single glyph and should have width 1.Root Cause
The grapheme breaking algorithm did not implement Unicode 15.1's GB9c rule (Indic Conjunct Break), which prevents grapheme cluster breaks between consonants joined by a virama (halant). Without this rule, the second consonant starts a new grapheme cluster.
The width calculation in
GraphemeState.width()accumulated widths of all non-zero-width codepoints in a cluster. For conjuncts that render as a single glyph, only the first consonant's width should count.Changes
InCBStatetracking tographemeBreak()alongside the existing precomputed GBC table (no table regeneration needed)isInCBLinker()covering virama characters across all major Indic scripts (Devanagari, Bengali, Gurmukhi, Gujarati, Oriya, Tamil, Telugu, Kannada, Malayalam, Sinhala, plus extended scripts like Brahmi, Kaithi, Chakma, etc.)isInCBConsonant()for the consonant ranges of those scriptshas_incb_linkerflag toGraphemeStatesowidth()returnsbase_widthfor conjunct graphemesTest Cases
Design
The GB9c implementation is layered on top of the existing precomputed grapheme break table without modifying or regenerating it. The InCB state machine runs alongside the precomputed GBC lookup:
Consonant [Extend|Linker]* Linker [Extend|Linker]* × ConsonantindicConjunctBreakProperty()function classifies codepoints using the existing GBC table for Extend detection and explicit switch statements for Linker/Consonant