-
Notifications
You must be signed in to change notification settings - Fork 51
feat(ui): add new theme "auto" that automatically switches between da… #241
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 |
|---|---|---|
|
|
@@ -39,6 +39,26 @@ export function shouldUseMouseForApp({ | |
| return stdinIsTTY || hasControllingTerminal; | ||
| } | ||
|
|
||
| /** Detect the system theme mode as a fallback when terminal-based detection is unavailable. */ | ||
| export function detectSystemThemeMode( | ||
| deps: { spawnSync: typeof Bun.spawnSync } = { spawnSync: Bun.spawnSync }, | ||
| ): "light" | "dark" | undefined { | ||
| if (process.platform !== "darwin") { | ||
| return undefined; | ||
|
Author
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. I don't know why the pager/piped flow works on Linux but it somehow does lol |
||
| } | ||
|
|
||
| try { | ||
| // defaults read -g AppleInterfaceStyle returns "Dark" in dark mode. | ||
| // It exits with code 1 if the key doesn't exist (which means light mode). | ||
| const proc = deps.spawnSync(["defaults", "read", "-g", "AppleInterfaceStyle"]); | ||
|
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. I find this idea a bit tricky. Some people use macOS in a light theme but still want a dark theme for the terminal. To me, the terminal should be the main source of information to keep things clear. For instance, you could use CSI to notify the tool when the colour palette changes. OpenTUI support it via the theme mode.
Author
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. yeah, but then you could just not use the auto theme, no? The issue with the OpenTUI theme mode is that it does not work when Hunk is used as a pager because stdin is redirected 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. Fair enough. It is just odd that Hunk will not follow the terminal when I change the light/dark themes outside of the OS general settings. |
||
| const output = proc.stdout.toString().trim(); | ||
| return output === "Dark" ? "dark" : "light"; | ||
| } catch { | ||
| // On macOS, if the key is missing, it's light mode. | ||
| return "light"; | ||
| } | ||
| } | ||
|
|
||
| export interface ControllingTerminal { | ||
| stdin: tty.ReadStream; | ||
| close: () => void; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| export const HUNK_DIFF_THEME_NAMES = ["graphite", "midnight", "paper", "ember"] as const; | ||
| export const HUNK_DIFF_THEME_NAMES = ["auto", "graphite", "midnight", "paper", "ember"] as const; | ||
|
|
||
| export type HunkDiffThemeName = (typeof HUNK_DIFF_THEME_NAMES)[number]; |
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.
The three macOS detection tests each open with
if (!isDarwin) return, which causes them to silently pass on Linux CI without executing any assertions. BecausedetectSystemThemeModeaccepts an injectedspawnSync, the tests can actually run on any platform without needing a realdefaultsbinary. Only the final test ("returns undefined on non-macOS platforms") genuinely requires the real platform branch. The first three tests would be more reliable as unconditional tests that always inject the mock.Prompt To Fix With AI