fix(cli): use high-contrast software cursor - #5720
Conversation
|
@qwen-code /triage |
|
Thanks for the PR, @tt-a1i! Template looks good ✓ On direction: this fixes a real user-reported problem — the software cursor being semi-invisible in Alacritty with Solarized palettes (#5713). Terminal cursor visibility is squarely within the CLI's core UX, and the issue has clear reproduction evidence. Aligned. On approach: the scope is tight and focused. One new utility ( Moving on to code review and testing. 🔍 中文说明感谢贡献,@tt-a1i! 模板完整 ✓ 方向:这个 PR 修复了用户实际报告的问题 —— 在 Alacritty + Solarized 配色下软件光标几乎不可见(#5713)。终端光标可见性属于 CLI 核心 UX 范畴,issue 也有清晰的复现证据。方向对齐。 方案:范围紧凑、专注。一个新工具函数( 进入代码审查和测试 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe implementation is clean and well-focused. A quick summary:
No correctness bugs, no security issues, no regressions found. Test ResultsUnit tests (3/5 test files pass, 19/19 tests): The remaining 2 test files (BaseTextInput, InputPrompt) fail with a pre-existing tmux smoke test (installed qwen v0.18.5 — before): tmux smoke test (after — PR dev build): could not run — the dev build fails with a pre-existing The cursor rendering change is purely visual (ANSI escape codes), so tmux 中文说明代码审查实现干净、专注。简要总结:
未发现正确性 bug、安全问题或回归。 测试结果单元测试(3/5 测试文件通过,19/19 测试用例): 其余 2 个测试文件(BaseTextInput、InputPrompt)因 tmux 冒烟测试(已安装 qwen v0.18.5 —— before):正常运行。PR dev build(after)因预存的 光标渲染变更是纯视觉的(ANSI 转义码),tmux — Qwen Code · qwen3.7-max |
Final AssessmentThis is a straightforward, well-executed bugfix. The motivation is a real user-reported issue (#5713 — invisible cursor in Alacritty + Solarized), the implementation is the minimal change needed (one utility function, mechanical call-site updates), and the tests assert the right things (no reverse video, no foreground reset, correct luminance-based background selection). Looking back at my independent proposal from Stage 2a: I would have done the same thing — replace The only flag I raised — Verdict: Approve ✅ 中文说明最终评估这是一个简洁、执行良好的 bugfix。动机来自用户实际报告的问题(#5713 —— Alacritty + Solarized 下光标不可见),实现是所需的最小变更(一个工具函数,机械的调用点替换),测试断言了正确的行为(无反色视频、无前景色重置、正确的亮度背景选择)。 回顾 Stage 2a 的独立方案:我会做同样的事 —— 用亮度检测驱动的 唯一提到的问题 —— 结论:批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Real-TUI verification report (local, tmux)I built the real Verdict: the fix is correct and resolves #5713 — safe to merge. One non-blocking note on test quality at the end. 1. Decisive proof — real TTY (this is what actually validates the fix)Same keystrokes on both builds (type
Captured cursor cell (ESC shown as So the cursor genuinely switches from terminal-dependent reverse-video (the #5713 low-contrast root cause in Alacritty/Solarized) to an explicit, theme-aware high-contrast background. Truecolor survives through tmux; on the dark default theme (bg 2. Color-choice logic — function-level A/B (
|
| 构建 | 光标格 SGR | 反色 \e[7m |
显式背景 \e[48;2;212;212;212m |
|---|---|---|---|
PRE-FIX(chalk.inverse) |
\e[7m 反色 |
1 | 0 |
FIXED(renderSoftwareCursor) |
\e[48;2;212;212;212m(#D4D4D4) |
0 | 1 |
抓到的光标格(ESC 显示为 \e,已裁掉行尾空格):
PRE-FIX : …\e[48;2;30;30;46m CURSORZZZ \e[7m · \e[0m… ← 光标 = 反色空格(依赖终端)
FIXED : …\e[48;2;30;30;46m CURSORZZZ \e[48;2;212;212;212m · … ← 光标 = 显式 #D4D4D4 背景空格
可见光标确实从"依赖终端的反色视频"(#5713 在 Alacritty/Solarized 下低对比的根因)切换为"显式、跟随主题的高对比背景"。真彩透过 tmux 完整保留;在暗色默认主题(背景 #1E1E2E,亮度 < 128)下正确选了浅色光标 #D4D4D4。代码里的 (零宽空格)防裁剪保护也保留了。
2. 选色逻辑 —— 函数级 A/B(FORCE_COLOR=3)
直接 import 构建后的 software-cursor.js:
- 旧
chalk.inverse('X')→\e[7mX\e[27m(反色) - 新
renderSoftwareCursor('X')→\e[48;2;212;212;212mX\e[49m(显式背景)
getSoftwareCursorBackground() 的亮度分支逐例验证:暗背景 → 浅光标 #D4D4D4;亮背景 → 深光标 #3A3A3A;边界 #808080(亮度 == 128)→ 深;命名色走 INK 映射(white→深、black→浅);三位十六进制展开(#fff→深);未知 / 空 → 默认浅色。全部正确。
3. 变异测试 —— 新增单测对"样式切换"是空过的(不阻塞合并)
上面的决定性证据来自真实 TTY A/B,而不是单测,因为单测看不到颜色:
- 探针: vitest 环境里
chalk.level === 0,所以renderSoftwareCursor('x')和chalk.inverse('x')都返回裸"x"(完全没有 SGR)。 - Mutation A —— 把 5 个组件源码全部回退到 base(
chalk.inverse),保留 PR 的测试:BaseTextInput.test.tsx(5)、InputPrompt.test.tsx › Highlighting and Cursor Display(12)、shared/TextInput.test.tsx(6)、SettingInputPrompt.test.tsx(6) 依旧全部通过。 - Mutation B —— 把
renderSoftwareCursor改回chalk.inverse:software-cursor.test.ts仍 7/7 通过,包括 "uses an explicit background instead of reverse-video styling"(expect(rendered).not.toContain('�[7m'))。
原因:chalk.level === 0 时样式退化为纯文本,于是所有"用 renderSoftwareCursor(...) 构造期望串"的断言、以及 not.toContain('�[7m'),对两种实现都成立。只有那 4 个 getSoftwareCursorBackground(...) 断言是非空过的(测纯逻辑,与 chalk 无关)。FORCE_COLOR=1 只在 integration-tests/ 的 terminal-capture e2e 里设置 —— 这些单测里没有 —— 而本 PR 没有为光标新增任何 integration test。
可选建议(不阻塞合并): 想真正防住"回退到反色"的回归,可在样式测试里强制一个 color level —— 例如断言 new chalk.Instance({ level: 3 }) 的输出,或在测试里设 chalk.level = 3 —— 这样 renderSoftwareCursor 才会发出真实 SGR,not.toContain('�[7m') / toContain('[48;2;') 才有意义。
环境
真实 qwen 二进制由 PR head(70ef0af)经 npm ci && npm run build 构建;tmux 3.6a、真彩(terminal-overrides ',*:Tc')、隔离 HOME;macOS、Node v22。A/B 通过只把相关源码回退到 origin/main 并在同一 worktree 重建得到。
✅ Independent re-confirmation — raw app bytes + live light-theme flipComplements the thorough real-TTY / function-level / mutation report above — same verdict, safe to merge. Two things this run adds rather than repeats: 1. Captured the raw app output (not a screen reconstruction). Instead of
2. The theme-awareness flip, end-to-end in the live TUI. Rather than only computing Net: reverse-video → explicit, theme-aware background is confirmed at the real surface from a second capture method, and the dark/light flip is real end-to-end. Concur with LGTM. (No new concerns; I agree with the note above that the added vitest cases are color-blind under BASE = current main-line cursor code ( 🇨🇳 中文版✅ 独立复核 —— 原始 app 字节 + 实时浅色主题翻转作为上面那份「真实 TTY / 函数级 / 变异测试」报告的补充 —— 结论一致,可以合并。这一轮新增(而非重复)两点: 1. 抓的是 app 的原始输出(不是屏幕重建)。 我没用
2. 主题自适应翻转,在实时 TUI 里端到端验证。 我没有只在函数级算 结论:从第二种抓取方式确认了「反色 → 显式、跟随主题的背景」在真实界面上的切换,且暗/亮翻转端到端为真。同意 LGTM。(无新问题;也同意上面的提示 —— 新增的 vitest 用例在 |
| } | ||
|
|
||
| export function renderSoftwareCursor(text: string): string { | ||
| return chalk.bgHex(getSoftwareCursorBackground())(text || ' '); |
There was a problem hiding this comment.
[Critical — two regressions from chalk.inverse]
1. Missing foreground color: chalk.bgHex() sets only the background. On dark themes (the default), the terminal's text foreground is white while the cursor background becomes #D4D4D4 — a contrast ratio of ~1.1:1, making the cursor character invisible. The old chalk.inverse() swapped fg/bg automatically.
2. No truecolor fallback: On terminals without 24-bit color (chalk.level === 0 — TERM=dumb, some SSH/CI environments), chalk.bgHex() produces zero escape codes and the cursor disappears entirely. The codebase already has supportsTrueColor() in color-utils.ts.
Suggested fix addressing both issues:
| return chalk.bgHex(getSoftwareCursorBackground())(text || ' '); | |
| import { supportsTrueColor } from '../themes/color-utils.js'; | |
| const DARK_TEXT = '#1E1E1E'; | |
| const LIGHT_TEXT = '#F0F0F0'; | |
| export function renderSoftwareCursor(text: string): string { | |
| if (!supportsTrueColor()) { | |
| return chalk.inverse(text || ' '); | |
| } | |
| const bg = getSoftwareCursorBackground(); | |
| const fg = bg === LIGHT_CURSOR_BACKGROUND ? DARK_TEXT : LIGHT_TEXT; | |
| return chalk.bgHex(bg).hex(fg)(text || ' '); | |
| } |
— Claude 3.5 Sonnet via Qwen Code /review
What this PR does
Replaces the input software cursor's terminal-dependent reverse-video styling with a shared high-contrast cursor renderer. The renderer chooses a light or dark cursor background from the active theme background, so dark themes such as Solarized/Alacritty get a visible light cursor while light themes get a visible dark cursor.
The shared renderer is used by the main prompt, the base text input, the legacy shared text input, the sensitive setting prompt, and the settings dialog edit buffer. Existing line-end zero-width-space behavior is preserved so trailing cursor spaces are not trimmed by Ink.
Why it's needed
Some terminals and color palettes render
chalk.inverse()with very low contrast. In the reported Alacritty setup, the cursor becomes semi-invisible because reverse video depends on how the terminal swaps the current foreground and background colors.Using an explicit, theme-aware cursor background makes the software cursor visible without adding terminal-specific branches.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: the software cursor used
chalk.inverse(), which emits reverse-video styling and can be low-contrast in Alacritty palettes.After: cursor rendering uses
renderSoftwareCursor(), which emits an explicit theme-aware background and unit tests assert it does not use SGR 7 reverse video or reset surrounding foreground color.Local validation:
Tested on
Environment (optional)
Local validation used Node v26.3.0 and npm 11.16.0.
Risk & Scope
Linked Issues
Fixes #5713
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.
中文说明
What this PR does
这个 PR 将输入框的软件光标从依赖终端反色视频的样式,改为共享的高对比光标渲染器。渲染器会根据当前主题背景选择浅色或深色光标背景,因此像 Solarized/Alacritty 这类深色主题会得到可见的浅色光标,浅色主题会得到可见的深色光标。
共享渲染器用于主输入框、基础文本输入框、旧的共享文本输入框、敏感设置输入框,以及设置对话框的编辑缓冲区。已有的行尾零宽空格行为保持不变,避免 Ink 裁掉行尾光标空格。
Why it's needed
某些终端和配色方案会把
chalk.inverse()渲染成很低的对比度。在本 issue 报告的 Alacritty 配置里,光标会变得半透明/不明显,因为反色视频依赖终端如何交换当前前景色和背景色。使用明确的、跟随主题背景选择的光标背景,可以让软件光标保持可见,同时不需要加入终端特定分支。
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before:软件光标使用
chalk.inverse(),会发出反色视频样式,在 Alacritty 配色下可能低对比。After:光标渲染改为
renderSoftwareCursor(),它会发出明确的、跟随主题选择的背景色;单元测试断言它不会使用 SGR 7 反色视频,也不会重置外层前景色。本地验证:
Tested on
Environment (optional)
本地验证使用 Node v26.3.0 和 npm 11.16.0。
Risk & Scope
Linked Issues
Fixes #5713
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.