Skip to content

fix(ui): set terminal background via OSC 11 to match theme#2223

Closed
HUQIANTAO wants to merge 4 commits into
esengine:v1from
HUQIANTAO:fix/osc11-terminal-bg
Closed

fix(ui): set terminal background via OSC 11 to match theme#2223
HUQIANTAO wants to merge 4 commits into
esengine:v1from
HUQIANTAO:fix/osc11-terminal-bg

Conversation

@HUQIANTAO

Copy link
Copy Markdown
Contributor

Summary

  • Emit OSC 11 to set terminal default background to the theme's surface.bg, eliminating white/empty areas below Ink's content in iTerm and other terminals
  • Crash-safe restore via process.once("exit"), SIGINT, and SIGTERM handlers ensure OSC 111 restores the original background on any exit path
  • Belt-and-suspenders: finally block also calls restoreTerminalBg()

Test plan

  • Launch in iTerm2 with a dark theme — verify no white areas below rendered content
  • Ctrl-C during a long operation — verify terminal background is restored
  • Normal exit — verify terminal background is restored
  • SIGTERM (e.g. kill) — verify terminal background is restored

🤖 Generated with Claude Code

Emit OSC 11 (`ESC]11;#rrggbb BEL`) before render to set the terminal's
default background to the theme's surface.bg. This eliminates white/empty
areas below Ink's content in iTerm and other terminals that don't inherit
the theme background.

Crash-safe restore: process.once("exit"), SIGINT, and SIGTERM handlers
ensure OSC 111 is emitted to restore the original background on any exit
path. The finally block also calls restoreTerminalBg() as belt-and-suspenders.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@esengine

Copy link
Copy Markdown
Owner

Thanks for splitting this out and fixing the crash-restore — restoring on process.once("exit") plus SIGINT/SIGTERM is the right instinct and resolves the 'terminal left repainted after a kill' concern from #2221.

One interaction to check though: the SIGINT/SIGTERM handlers call process.exit(130/143), and useQuit (src/cli/ui/hooks/useQuit.ts) already registers a SIGINT handler that does async cleanup — flush the transcript, save the .cpuprofile — before it exits. Node runs all SIGINT listeners; if this one registers first and calls process.exit synchronously, it preempts useQuit's flush, so Ctrl+C could drop the transcript/profile. Two safer options:

  • Restore-only on signals: in the SIGINT/SIGTERM handlers just restoreTerminalBg() and don't call process.exit — let useQuit own the exit. The process.once("exit") handler will still fire when useQuit exits, so the bg is restored either way.
  • Or hook the restore into useQuit's existing exit path so there's a single, ordered teardown.

The exit handler alone already covers the normal/useQuit-driven exit; it's the signal handlers racing useQuit I'd want resolved. CI is green; just this ordering.

…cing useQuit

The terminal bg restore handlers called process.exit() directly, which
preempts useQuit's async cleanup (transcript flush + .cpuprofile save).
Now the handlers only restore the background color; useQuit owns the
exit path. The process.once('exit') listener still fires as a fallback.
@HUQIANTAO

Copy link
Copy Markdown
Contributor Author

Fixed: removed process.exit() from SIGINT/SIGTERM handlers. Now the handlers only call restoreTerminalBg() without preempting useQuit's async cleanup path. The process.once('exit') listener still fires as a fallback when useQuit eventually exits.

@esengine

Copy link
Copy Markdown
Owner

Thanks for splitting this out of the larger change and for handling the crash-restore path carefully. The OSC sequences are correct: OSC 11 (ESC ]11;<color>BEL) sets the background and OSC 111 (ESC ]111BEL) is the right reset counterpart. The isTTY guard plus the startsWith("#") color validation are good, and I appreciate that you addressed my earlier concern by dropping process.exit() from the SIGINT/SIGTERM handlers so they only restore the background and let useQuit own the async exit (transcript flush + .cpuprofile) — the process.once("exit") listener and the finally belt-and-suspenders both still cover the restore. Scope is tight (one file, +20/-0) and CI (build ubuntu/windows + CodeQL) is green.

Because this is a terminal visual change, I can't verify the actual result or the restore behavior from code or CI. Before I merge, could you attach:

  1. A short screen recording (or before/after screenshots) showing the empty area below Ink's content now matching the theme background in iTerm2 with a dark theme.
  2. The same showing the original terminal background fully restored on (a) normal /quit exit and (b) Ctrl-C mid-operation — ideally on a terminal whose default background differs from the Reasonix theme so the reset is actually visible. A quick check on one terminal beyond iTerm2 (e.g. tmux or VS Code's integrated terminal) would also be reassuring since OSC 11/111 passthrough varies.

Two small non-blocking items, take or leave:

  • Consider honoring NO_COLOR: right now the background is set even when NO_COLOR is set (it's restored on exit, but it still contradicts the user's no-color preference). Adding a process.env.NO_COLOR guard alongside the isTTY check would respect that.
  • A one-line comment noting that non-#hex theme backgrounds are intentionally skipped would help future theme authors understand the startsWith("#") guard.

Happy to approve once the recording confirms the visual and the clean restore on both exit paths.

HUQIANTAO added 2 commits May 29, 2026 17:42
- Add process.env.NO_COLOR guard alongside isTTY check
- Add comment explaining why non-#hex backgrounds are skipped

@esengine esengine left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OSC 11 is the right way to sync the xterm background with the theme. Tiny diff, CI is green.

@esengine esengine added the v1 Legacy TypeScript line (0.x) — v1 branch, maintenance only label May 31, 2026
@Bernardxu123

Copy link
Copy Markdown
Collaborator

🙏 真诚感谢您的贡献!

感谢您为 DeepSeek-Reasonix v1 分支提交的 PR。v1 目前已 完全停止维护,项目已全面迁移至 v2 (Go rewrite)。

由于 v1 已不再接受代码变更,此 PR 将被关闭。

我们非常欢迎您将这些修复贡献到 v2 分支! 请查看 v2 Issues 或基于 main-v2 创建新 PR。

再次感谢您的时间和精力!❤️

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

Labels

v1 Legacy TypeScript line (0.x) — v1 branch, maintenance only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants