Repository navigation
fix(tui): follow macOS system appearance for theme mode - #58678
LaurentTreguier wants to merge 2 commits into
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
9571cd2 to
cdfe4e1
Compare
cdfe4e1 to
57d3a0f
Compare
There was a problem hiding this comment.
I read the change closely but couldn't run it: it only does anything on macOS, and I had no macOS machine to reproduce #58679 or check the fix. The existing theme test passes. A type-check found no errors in the three changed files (the errors it did report are elsewhere and look unrelated).
Public API: unchanged. There are no new config keys, flags, events or environment variables.
The approach makes sense: on an appearance change it asks the terminal for its colours again first and only falls back to the OS setting, and it respects the mode lock. Things worth fixing:
- The helper process always runs. Every TUI session on macOS starts a
while (true)osascriptprocess. That happens even when the mode is pinned to dark or light, and even when the terminal already reports its own theme. It would be better to start it only while the mode isn't locked (and stop it when the user pins a mode). - The helper can be left running. It's only killed by the provider's cleanup. If OpenCode crashes or is killed with SIGKILL, the helper keeps running in the background. It only notices its parent is gone the next time it tries to report an appearance change.
- Errors in the fallbacks aren't caught. If
fs.watchthrows inside the exit callback, the error goes unhandled. At startup,systemAppearance()runs inside the main startuptryPromise, so ifBun.spawnthrows there, startup fails instead of falling back to"dark". Both are unlikely on a normal macOS install, but each should fall back to the current behaviour.
There are no tests for the new logic. A small test of watchSystemAppearance's first-read and change handling (with the spawned process stubbed) would help, since CI can't exercise any of this.
57d3a0f to
5b28bad
Compare
There was a problem hiding this comment.
I re-read the new commit but still couldn't run the fix: it only does anything on macOS, and I don't have a macOS machine to test it. The existing theme test passes, and a type-check found no errors in the changed files. The public API is unchanged.
All three earlier points are addressed:
- The watcher now runs only while the mode isn't pinned, and stops once the terminal reports theme changes itself.
- The helper's 5-second tick through the stderr pipe should make it exit within about 5 seconds if OpenCode dies.
- The spawn and file-watch fallbacks now catch their errors.
I left one inline question: could the first theme-change event come from a late answer to the startup colour query, rather than from a terminal that reports changes? If so, live switching would be turned off for that terminal.
5b28bad to
422131e
Compare
There was a problem hiding this comment.
I re-read the new commit. I still couldn't run the fix, because it only does anything on macOS. The existing theme test passes, and a type-check found no errors in the changed files. The public API is unchanged.
The earlier point is addressed: the OS watcher now stops only after a real ?997 notification from the terminal, so a late answer to the startup colour query no longer turns off live switching. Nothing new to raise.
Issue for this PR
Closes #58679
Type of change
What does this PR do?
Gives theme-mode resolution a macOS appearance fallback, in two parts.
At startup, when the renderer cannot detect the theme mode from OSC 10/11 responses, read the OS appearance via
defaults read -g AppleInterfaceStyle(key present = dark, absent = light).While running, watch for OS appearance changes: a long-lived JXA
osascripthelper observes theAppleInterfaceThemeChangedNotificationdistributed notification, with.GlobalPreferences.plistwrite watching as fallback when the helper cannot run. On a change the terminal's own colors are re-queried first viagetPalette; the OS appearance only applies when the terminal does not answer, so manually themed terminals stay authoritative. The theme-mode lock is respected. No behavior change on other platforms.How did you verify your code works?
Tested in Apple Terminal on macOS 27: startup detection in light and dark OS modes, live switching in both directions (sub-second), no override when the mode is pinned, and unchanged behavior paths on a non-reporting terminal.
Screenshots / recordings
Theme switches live; no visual layout change.
Current behaviour:
Enregistrement.de.l.ecran.2026-10-24.a.12.38.37.mp4
With the fixes:
Enregistrement.de.l.ecran.2026-10-24.a.12.39.14.mp4
Checklist