Skip to content

fix: align CJK ASCII hyphen wrapping with browsers - #215

Open
ChuanfengZhang wants to merge 2 commits into
chenglou:mainfrom
ChuanfengZhang:fix/cjk-ascii-hyphen-wrapping
Open

fix: align CJK ASCII hyphen wrapping with browsers#215
ChuanfengZhang wants to merge 2 commits into
chenglou:mainfrom
ChuanfengZhang:fix/cjk-ascii-hyphen-wrapping

Conversation

@ChuanfengZhang

Copy link
Copy Markdown

fix #214

@pyrex41

pyrex41 commented Jul 22, 2026

Copy link
Copy Markdown

Nice fix — the numeric-minus handling (gluing 度-5 on both sides, gated on unitContainsCJK, with allowOverflowBreaks as the escape hatch for overlong glued units) reads carefully, and the 日本語foo-5 test is a good discriminator showing Latin foo- still breaks.

One clarifying question before it lands: the numeric-minus logic is CJK-gated, but the '-' addition to kinsokuStart is global — it now prohibits a hyphen from starting any wrapped line, not just CJK-adjacent ones. Have you confirmed that doesn't change line-start behavior in non-CJK contexts where a wrapped line would legitimately begin with a hyphen-minus? I'm thinking of things like code/identifier text or numeric ranges (a - b, foo -bar, an option list) where the browser would allow the - at the start of a line. The existing Latin/URL tests suggest it's fine, but a test that pins the non-CJK "hyphen may start a line" case (asserting no regression vs. main) would make the global kinsokuStart change safe to reason about.

Not a blocker — just want to be sure the global set change is intentional and side-effect-free outside CJK.

@pyrex41

pyrex41 commented Jul 22, 2026

Copy link
Copy Markdown

Followed up on my own question — I traced it and diffed both branches, and the answer is reassuring: the global kinsokuStart '-' addition is inert outside CJK. Sharing so you don't have to redo the legwork.

Trace — all three consult sites are CJK-guarded or already shadowed:

  • buildBaseCjkUnits (layout.ts) is only called under if (segKind === 'text' && segMetrics.containsCJK), so pure-Latin segments never reach it. For Latin sub-runs inside a mixed segment the new kinsokuStart.has(grapheme) branch calls the same appendToUnit the old fall-through did — it only diverges when unitContainsCJK is already true (the bug being fixed).
  • isCJKLineStartProhibitedSegment (analysis.ts) is consumed only in the && tailContainsCJK branch of buildMergedSegmentation.
  • The keep-all path already had - in keepAllDashBreakChars, so canContinueKeepAllTextRun returns the same result before/after.

Empirical — built both branches (bun runs the TS sources directly with the repo's fake-canvas backend) and diffed prepareWithSegments + layoutWithLines:

  • 33 adversarial non-CJK inputs (a - b, foo -bar -baz, --flag -x, x = a-b, well-known, option lists…), each width-swept 3px→natural in 0.5px steps under default / pre-wrap / keep-all / break-all / break-word → byte-identical main vs PR.
  • A 400-string fuzz including Latin, digits, Arabic, Hebrew, Devanagari, Thai → ~19k lines/side, identical.
  • Positive control: the BUG: CJK text can wrap with an ASCII hyphen at the start of a line #214 CJK repro does change (44 differing lines across the sweep), so the harness isn't blind — the non-CJK identity is real, not a false negative.
  • bun test on the branch: 120 pass, 0 fail.

So this is safe as-is. The only optional polish would be gating the set membership behind a CJK check (or adding a non-CJK no-regression test) to make the intent legible — purely cosmetic, not a correctness issue. Nice fix.

@ChuanfengZhang

Copy link
Copy Markdown
Author

Thank you @pyrex41 so much for digging into this so thoroughly, and especially for following up on your own question with the trace, branch diff, width sweeps, fuzzing, and positive control. That saved me a ton of legwork and gave me much more confidence that the global kinsokuStart addition is truly inert outside CJK.
I really appreciate how patient and concrete this review was. I’ve pushed a small non-CJK regression test using foo -bar, just to make the intent explicit and guard against future call sites accidentally broadening the behavior. Honestly, this was an exceptionally thoughtful review. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: CJK text can wrap with an ASCII hyphen at the start of a line

2 participants