From cd49015243b794cf51ca30e8d32f09d4d362925f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Nov 2024 13:42:48 -0800 Subject: [PATCH] App applies conditional state, supports theme setting The prior light/dark mode awareness work works on surface-level APIs. As a result, configurations used at the app-level (such as split divider colors, inactive split opacity, etc.) are not aware of the current theme configurations and default to the "light" theme. This commit adds APIs to specify app-level color scheme changes. This changes the configuration for the app and sets the default conditional state to use that new theme. This latter point makes it so that future surfaces use the correct theme on load rather than requiring some apprt event loop ticks. Some users have already reported a short "flicker" to load the correct theme, so this should help alleviate that. --- include/ghostty.h | 1 + macos/Sources/App/macOS/AppDelegate.swift | 20 ++++++++++ macos/Sources/Ghostty/Ghostty.App.swift | 9 +++++ src/App.zig | 48 ++++++++++++++++++++++- src/Surface.zig | 21 +++++++++- src/apprt/embedded.zig | 16 ++++++++ 6 files changed, 112 insertions(+), 3 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 681a4e9dc..d2e59b09f 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -675,6 +675,7 @@ void ghostty_app_open_config(ghostty_app_t); void ghostty_app_update_config(ghostty_app_t, ghostty_config_t); bool ghostty_app_needs_confirm_quit(ghostty_app_t); bool ghostty_app_has_global_keybinds(ghostty_app_t); +void ghostty_app_set_color_scheme(ghostty_app_t, ghostty_color_scheme_e); ghostty_surface_config_s ghostty_surface_config_new(); diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index faafa7205..ed257d9ec 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -95,6 +95,9 @@ class AppDelegate: NSObject, /// makes our logic very easy. private var isVisible: Bool = true + /// The observer for the app appearance. + private var appearanceObserver: NSKeyValueObservation? = nil + override init() { terminalManager = TerminalManager(ghostty) updaterController = SPUStandardUpdaterController( @@ -187,6 +190,23 @@ class AppDelegate: NSObject, ) ]) center.delegate = self + + // Observe our appearance so we can report the correct value to libghostty. + self.appearanceObserver = NSApplication.shared.observe( + \.effectiveAppearance, + options: [.new, .initial] + ) { _, change in + guard let appearance = change.newValue else { return } + guard let app = self.ghostty.app else { return } + let scheme: ghostty_color_scheme_e + if (appearance.isDark) { + scheme = GHOSTTY_COLOR_SCHEME_DARK + } else { + scheme = GHOSTTY_COLOR_SCHEME_LIGHT + } + + ghostty_app_set_color_scheme(app, scheme) + } } func applicationDidBecomeActive(_ notification: Notification) { diff --git a/macos/Sources/Ghostty/Ghostty.App.swift b/macos/Sources/Ghostty/Ghostty.App.swift index 52a535365..9056e692a 100644 --- a/macos/Sources/Ghostty/Ghostty.App.swift +++ b/macos/Sources/Ghostty/Ghostty.App.swift @@ -1212,6 +1212,7 @@ extension Ghostty { switch (target.tag) { case GHOSTTY_TARGET_APP: + // Notify the world that the app config changed NotificationCenter.default.post( name: .ghosttyConfigDidChange, object: nil, @@ -1219,6 +1220,14 @@ extension Ghostty { SwiftUI.Notification.Name.GhosttyConfigChangeKey: config, ] ) + + // We also REPLACE our app-level config when this happens. This lets + // all the various things that depend on this but are still theme specific + // such as split border color work. + guard let app_ud = ghostty_app_userdata(app) else { return } + let ghostty = Unmanaged.fromOpaque(app_ud).takeUnretainedValue() + ghostty.config = config + return case GHOSTTY_TARGET_SURFACE: diff --git a/src/App.zig b/src/App.zig index 57b30ada0..279c4e497 100644 --- a/src/App.zig +++ b/src/App.zig @@ -67,6 +67,11 @@ font_grid_set: font.SharedGridSet, last_notification_time: ?std.time.Instant = null, last_notification_digest: u64 = 0, +/// The conditional state of the configuration. See the equivalent field +/// in the Surface struct for more information. In this case, this applies +/// to the app-level config and as a default for new surfaces. +config_conditional_state: configpkg.ConditionalState, + /// Set to false once we've created at least one surface. This /// never goes true again. This can be used by surfaces to determine /// if they are the first surface. @@ -95,6 +100,7 @@ pub fn create( .mailbox = .{}, .quit = false, .font_grid_set = font_grid_set, + .config_conditional_state = .{}, }; errdefer app.surfaces.deinit(alloc); @@ -154,11 +160,24 @@ pub fn updateConfig(self: *App, rt_app: *apprt.App, config: *const Config) !void try surface.core_surface.handleMessage(.{ .change_config = config }); } + // Apply our conditional state. If we fail to apply the conditional state + // then we log and attempt to move forward with the old config. + // We only apply this to the app-level config because the surface + // config applies its own conditional state. + var applied_: ?configpkg.Config = config.changeConditionalState( + self.config_conditional_state, + ) catch |err| err: { + log.warn("failed to apply conditional state to config err={}", .{err}); + break :err null; + }; + defer if (applied_) |*c| c.deinit(); + const applied: *const configpkg.Config = if (applied_) |*c| c else config; + // Notify the apprt that the app has changed configuration. try rt_app.performAction( .app, .config_change, - .{ .config = config }, + .{ .config = applied }, ); } @@ -380,6 +399,33 @@ pub fn keyEvent( return true; } +/// Call to notify Ghostty that the color scheme for the app has changed. +/// "Color scheme" in this case refers to system themes such as "light/dark". +pub fn colorSchemeEvent( + self: *App, + rt_app: *apprt.App, + scheme: apprt.ColorScheme, +) !void { + const new_scheme: configpkg.ConditionalState.Theme = switch (scheme) { + .light => .light, + .dark => .dark, + }; + + // If our scheme didn't change, then we don't do anything. + if (self.config_conditional_state.theme == new_scheme) return; + + // Setup our conditional state which has the current color theme. + self.config_conditional_state.theme = new_scheme; + + // Request our configuration be reloaded because the new scheme may + // impact the colors of the app. + try rt_app.performAction( + .app, + .reload_config, + .{ .soft = true }, + ); +} + /// Perform a binding action. This only accepts actions that are scoped /// to the app. Callers can use performAllAction to perform any action /// and any non-app-scoped actions will be performed on all surfaces. diff --git a/src/Surface.zig b/src/Surface.zig index ee2fe1ac5..b2530936a 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -359,11 +359,25 @@ const DerivedConfig = struct { pub fn init( self: *Surface, alloc: Allocator, - config: *const configpkg.Config, + config_original: *const configpkg.Config, app: *App, rt_app: *apprt.runtime.App, rt_surface: *apprt.runtime.Surface, ) !void { + // Apply our conditional state. If we fail to apply the conditional state + // then we log and attempt to move forward with the old config. + var config_: ?configpkg.Config = config_original.changeConditionalState( + app.config_conditional_state, + ) catch |err| err: { + log.warn("failed to apply conditional state to config err={}", .{err}); + break :err null; + }; + defer if (config_) |*c| c.deinit(); + + // We want a config pointer for everything so we get that either + // based on our conditional state or the original config. + const config: *const configpkg.Config = if (config_) |*c| c else config_original; + // Get our configuration var derived_config = try DerivedConfig.init(alloc, config); errdefer derived_config.deinit(); @@ -481,7 +495,10 @@ pub fn init( .io_thr = undefined, .size = size, .config = derived_config, - .config_conditional_state = .{}, + + // Our conditional state is initialized to the app state. This + // lets us get the most likely correct color theme and so on. + .config_conditional_state = app.config_conditional_state, }; // The command we're going to execute diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 92137e6d7..ccafb9aa6 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -1357,6 +1357,22 @@ pub const CAPI = struct { return v.hasGlobalKeybinds(); } + /// Update the color scheme of the app. + export fn ghostty_app_set_color_scheme(v: *App, scheme_raw: c_int) void { + const scheme = std.meta.intToEnum(apprt.ColorScheme, scheme_raw) catch { + log.warn( + "invalid color scheme to ghostty_surface_set_color_scheme value={}", + .{scheme_raw}, + ); + return; + }; + + v.core_app.colorSchemeEvent(v, scheme) catch |err| { + log.err("error setting color scheme err={}", .{err}); + return; + }; + } + /// Returns initial surface options. export fn ghostty_surface_config_new() apprt.Surface.Options { return .{};