-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(tui): ambiguous-width mode + circled-digit display insurance (fixes #3301) #3554
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,9 @@ export const TuiConfigFileSchema = z.object({ | |
| render_latex: z.boolean().optional(), | ||
| disable_paste_burst: z.boolean().optional(), | ||
| cache_expiry_hint: z.boolean().optional(), | ||
| /** East Asian Ambiguous chars (① ★ →) cell width: "narrow"=1, "wide"=2, | ||
| * "auto"=detect from locale (CJK locales default wide; upstream #3302). */ | ||
| ambiguous_width: z.enum(['narrow', 'wide', 'auto']).optional(), | ||
|
Comment on lines
+59
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This changes the published CLI's rendering behavior and adds a user-facing AGENTS.md reference: AGENTS.md:L85-L86 Useful? React with 👍 / 👎. |
||
| editor: z | ||
| .object({ | ||
| command: z.string().optional(), | ||
|
|
@@ -84,6 +87,9 @@ export const TuiConfigSchema = z.object({ | |
| /** Present in every normalized config; optional only so hand-built test | ||
| * fixtures from before this field existed still typecheck. */ | ||
| cacheExpiryHint: z.boolean().optional(), | ||
| /** Resolved cell width for East Asian Ambiguous chars; "auto" defers to | ||
| * locale detection at application time. */ | ||
| ambiguousWidth: z.enum(['narrow', 'wide', 'auto']).optional(), | ||
| editorCommand: z.string().nullable(), | ||
| notifications: NotificationsConfigSchema, | ||
| upgrade: UpgradePreferencesSchema, | ||
|
|
@@ -111,6 +117,7 @@ export const DEFAULT_TUI_CONFIG: TuiConfig = TuiConfigSchema.parse({ | |
| renderLatex: true, | ||
| disablePasteBurst: false, | ||
| cacheExpiryHint: true, | ||
| ambiguousWidth: 'auto', | ||
| editorCommand: null, | ||
| notifications: DEFAULT_NOTIFICATIONS_CONFIG, | ||
| upgrade: DEFAULT_UPGRADE_PREFERENCES, | ||
|
|
@@ -198,6 +205,7 @@ export function normalizeTuiConfig( | |
| renderLatex: config.render_latex ?? DEFAULT_TUI_CONFIG.renderLatex, | ||
| disablePasteBurst: config.disable_paste_burst ?? DEFAULT_TUI_CONFIG.disablePasteBurst, | ||
| cacheExpiryHint: config.cache_expiry_hint ?? DEFAULT_TUI_CONFIG.cacheExpiryHint, | ||
| ambiguousWidth: config.ambiguous_width ?? DEFAULT_TUI_CONFIG.ambiguousWidth, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a user manually sets Useful? React with 👍 / 👎. |
||
| editorCommand: command === undefined || command.length === 0 ? null : command, | ||
| notifications: { | ||
| enabled: config.notifications?.enabled ?? DEFAULT_NOTIFICATIONS_CONFIG.enabled, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /** | ||
| * Display-layer sanitizer for assistant-facing text. | ||
| * | ||
| * Circled/parenthesized digit glyphs (①-⑳ ❶-❿ ⓵-⓾ ⓪⓿) are East Asian | ||
| * Ambiguous: CJK terminals render them double-width, so a line that mixes them | ||
| * with single-width digits overlaps and garbles (upstream kimi-code #3302). | ||
| * The width-mode fix (ambiguous_width) only helps when the terminal agrees; | ||
| * when it doesn't, these glyphs are still visually fragile across fonts. | ||
| * Displaying "1." instead never misaligns — the underlying transcript data is | ||
| * untouched, this only rewrites what is painted. | ||
| */ | ||
|
|
||
| const CIRCLED_DIGIT_MAP: ReadonlyMap<number, string> = (() => { | ||
| const map = new Map<number, string>(); | ||
| // ①-⑳ U+2460..U+2473 → 1..20 (circled) | ||
| for (let i = 0; i < 20; i++) map.set(0x2460 + i, `${i + 1}.`); | ||
| // ⑴-⒇ U+2474..U+2487 → 1..20 (parenthesized) | ||
| for (let i = 0; i < 20; i++) map.set(0x2474 + i, `${i + 1}.`); | ||
| // ⒈-⒛ U+2488..U+249B → 1..20 (digit + period glyph) | ||
| for (let i = 0; i < 20; i++) map.set(0x2488 + i, `${i + 1}.`); | ||
| // ⓵-⓾ U+24F5..U+24FE → 1..10 (double-circled) | ||
| for (let i = 0; i < 10; i++) map.set(0x24f5 + i, `${i + 1}.`); | ||
| // ❶-❿ U+2776..U+277F → 1..10 (dingbat negative circled) | ||
| for (let i = 0; i < 10; i++) map.set(0x2776 + i, `${i + 1}.`); | ||
| // ⓪ U+24EA, ⓿ U+24FF → 0. | ||
| map.set(0x24ea, '0.'); | ||
| map.set(0x24ff, '0.'); | ||
| return map; | ||
| })(); | ||
|
|
||
| /** Replace circled/parenthesized digit glyphs with "N." display forms. */ | ||
| export function replaceCircledNumbers(text: string): string { | ||
| // Fast path: bail before iterating code points. | ||
| if (!/[①-⑳⑴-⒇⒈-⒛⓪⓵-⓾⓿❶-❿]/.test(text)) return text; | ||
| let out = ''; | ||
| for (const ch of text) { | ||
| const cp = ch.codePointAt(0)!; | ||
| const replacement = CIRCLED_DIGIT_MAP.get(cp); | ||
| out += replacement ?? ch; | ||
| } | ||
| return out; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { replaceCircledNumbers } from '#/tui/utils/text-sanitize'; | ||
|
|
||
| describe('replaceCircledNumbers (#3302 display-layer insurance)', () => { | ||
| it('replaces ①-⑳ with 1.-20.', () => { | ||
| expect(replaceCircledNumbers('①②③')).toBe('1.2.3.'); | ||
| expect(replaceCircledNumbers('第⑳项')).toBe('第20.项'); | ||
| }); | ||
|
|
||
| it('replaces ❶-❿ and ⓵-⓾ and zero forms', () => { | ||
| expect(replaceCircledNumbers('❶❿')).toBe('1.10.'); | ||
| // ⓵⓾ are double-circled 1 and 10 (U+24F5/U+24FE) | ||
| expect(replaceCircledNumbers('⓵⓾')).toBe('1.10.'); | ||
| expect(replaceCircledNumbers('⑴⒇')).toBe('1.20.'); | ||
| expect(replaceCircledNumbers('⓪⓿')).toBe('0.0.'); | ||
| }); | ||
|
|
||
| it('leaves ordinary text untouched', () => { | ||
| expect(replaceCircledNumbers('plain ASCII 123')).toBe('plain ASCII 123'); | ||
| expect(replaceCircledNumbers('中文没有圈号')).toBe('中文没有圈号'); | ||
| }); | ||
|
|
||
| it('handles the user regression probe', () => { | ||
| expect(replaceCircledNumbers('①测试 ★测试 →测试 α测试')).toBe('1.测试 ★测试 →测试 α测试'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import assert from "node:assert"; | ||
| import { describe, it } from "node:test"; | ||
| import { | ||
| getAmbiguousWidthMode, | ||
| setAmbiguousWidthMode, | ||
| visibleWidth, | ||
| } from "../src/utils.ts"; | ||
|
|
||
| describe("ambiguous width mode (upstream #3302)", () => { | ||
| it("defaults to narrow", () => { | ||
| setAmbiguousWidthMode("narrow"); | ||
| assert.strictEqual(getAmbiguousWidthMode(), "narrow"); | ||
| assert.strictEqual(visibleWidth("①"), 1); | ||
| }); | ||
|
|
||
| it("treats East Asian Ambiguous chars as 2 cells in wide mode", () => { | ||
| setAmbiguousWidthMode("wide"); | ||
| try { | ||
| // ① circled digit, ★ star, → arrow, α greek — all Ambiguous class | ||
| assert.strictEqual(visibleWidth("①"), 2); | ||
| assert.strictEqual(visibleWidth("★"), 2); | ||
| assert.strictEqual(visibleWidth("→"), 2); | ||
| assert.strictEqual(visibleWidth("α"), 2); | ||
| // CJK ideographs were always wide; unaffected by the mode | ||
| assert.strictEqual(visibleWidth("汉"), 2); | ||
| // plain ASCII stays 1 | ||
| assert.strictEqual(visibleWidth("a"), 1); | ||
| } finally { | ||
| setAmbiguousWidthMode("narrow"); | ||
| } | ||
| }); | ||
|
|
||
| it("padded columns align when mixing circled digits and CJK in wide mode", () => { | ||
| setAmbiguousWidthMode("wide"); | ||
| try { | ||
| // The regression shape from #3302: a line whose ambiguous glyphs were | ||
| // undercounted by 1 cell each wrapped/overlapped its neighbor. | ||
| const a = "①测试"; | ||
| const b = "1.测试"; | ||
| assert.strictEqual(visibleWidth(a), visibleWidth("1.") + visibleWidth("测试")); | ||
| assert.strictEqual(visibleWidth(b), visibleWidth("1.") + visibleWidth("测试")); | ||
| // The user's regression probe: ①测试 ★测试 →测试 α测试 — every token | ||
| // must sum to its true cell count so no padding overlap can occur. | ||
| assert.strictEqual(visibleWidth("★测试 →测试 α测试"), 2 + 4 + 1 + 2 + 4 + 1 + 2 + 4); | ||
| } finally { | ||
| setAmbiguousWidthMode("narrow"); | ||
| } | ||
| }); | ||
| }); |
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.
Whenever assistant Markdown contains one of these glyphs in non-display source, this transforms it before Markdown parsing. For example,
[docs](https://example.com/①)is parsed with an href ending in/1., so the OSC 8 link navigates to the wrong resource, and fenced or inline code containing①is likewise displayed inaccurately. Apply the substitution only to rendered prose text nodes rather than to the complete Markdown source.Useful? React with 👍 / 👎.