fix(svg): non-ASCII text misaligns token background rects - #1346
Conversation
SVG formatter was using len() to measure line and token widths, which counts bytes. Non-ASCII runes would inflate the width and misalign token background rects. Now uses utf8.RuneCountInString via a shared tokenWidth helper.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9430a9706c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // tokenWidth is the token's width in character cells, with tabs expanded. | ||
| func tokenWidth(token chroma.Token) int { | ||
| return utf8.RuneCountInString(strings.ReplaceAll(token.String(), ` `, " ")) |
There was a problem hiding this comment.
For decomposed Unicode text such as e\u0301, the combining accent has no horizontal advance in the rendered SVG, but RuneCountInString counts it as another character cell. Consequently, the token background is one ch too wide and every later background on the line is shifted, so the non-ASCII alignment bug remains for normalized NFD input; East Asian wide runes have the converse problem because they commonly occupy two cells but are counted once.
Useful? React with 👍 / 👎.
2 participants
The SVG formatter measures line widths in bytes rather than characters, so any non-ASCII rune inflates the canvas and pushes token background rects out of alignment with the text they sit behind.
Tokenising
x = ["éé", "ab"]with a style that sets a string background:"éé"is 4 characters and 6 bytes. The old code gives its rect a width of 6 cells, then starts the next rect 2 cells too far right, so the"ab"highlight is visibly offset from"ab". Every token after the first non-ASCII one on a line inherits that drift, and the<svg width>is overstated by the same amount.xandwidthare emitted inchunits, which are character cells in a monospace font, so the count has to be in runes.The fix
maxLineWidthandwriteTokenBackgroundscomputed the same expression withlen(). Extracted it astokenWidthusingutf8.RuneCountInString, keeping the existing tab expansion.Pure-ASCII output is unchanged. Rendering
lexer.gofrom this repo to SVG before and after gives byte-identical files.Verification
TestNonASCIIWidthsinformatters/svg/svg_test.gobuilds a style with a string background so the rect path is exercised, then asserts the canvas width and both rect offsets.Reverting
svg.gofails it, and the failure output shows the misalignment directly:go test ./...passes andgolangci-lint runis clean.Disclosure: written with AI assistance (Claude Code). I produced the before and after by rendering real files through both builds, and ran the revert check myself.