From 723cf49fdb8d5b88dd7463ca162369fcc2be1061 Mon Sep 17 00:00:00 2001 From: Guillaume Jay Date: Fri, 2 Oct 2026 08:06:39 +0200 Subject: [PATCH 1/6] feat(settings): named chrome themes matching the terminal schemes theme now accepts solarized-dark, solarized-light, gruvbox-dark and gruvbox-light alongside dark/light, mapping to iced's built-in themes so the chrome can match the terminal grid it frames. --- README.md | 5 +++-- crates/app/src/settings.rs | 24 +++++++++++++++++++++++- crates/mcp/src/lib.rs | 21 +++++++++++++++++++-- docs/settings.example.jsonc | 6 ++++-- docs/src/mcp/stdio.md | 2 +- docs/src/reference/settings.md | 7 +++++-- 6 files changed, 55 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 2235d7c..4bf4d88 100644 --- a/README.md +++ b/README.md @@ -127,8 +127,9 @@ you want and strip the comments (the real file is strict JSON). In short: - `shell` — program + args launched for each session (default: the platform login shell). -- `theme` — `"dark"` (default) or `"light"` GUI chrome; the terminal grid - keeps its own colours. +- `theme` — GUI chrome: `"dark"` (default), `"light"`, or a named preset + (`solarized-*`, `gruvbox-*`) matching the terminal schemes; the terminal + grid keeps its own colours. - `close` — per-action close confirmation (`tab`, `app`): always, only while a foreground process runs (default), or never. - `terminal` — base `font_size` (the zoom shortcuts step from it), grid diff --git a/crates/app/src/settings.rs b/crates/app/src/settings.rs index 4e5b094..c53de5f 100644 --- a/crates/app/src/settings.rs +++ b/crates/app/src/settings.rs @@ -361,14 +361,24 @@ impl TerminalSettings { } /// Which iced theme dresses the GUI chrome (sidebar, tab strip, buttons). +/// The named variants mirror the built-in `terminal.colors.scheme` presets, +/// so the chrome can match the grid it frames. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "kebab-case")] pub enum ThemeChoice { /// A dark chrome, matching the terminal's dark background. #[default] Dark, /// A light chrome. Light, + /// Solarized Dark chrome. + SolarizedDark, + /// Solarized Light chrome. + SolarizedLight, + /// Gruvbox Dark chrome. + GruvboxDark, + /// Gruvbox Light chrome. + GruvboxLight, } /// When a close that would terminate running session(s) asks for confirmation @@ -435,6 +445,10 @@ impl ThemeChoice { match self { ThemeChoice::Dark => iced::Theme::Dark, ThemeChoice::Light => iced::Theme::Light, + ThemeChoice::SolarizedDark => iced::Theme::SolarizedDark, + ThemeChoice::SolarizedLight => iced::Theme::SolarizedLight, + ThemeChoice::GruvboxDark => iced::Theme::GruvboxDark, + ThemeChoice::GruvboxLight => iced::Theme::GruvboxLight, } } } @@ -915,6 +929,14 @@ mod tests { } } + #[test] + fn named_themes_serialise_kebab_cased_like_the_terminal_schemes() { + let json = serde_json::to_string(&ThemeChoice::SolarizedLight).expect("serialise"); + assert_eq!(json, "\"solarized-light\""); + let back: ThemeChoice = serde_json::from_str(r#""gruvbox-dark""#).expect("deserialise"); + assert_eq!(back, ThemeChoice::GruvboxDark); + } + #[test] fn theme_round_trips_through_json_lowercased() { let json = serde_json::to_string(&ThemeChoice::Light).expect("serialise"); diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index c0608a0..99792b4 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -55,7 +55,14 @@ pub const OPTIONS: &[OptionSpec] = &[ pointer: "/theme", description: "GUI chrome theme (the terminal grid keeps its own colours).", kind: "enum", - choices: &["dark", "light"], + choices: &[ + "dark", + "light", + "solarized-dark", + "solarized-light", + "gruvbox-dark", + "gruvbox-light", + ], writable: true, }, OptionSpec { @@ -495,7 +502,17 @@ mod tests { let opts = resolve_options(&settings()); let theme = opts.iter().find(|o| o["id"] == "theme").expect("theme"); assert_eq!(theme["value"], json!("light")); - assert_eq!(theme["choices"], json!(["dark", "light"])); + assert_eq!( + theme["choices"], + json!([ + "dark", + "light", + "solarized-dark", + "solarized-light", + "gruvbox-dark", + "gruvbox-light" + ]) + ); let program = opts .iter() .find(|o| o["id"] == "shell.program") diff --git a/docs/settings.example.jsonc b/docs/settings.example.jsonc index cf65b73..47ac444 100644 --- a/docs/settings.example.jsonc +++ b/docs/settings.example.jsonc @@ -32,8 +32,10 @@ }, // ── Theme ──────────────────────────────────────────────────────────────── - // GUI chrome theme (sidebar, tab strip, buttons): "dark" (default) or - // "light". The terminal grid keeps its own colours — see `terminal.colors`. + // GUI chrome theme (sidebar, tab strip, buttons): "dark" (default), + // "light", "solarized-dark", "solarized-light", "gruvbox-dark" or + // "gruvbox-light". The terminal grid keeps its own colours — pair a named + // theme with the same `terminal.colors.scheme` for a matching chrome. "theme": "dark", // ── Close confirmation ────────────────────────────────────────────────── diff --git a/docs/src/mcp/stdio.md b/docs/src/mcp/stdio.md index 06fc06a..96b8a1d 100644 --- a/docs/src/mcp/stdio.md +++ b/docs/src/mcp/stdio.md @@ -35,7 +35,7 @@ Both speak the option **id** — a stable, dotted name: | id | Kind | Writable | Values | | --- | --- | --- | --- | -| `theme` | enum | yes | `dark`, `light` | +| `theme` | enum | yes | `dark`, `light`, `solarized-dark`, `solarized-light`, `gruvbox-dark`, `gruvbox-light` | | `shell.program` | string | **no** | unset means the platform default login shell | | `shell.args` | array | **no** | | | `terminal.colors.scheme` | enum | yes | `solarized-dark`, `solarized-light`, `gruvbox-dark`, `gruvbox-light` | diff --git a/docs/src/reference/settings.md b/docs/src/reference/settings.md index 34ad4b3..4ed3150 100644 --- a/docs/src/reference/settings.md +++ b/docs/src/reference/settings.md @@ -44,8 +44,11 @@ only when named explicitly. See [Status and attention](../workspace/status.md). ### `theme` -`"dark"` (default) or `"light"`. GUI chrome only — sidebar, tab strip, buttons. -The terminal grid keeps its own colours. +`"dark"` (default), `"light"`, or one of the named presets +`"solarized-dark"`, `"solarized-light"`, `"gruvbox-dark"`, `"gruvbox-light"`. +GUI chrome only — sidebar, tab strip, buttons. The terminal grid keeps its own +colours: pick the matching `terminal.colors.scheme` for a chrome that blends +with it. ### `close` From 90474582e63978c3d4e78f7aeb4fedcf6427f9c5 Mon Sep 17 00:00:00 2001 From: Guillaume Jay Date: Fri, 2 Oct 2026 08:38:51 +0200 Subject: [PATCH 2/6] feat(settings): an in-app settings panel that applies the appearance live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A gear in the sidebar header, or mod+, (open-settings), opens a modal panel with the chrome theme and the terminal scheme. A pick applies at once — the chrome on the next frame, every running terminal through a palette the PTY manager now shares with its terminal threads and repaints on demand — and persists only its own key to settings.json, leaving the rest of the file (and any key this build does not understand) as written. Light themes, made to read properly: - the theme key degrades alone on an unknown value instead of resetting the whole file; - status dots darken on a light surface, held to the WCAG 3:1 floor for UI components by a test over every light theme; - the unfocused-window scrim fades toward the grid's own background rather than black, which turned a light grid grey. The panel is a rung of the keyboard ladder, so escape leaves it and the overlay sweep covers it. --- .roadmap/features/f-settings-panel.md | 20 +++ .roadmap/features/f-settings.md | 4 +- README.md | 4 +- ROADMAP.md | 22 ++- crates/app/src/json_store.rs | 28 ++++ crates/app/src/main.rs | 6 +- crates/app/src/settings.rs | 170 ++++++++++++++++++++ crates/app/src/shell.rs | 150 ++++++++++++++++- crates/app/src/shell/appearance.rs | 144 +++++++++++++++++ crates/app/src/shell/routing.rs | 20 ++- crates/app/src/shell/terminal/canvas.rs | 8 +- crates/app/src/shell/view/mod.rs | 14 +- crates/app/src/shell/view/settings_panel.rs | 67 ++++++++ crates/app/src/shell/view/sidebar.rs | 27 +++- crates/app/src/shell/view/style.rs | 94 ++++++++++- crates/app/src/shell/view/tabs.rs | 6 +- crates/app/src/strings.rs | 12 ++ crates/core/src/keymap.rs | 7 + crates/pty/src/grid.rs | 19 +++ crates/pty/src/manager.rs | 68 +++++++- crates/pty/src/session.rs | 48 ++++-- docs/src/SUMMARY.md | 1 + docs/src/reference/keyboard.md | 1 + docs/src/reference/settings.md | 9 +- docs/src/workspace/settings-panel.md | 40 +++++ docs/src/workspace/sidebar.md | 2 +- 26 files changed, 934 insertions(+), 57 deletions(-) create mode 100644 .roadmap/features/f-settings-panel.md create mode 100644 crates/app/src/shell/appearance.rs create mode 100644 crates/app/src/shell/view/settings_panel.rs create mode 100644 docs/src/workspace/settings-panel.md diff --git a/.roadmap/features/f-settings-panel.md b/.roadmap/features/f-settings-panel.md new file mode 100644 index 0000000..a1ad0af --- /dev/null +++ b/.roadmap/features/f-settings-panel.md @@ -0,0 +1,20 @@ ++++ +id = "F-settings-panel" +type = "feature" +area = ["workspace"] +status = "done" +target = ["Should"] ++++ + +An in-app settings panel — appearance first, applied live and saved on pick. + +The ⚙ in the sidebar header, or `mod+,` (`open-settings`), opens a modal +panel over the workspace: the chrome theme (`dark`, `light`, and the four +presets named after the terminal schemes) and the terminal scheme. A pick +applies at once — the chrome on the next frame, every running terminal through +a shared palette the PTY manager repaints — and writes only its own key to +`settings.json`, leaving the rest of the file as written. This is the live +reload [F-terminal-palette](#f-terminal-palette) deferred, and the in-app +panel [F-settings](#f-settings) promised. A running Claude session keeps the +light/dark look it chose from its startup colour query until it restarts. +Other settings stay file-only for now. diff --git a/.roadmap/features/f-settings.md b/.roadmap/features/f-settings.md index afe34f9..5268c9a 100644 --- a/.roadmap/features/f-settings.md +++ b/.roadmap/features/f-settings.md @@ -12,5 +12,5 @@ Shell select, theme, window prefs (M3): `~/.termherd/settings.json` (serde, defaults on missing/corrupt) carries a shell profile (program + args), injected into the `PtyManager` so each session launches the chosen shell, and a GUI theme (dark/light) wired to the iced chrome. (thin) Window bounds keep their -own `window.json` (FR12). File-based for now; an in-app settings panel is the -full version later +own `window.json` (FR12). The appearance is now also set live from +[F-settings-panel](#f-settings-panel); the rest stays file-based diff --git a/README.md b/README.md index 4bf4d88..05b2904 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,9 @@ Optional user settings live in `~/.termherd/settings.json` (on Windows, is missing or invalid, TermHerd falls back to defaults rather than refusing to start — out-of-range values clamp, and a single bad value (a typo'd colour, an unknown key action) degrades alone with a logged warning instead -of resetting the rest of the file. There is no in-app settings panel yet — +of resetting the rest of the file. The appearance (chrome theme and +terminal colours) can also be changed live from the settings panel — ⚙ in +the sidebar, or `Cmd`/`Ctrl`+`,` — which saves it here; everything else: edit the file and restart. The annotated reference template is diff --git a/ROADMAP.md b/ROADMAP.md index 523e65b..85d6a20 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -73,6 +73,7 @@ issues #18–#29. | [F-terminal-split](#f-terminal-split) | feature | workspace, keymap | ☐ | Split panes with directional focus; drag-resize is what remains. | | [F-close-on-exit](#f-close-on-exit) | feature | terminal, workspace | ✅ | A pane whose shell exits cleanly closes itself; a failed one stays readable. | | [F-repo-add](#f-repo-add) | feature | sidebar, sessions | ✅ | Declare a repository in the sidebar, before it has any session. | +| [F-settings-panel](#f-settings-panel) | feature | workspace | ✅ | An in-app settings panel — appearance first, applied live and saved on pick. | | [F-terminal-cwd](#f-terminal-cwd) | feature | terminal, mcp, sessions | ✅ | The shell announces the directory it is in, so a session's `cwd` follows a `cd`. | ## Could @@ -409,8 +410,8 @@ Shell select, theme, window prefs (M3): `~/.termherd/settings.json` (serde, defaults on missing/corrupt) carries a shell profile (program + args), injected into the `PtyManager` so each session launches the chosen shell, and a GUI theme (dark/light) wired to the iced chrome. (thin) Window bounds keep their -own `window.json` (FR12). File-based for now; an in-app settings panel is the -full version later +own `window.json` (FR12). The appearance is now also set live from +[F-settings-panel](#f-settings-panel); the rest stays file-based @@ -672,6 +673,23 @@ read as one that never had any and was pinned to the top as freshly added. Adjacent: [F-repo-view](#f-repo-view) (#148) takes the other end — this is about a repository *existing* in the sidebar, that one about *viewing* it. + + +### F-settings-panel + +An in-app settings panel — appearance first, applied live and saved on pick. + +The ⚙ in the sidebar header, or `mod+,` (`open-settings`), opens a modal +panel over the workspace: the chrome theme (`dark`, `light`, and the four +presets named after the terminal schemes) and the terminal scheme. A pick +applies at once — the chrome on the next frame, every running terminal through +a shared palette the PTY manager repaints — and writes only its own key to +`settings.json`, leaving the rest of the file as written. This is the live +reload [F-terminal-palette](#f-terminal-palette) deferred, and the in-app +panel [F-settings](#f-settings) promised. A running Claude session keeps the +light/dark look it chose from its startup colour query until it restarts. +Other settings stay file-only for now. + ### F-terminal-cwd diff --git a/crates/app/src/json_store.rs b/crates/app/src/json_store.rs index fb390c9..aaca149 100644 --- a/crates/app/src/json_store.rs +++ b/crates/app/src/json_store.rs @@ -7,6 +7,7 @@ use serde::Serialize; use serde::de::DeserializeOwned; +use serde_json::{Map, Value}; use std::path::PathBuf; use tracing::warn; @@ -48,6 +49,33 @@ pub fn save_json(file: &str, value: &T) { } } +/// Rewrite some keys of the JSON object in `~/.termherd/`, leaving every +/// other key as found — including ones this build does not understand. A +/// missing file starts from an empty object; a file that does not parse as an +/// object is left alone (with a warning), since rewriting it would replace the +/// user's whole configuration with the few keys being set. +pub fn update_json(file: &str, edit: impl FnOnce(&mut Map)) { + let Some(path) = config_path(file) else { + return; + }; + let mut root = match std::fs::read_to_string(&path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Map::new(), + Err(e) => { + warn!(error = %e, path = %path.display(), "could not read config file; not updating it"); + return; + } + Ok(raw) => match serde_json::from_str(&raw) { + Ok(Value::Object(root)) => root, + _ => { + warn!(path = %path.display(), "config file is not a JSON object; not updating it"); + return; + } + }, + }; + edit(&mut root); + save_json(file, &root); +} + /// `~/.termherd/` — the app data dir from the PRD (§7). fn config_path(file: &str) -> Option { Some(crate::paths::termherd_dir()?.join(file)) diff --git a/crates/app/src/main.rs b/crates/app/src/main.rs index c670021..7c380eb 100644 --- a/crates/app/src/main.rs +++ b/crates/app/src/main.rs @@ -28,7 +28,7 @@ mod window_geometry; use std::sync::Arc; -use termherd_core::ports::{PathResolver, ProjectScanner, PtyHost, ScanError}; +use termherd_core::ports::{PathResolver, ProjectScanner, ScanError}; use termherd_pty::{EventSink, PtyEvent, PtyManager, Shell}; use termherd_scan::FsScanner; use tracing::{info, warn}; @@ -68,12 +68,12 @@ fn main() -> iced::Result { // PTY output flows from the reader threads through this channel into the // iced subscription (M2). The manager is built here and injected as a - // `dyn PtyHost` — no global state (Q4). + // `dyn PtyHost` (and as the palette's live repaint) — no global state (Q4). let (tx, pty_rx) = iced::futures::channel::mpsc::unbounded::(); let sink: EventSink = Arc::new(move |event| { let _ = tx.unbounded_send(event); }); - let pty: Arc = Arc::new(PtyManager::new(sink, shell, settings.palette())); + let pty = Arc::new(PtyManager::new(sink, shell, settings.palette())); // Async transport substrate (composition root only): a tokio runtime to host // future transport tasks, and the bridge channel that carries their requests diff --git a/crates/app/src/settings.rs b/crates/app/src/settings.rs index c53de5f..3467efd 100644 --- a/crates/app/src/settings.rs +++ b/crates/app/src/settings.rs @@ -21,6 +21,7 @@ pub struct Settings { /// shell. pub shell: Option, /// GUI chrome theme (the terminal grid keeps its own colours). + #[serde(deserialize_with = "theme_or_default")] pub theme: ThemeChoice, /// Keyboard overrides: action name (kebab-case) → one chord or a list of /// chords. Each entry replaces that action's platform default (FR9). Same @@ -237,6 +238,20 @@ where })) } +/// Parse the chrome theme, degrading an unknown name to the default (with a +/// warning) rather than failing serde for the whole file — a typo, or a theme a +/// newer build added, costs the chrome and nothing else. +fn theme_or_default<'de, D>(de: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let value = serde_json::Value::deserialize(de)?; + Ok(ThemeChoice::deserialize(&value).unwrap_or_else(|_| { + warn!(%value, "unknown theme; using the default"); + ThemeChoice::default() + })) +} + /// Which mouse gestures reach the clipboard — the classic terminal /// conventions, both off unless asked for. Named once and carried whole, so /// the settings file, the shell and the canvas cannot disagree about which @@ -381,6 +396,14 @@ pub enum ThemeChoice { GruvboxLight, } +/// The name a person reads — iced's own, so the picker and the theme it +/// applies cannot disagree. +impl std::fmt::Display for ThemeChoice { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.to_iced().fmt(f) + } +} + /// When a close that would terminate running session(s) asks for confirmation /// first. Shared vocabulary for both the tab close and the app quit; each gets /// its own policy via [`CloseSettings`]. @@ -439,6 +462,16 @@ impl Default for CloseSettings { } impl ThemeChoice { + /// Every choice, in the order the settings panel lists them. + pub const ALL: [Self; 6] = [ + Self::Dark, + Self::Light, + Self::SolarizedDark, + Self::SolarizedLight, + Self::GruvboxDark, + Self::GruvboxLight, + ]; + /// The iced theme this choice maps to. #[must_use] pub fn to_iced(self) -> iced::Theme { @@ -539,6 +572,59 @@ impl Settings { } } +/// One appearance key the settings panel changed. Each pick persists only its +/// own key, so a value another writer put in the file meanwhile — an MCP +/// `set_option`, a hand edit — is not overwritten with a stale copy. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AppearanceChange { + /// The GUI chrome theme. + Theme(ThemeChoice), + /// The terminal scheme; `None` clears it back to the built-in palette. + Scheme(Option), +} + +/// Persist one appearance change to `settings.json`, keeping the rest of the +/// file — explicit colour overrides included — as the user wrote it. +pub fn save_appearance(change: &AppearanceChange) { + crate::json_store::update_json(FILE, |root| apply_appearance(root, change)); +} + +/// The pure half of [`save_appearance`]. +fn apply_appearance( + root: &mut serde_json::Map, + change: &AppearanceChange, +) { + use serde_json::{Map, Value}; + let scheme = match change { + AppearanceChange::Theme(theme) => { + if let Ok(theme) = serde_json::to_value(theme) { + root.insert("theme".into(), theme); + } + return; + } + AppearanceChange::Scheme(scheme) => scheme, + }; + // Descend into `terminal.colors`, replacing a level that is not an object: + // it could not have parsed as one, so nothing readable is lost. + let mut level = root; + for key in ["terminal", "colors"] { + let entry = level + .entry(key) + .or_insert_with(|| Value::Object(Map::new())); + if !entry.is_object() { + *entry = Value::Object(Map::new()); + } + let Value::Object(next) = entry else { + return; + }; + level = next; + } + match scheme { + Some(name) => level.insert("scheme".into(), Value::String(name.clone())), + None => level.remove("scheme"), + }; +} + /// `~/.termherd/settings.json` — the app data dir from the PRD (§7). const FILE: &str = "settings.json"; @@ -929,6 +1015,90 @@ mod tests { } } + #[test] + fn saving_the_appearance_keeps_every_other_key() { + let mut root = serde_json::json!({ + "theme": "dark", + "shell": { "program": "pwsh" }, + "terminal": { "font_size": 16, "colors": { "scheme": "gruvbox-dark", "cursor": "#ff0000" } }, + "future-key": [1, 2] + }); + let map = root.as_object_mut().expect("object"); + apply_appearance(map, &AppearanceChange::Theme(ThemeChoice::SolarizedLight)); + apply_appearance( + map, + &AppearanceChange::Scheme(Some("solarized-light".to_string())), + ); + assert_eq!( + root, + serde_json::json!({ + "theme": "solarized-light", + "shell": { "program": "pwsh" }, + "terminal": { "font_size": 16, "colors": { "scheme": "solarized-light", "cursor": "#ff0000" } }, + "future-key": [1, 2] + }) + ); + } + + #[test] + fn a_theme_change_leaves_the_scheme_alone_and_the_reverse() { + let mut root = serde_json::json!({ + "theme": "light", + "terminal": { "colors": { "scheme": "gruvbox-dark" } } + }); + let map = root.as_object_mut().expect("object"); + apply_appearance(map, &AppearanceChange::Theme(ThemeChoice::Dark)); + assert_eq!(map["terminal"]["colors"]["scheme"], "gruvbox-dark"); + apply_appearance(map, &AppearanceChange::Scheme(None)); + assert_eq!(map["theme"], "dark"); + assert_eq!(map["terminal"]["colors"], serde_json::json!({})); + } + + #[test] + fn a_scheme_reaches_a_file_whose_terminal_block_was_not_an_object() { + let mut root = serde_json::Map::new(); + root.insert("terminal".into(), serde_json::json!("not an object")); + apply_appearance( + &mut root, + &AppearanceChange::Scheme(Some("gruvbox-light".to_string())), + ); + assert_eq!( + serde_json::Value::Object(root), + serde_json::json!({ "terminal": { "colors": { "scheme": "gruvbox-light" } } }) + ); + } + + #[test] + fn the_saved_theme_reads_back_as_what_was_picked() { + for theme in ThemeChoice::ALL { + let mut root = serde_json::Map::new(); + apply_appearance(&mut root, &AppearanceChange::Theme(theme)); + let s: Settings = + serde_json::from_value(serde_json::Value::Object(root)).expect("valid settings"); + assert_eq!(s.theme, theme); + } + } + + #[test] + fn an_unknown_theme_degrades_alone() { + for bad in [r#""banana""#, "3", "null"] { + let json = format!( + r#"{{ "theme": {bad}, "terminal": {{ "colors": {{ "scheme": "solarized-light" }} }} }}"# + ); + let s: Settings = serde_json::from_str(&json).expect("bad theme keeps the file"); + assert_eq!( + s.theme, + ThemeChoice::Dark, + "{bad} falls back to the default" + ); + assert_eq!( + s.terminal.colors.scheme.as_deref(), + Some("solarized-light"), + "the rest of the file survives" + ); + } + } + #[test] fn named_themes_serialise_kebab_cased_like_the_terminal_schemes() { let json = serde_json::to_string(&ThemeChoice::SolarizedLight).expect("serialise"); diff --git a/crates/app/src/shell.rs b/crates/app/src/shell.rs index fd42dcf..6b8ff59 100644 --- a/crates/app/src/shell.rs +++ b/crates/app/src/shell.rs @@ -31,9 +31,10 @@ use termherd_pty::{PtyEvent, Screen}; use crate::docs::DocEntry; use crate::record_config::RecordConfig; -use crate::settings::{ClipboardGestures, CloseSettings, ThemeChoice}; +use crate::settings::{ClipboardGestures, CloseSettings, ColorSettings, ThemeChoice}; use crate::window_config::WindowConfig; +mod appearance; pub(crate) mod bridge; mod docs; mod effects; @@ -96,6 +97,9 @@ fn home_dir() -> String { /// Bundled so the composition root passes one value, not a long argument list. pub struct Startup { pub theme: ThemeChoice, + /// The terminal colour settings, kept whole so a scheme picked in the + /// settings panel still honours the user's explicit overrides. + pub colors: ColorSettings, pub keymap: Keymap, pub metadata: Overlay, /// Folded project paths restored from disk. @@ -129,6 +133,7 @@ impl Startup { let record = settings.record_config(); Self { theme: settings.theme, + colors: settings.terminal.colors.clone(), keymap: settings.keymap(), metadata, collapsed, @@ -152,7 +157,7 @@ pub fn run( scanner: Arc, watch_root: Option, path_resolver: Arc, - pty: Arc, + pty: Arc, pty_rx: UnboundedReceiver, live_bridge: LiveBridge, startup: Startup, @@ -177,10 +182,12 @@ pub fn run( path_resolver: path_resolver.clone(), pty: pty.clone(), pty_output: pty_output.clone(), + appearance: Arc::new(appearance::LiveAppearance(pty.clone())), }, live_bridge.clone(), Startup { theme: startup.theme, + colors: startup.colors.clone(), keymap: startup.keymap.clone(), metadata: startup.metadata.clone(), collapsed: startup.collapsed.clone(), @@ -277,8 +284,13 @@ struct Shell { waiters: Vec, /// Current keyboard target. focus: Focus, - /// GUI chrome theme (FR10). - theme: Theme, + /// The appearance the settings panel edits: the chrome theme, read every + /// frame by [`Shell::theme`], and the terminal colours. + appearance: appearance::Appearance, + /// Whether the settings panel is open. + settings_open: bool, + /// See [`Ports::appearance`]. + appearance_sink: Arc, /// Configurable shortcut bindings (FR9). keymap: Keymap, /// In-progress inline rename: `(session id, edit buffer)` (F-session-metadata). @@ -525,6 +537,14 @@ enum Message { ToggleCollapsed(String), /// Collapse or restore the whole session-browser sidebar. ToggleSidebar, + /// Open the settings panel, or close it when open. + ToggleSettings, + /// Close the settings panel (its close button, the scrim, Escape). + CloseSettings, + /// Apply and persist a GUI chrome theme picked in the settings panel. + PickTheme(ThemeChoice), + /// Apply and persist a terminal scheme picked in the settings panel. + PickScheme(appearance::SchemeChoice), /// Begin renaming a session inline, seeded with its current title. StartRename { session: String, @@ -674,6 +694,9 @@ pub(crate) struct Ports { pub(crate) path_resolver: Arc, pub(crate) pty: Arc, pub(crate) pty_output: PtyOutput, + /// Where the settings panel's picks go: the live terminal palette and + /// `settings.json`. + pub(crate) appearance: Arc, } impl Shell { @@ -684,6 +707,7 @@ impl Shell { path_resolver, pty, pty_output, + appearance: appearance_sink, } = ports; let LiveBridge { requests: bridge_requests, @@ -714,7 +738,12 @@ impl Shell { screens: HashMap::new(), waiters: Vec::new(), focus: Focus::Search, - theme: startup.theme.to_iced(), + appearance: appearance::Appearance { + theme: startup.theme, + colors: startup.colors, + }, + settings_open: false, + appearance_sink, keymap: startup.keymap, renaming: None, tab_rename: None, @@ -740,7 +769,7 @@ impl Shell { /// The GUI chrome theme (FR10); the terminal grid keeps its own colours. fn theme(&self) -> Theme { - self.theme.clone() + self.appearance.theme.to_iced() } /// Run one scan off the UI thread (FR2) and feed the result back. At most @@ -1161,6 +1190,22 @@ impl Shell { self.perform(effects) } Message::ToggleSidebar => self.toggle_sidebar(), + Message::ToggleSettings => { + self.settings_open = !self.settings_open; + Task::none() + } + Message::CloseSettings => { + self.settings_open = false; + Task::none() + } + Message::PickTheme(theme) => { + self.pick_theme(theme); + Task::none() + } + Message::PickScheme(scheme) => { + self.pick_scheme(scheme); + Task::none() + } Message::StartRename { session, current } => { self.renaming = Some((session, current)); operate(focusable::focus(rename_id())) @@ -1530,6 +1575,7 @@ mod key_routing { fn test_startup() -> Startup { Startup { theme: ThemeChoice::default(), + colors: ColorSettings::default(), keymap: Keymap::defaults(), metadata: Overlay::default(), collapsed: HashSet::new(), @@ -1578,6 +1624,7 @@ mod key_routing { path_resolver: Arc::new(termherd_scan::FsPathResolver::new()), pty, pty_output: PtyOutput::new(rx), + appearance: Arc::new(()), } } @@ -3758,6 +3805,94 @@ mod key_routing { /// /// The `match` is what makes the sweep above honest: a new `KeyboardOwner` /// variant fails to compile here rather than quietly escaping it. + /// Records what the settings panel sent beyond the shell. + #[derive(Default)] + struct RecordingAppearance { + repainted: std::sync::Mutex>, + persisted: std::sync::Mutex>, + } + + impl appearance::AppearanceSink for RecordingAppearance { + fn repaint(&self, palette: termherd_pty::Palette) { + self.repainted.lock().expect("lock").push(palette); + } + + fn persist(&self, change: &crate::settings::AppearanceChange) { + self.persisted.lock().expect("lock").push(change.clone()); + } + } + + fn shell_with_appearance(colors: ColorSettings) -> (Shell, Arc) { + let sink = Arc::new(RecordingAppearance::default()); + let (_tx, rx) = iced::futures::channel::mpsc::unbounded::(); + let mut ports = test_ports(Arc::new(RecordingPty::default()), rx); + ports.appearance = sink.clone(); + let startup = Startup { + colors, + ..test_startup() + }; + let shell = Shell::new(WindowConfig::default(), ports, test_live_bridge(), startup); + (shell, sink) + } + + #[test] + fn the_settings_chord_opens_the_panel_and_escape_closes_it() { + let (mut shell, _pty) = shell_with_terminal(); + let chord = press(Key::Character(",".into()), Modifiers::COMMAND, Some(",")); + let (verdict, _) = shell.on_key(chord); + assert_eq!( + verdict, + routing::KeyVerdict::Ran("open-settings".to_string()) + ); + assert!(shell.settings_open); + + let _ = shell.on_key(press(Key::Named(Named::Escape), Modifiers::default(), None)); + assert!(!shell.settings_open); + } + + #[test] + fn picking_a_theme_restyles_the_chrome_and_saves_it_without_recolouring() { + let (mut shell, sink) = shell_with_appearance(ColorSettings::default()); + let _ = shell.update(Message::PickTheme(ThemeChoice::SolarizedLight)); + assert_eq!(shell.theme(), iced::Theme::SolarizedLight); + assert_eq!( + *sink.persisted.lock().expect("lock"), + [crate::settings::AppearanceChange::Theme( + ThemeChoice::SolarizedLight + )] + ); + assert!(sink.repainted.lock().expect("lock").is_empty()); + } + + #[test] + fn picking_a_scheme_recolours_with_the_overrides_kept_and_saves_it() { + // An explicit cursor override must survive a scheme picked live, as it + // does one read from the file at startup. + let colors: ColorSettings = + serde_json::from_str(r##"{ "cursor": "#ff0000" }"##).expect("valid colours"); + let (mut shell, sink) = shell_with_appearance(colors); + let _ = shell.update(Message::PickScheme(appearance::SchemeChoice(Some( + "gruvbox-light", + )))); + + let expected = termherd_pty::Palette { + cursor: [0xff, 0, 0], + ..termherd_pty::Palette::named("gruvbox-light").expect("known scheme") + }; + assert_eq!(*sink.repainted.lock().expect("lock"), [expected]); + assert_eq!( + *sink.persisted.lock().expect("lock"), + [crate::settings::AppearanceChange::Scheme(Some( + "gruvbox-light".to_string() + ))] + ); + assert_eq!( + shell.config.terminal_scheme.as_deref(), + Some("gruvbox-light"), + "the snapshot reports the scheme in force, not the startup one" + ); + } + fn arm_overlay(shell: &mut Shell, owner: KeyboardOwner) { match owner { KeyboardOwner::TabRename => { @@ -3773,6 +3908,9 @@ mod key_routing { KeyboardOwner::Quit => shell.closing_window = Some(window::Id::unique()), KeyboardOwner::TabClose(index) => shell.closing = Some(index), KeyboardOwner::Archive => shell.archiving = Some("sess".to_string()), + KeyboardOwner::Settings => { + let _ = shell.update(Message::ToggleSettings); + } KeyboardOwner::Doc => { let _ = shell.update(Message::DocLoaded { label: "CLAUDE.md".to_string(), diff --git a/crates/app/src/shell/appearance.rs b/crates/app/src/shell/appearance.rs new file mode 100644 index 0000000..7c73b49 --- /dev/null +++ b/crates/app/src/shell/appearance.rs @@ -0,0 +1,144 @@ +//! The settings panel's appearance controls: the chrome theme and the +//! terminal scheme, each applied the moment it is picked and then persisted. +//! The chrome follows on the next frame, since [`Shell::theme`] reads it every +//! frame; the terminals and the file follow through [`AppearanceSink`]. + +use std::fmt; +use std::sync::Arc; + +use termherd_pty::{Palette, PtyManager}; + +use super::Shell; +use crate::settings::{AppearanceChange, ColorSettings, ThemeChoice, save_appearance}; + +/// Where a picked appearance goes beyond the shell's own state. A port of its +/// own rather than a [`termherd_core::ports::PtyHost`] method: a palette is +/// RGB, which `core` never sees. Injected so a shell under test neither +/// recolours real terminals nor rewrites the user's `settings.json`. +pub(crate) trait AppearanceSink: Send + Sync { + /// Recolour every terminal, running and future. + fn repaint(&self, palette: Palette); + /// Persist one picked key. + fn persist(&self, change: &AppearanceChange); +} + +/// The real sink: the PTY manager's live palette and `settings.json`. +pub(crate) struct LiveAppearance(pub(crate) Arc); + +impl AppearanceSink for LiveAppearance { + fn repaint(&self, palette: Palette) { + self.0.set_palette(palette); + } + + fn persist(&self, change: &AppearanceChange) { + save_appearance(change); + } +} + +/// Goes nowhere — the double for a shell whose test does not look. +impl AppearanceSink for () { + fn repaint(&self, _palette: Palette) {} + fn persist(&self, _change: &AppearanceChange) {} +} + +/// What the settings panel edits. +pub(super) struct Appearance { + pub(super) theme: ThemeChoice, + /// Kept whole, so a picked scheme still honours the user's explicit + /// colour overrides from `settings.json`. + pub(super) colors: ColorSettings, +} + +/// One entry of the terminal-scheme picker: a built-in scheme by name, or +/// `None` for the default palette. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct SchemeChoice(pub(crate) Option<&'static str>); + +impl SchemeChoice { + /// The default palette, then every named scheme. + pub(super) fn all() -> Vec { + std::iter::once(Self(None)) + .chain(Palette::SCHEMES.into_iter().map(|name| Self(Some(name)))) + .collect() + } + + /// The entry matching a configured scheme name. `None` for a name no + /// built-in answers to, so the picker shows no selection rather than a + /// wrong one. + pub(super) fn of(scheme: Option<&str>) -> Option { + match scheme { + None => Some(Self(None)), + Some(name) => Palette::SCHEMES + .into_iter() + .find(|known| *known == name) + .map(|known| Self(Some(known))), + } + } +} + +/// `solarized-light` reads as `Solarized Light`, as the chrome themes do. +impl fmt::Display for SchemeChoice { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Some(name) = self.0 else { + return f.write_str(crate::strings::SETTINGS_SCHEME_BUILTIN); + }; + let words: Vec = name + .split('-') + .map(|word| { + let mut chars = word.chars(); + chars.next().map_or_else(String::new, |first| { + first.to_uppercase().chain(chars).collect() + }) + }) + .collect(); + f.write_str(&words.join(" ")) + } +} + +impl Shell { + /// Apply a chrome theme and persist it. + pub(super) fn pick_theme(&mut self, theme: ThemeChoice) { + self.appearance.theme = theme; + self.appearance_sink + .persist(&AppearanceChange::Theme(theme)); + } + + /// Apply a terminal scheme to every session and persist it. + pub(super) fn pick_scheme(&mut self, scheme: SchemeChoice) { + let name = scheme.0.map(str::to_owned); + self.appearance.colors.scheme.clone_from(&name); + self.config.terminal_scheme = name; + self.appearance_sink + .repaint(self.appearance.colors.to_palette()); + self.appearance_sink.persist(&AppearanceChange::Scheme( + self.appearance.colors.scheme.clone(), + )); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_scheme_entry_reads_as_words_and_finds_itself() { + let labels: Vec = SchemeChoice::all() + .iter() + .map(ToString::to_string) + .collect(); + assert_eq!( + labels, + [ + "Built-in", + "Solarized Dark", + "Solarized Light", + "Gruvbox Dark", + "Gruvbox Light" + ] + ); + for choice in SchemeChoice::all() { + assert_eq!(SchemeChoice::of(choice.0), Some(choice)); + } + assert_eq!(SchemeChoice::of(Some("no-such")), None); + } +} diff --git a/crates/app/src/shell/routing.rs b/crates/app/src/shell/routing.rs index 806c2ae..6c8ecad 100644 --- a/crates/app/src/shell/routing.rs +++ b/crates/app/src/shell/routing.rs @@ -42,6 +42,8 @@ pub(super) enum KeyboardOwner { TabClose(usize), /// The archive confirmation. Archive, + /// The settings panel. + Settings, /// The document editor, which handles its own keys. Doc, } @@ -54,12 +56,13 @@ impl KeyboardOwner { /// `match` below: a new variant fails to compile there, in this file, where /// this array is the next thing the author reads. #[cfg(test)] - pub(super) const ALL: [Self; 6] = [ + pub(super) const ALL: [Self; 7] = [ Self::TabRename, Self::SessionRename, Self::Quit, Self::TabClose(0), Self::Archive, + Self::Settings, Self::Doc, ]; @@ -71,6 +74,7 @@ impl KeyboardOwner { Self::Quit => "quit-confirm", Self::TabClose(_) => "tab-close-confirm", Self::Archive => "archive-confirm", + Self::Settings => "settings", Self::Doc => "doc-editor", } } @@ -189,6 +193,7 @@ impl Shell { operate(focusable::focus(search_id())) } Action::ToggleSidebar => self.toggle_sidebar(), + Action::OpenSettings => self.update(Message::ToggleSettings), Action::ScrollTop => self .scroll_focused(ScrollTarget::Top) .ok_or(Inertia::NoContext)?, @@ -317,6 +322,9 @@ impl Shell { if self.archiving.is_some() { return Some(KeyboardOwner::Archive); } + if self.settings_open { + return Some(KeyboardOwner::Settings); + } if self.open_doc.is_some() { return Some(KeyboardOwner::Doc); } @@ -333,6 +341,7 @@ impl Shell { KeyboardOwner::Quit => self.quit_confirm_key(event), KeyboardOwner::TabClose(index) => self.tab_close_confirm_key(event, index), KeyboardOwner::Archive => self.archive_confirm_key(event), + KeyboardOwner::Settings => self.settings_key(event), KeyboardOwner::Doc => self.open_doc_key(event), } } @@ -389,6 +398,15 @@ impl Shell { } } + /// Escape closes the settings panel; it has no text field, so every other + /// key is swallowed rather than typed into the terminal beneath it. + fn settings_key(&mut self, event: &keyboard::Event) -> Task { + if is_escape(event) { + return self.update(Message::CloseSettings); + } + Task::none() + } + /// The doc editor handles its own keys; only the save chord (Cmd/Ctrl+S) /// and Escape are intercepted. Escape closes the editor exactly as its own /// close button does — unsaved edits included, since a stricter gesture for diff --git a/crates/app/src/shell/terminal/canvas.rs b/crates/app/src/shell/terminal/canvas.rs index ba88a1d..334313b 100644 --- a/crates/app/src/shell/terminal/canvas.rs +++ b/crates/app/src/shell/terminal/canvas.rs @@ -539,14 +539,16 @@ impl canvas::Program for TerminalView<'_> { } // An unfocused window renders behind a translucent scrim so the - // active window is visually obvious among several. + // active window is visually obvious among several. The scrim is the + // grid's own background, so the text fades into it on a light scheme + // as on a dark one — black would turn a light grid grey instead. if self.dimmed { frame.fill_rectangle( Point::ORIGIN, bounds.size(), Color { - a: 0.35, - ..Color::BLACK + a: 0.45, + ..rgb(self.screen.default_bg) }, ); } diff --git a/crates/app/src/shell/view/mod.rs b/crates/app/src/shell/view/mod.rs index dcf5d17..9ae36c2 100644 --- a/crates/app/src/shell/view/mod.rs +++ b/crates/app/src/shell/view/mod.rs @@ -22,13 +22,14 @@ use crate::strings; mod doc_editor; mod modals; +mod settings_panel; mod sidebar; mod style; mod tabs; use doc_editor::doc_editor; use modals::modal; -use style::{card_secondary_text, card_style, clip, mix, sidebar_secondary_text, status_color}; +use style::{card_secondary_text, card_style, clip, mix, sidebar_secondary_text, status_dot}; impl Shell { pub(super) fn view(&self) -> Element<'_, Message> { @@ -52,10 +53,15 @@ impl Shell { // Any armed confirmation — quit, tab-close or archive — overlays the // same centred modal, so the about-to-change sessions stay untouchable // until the user decides. `active_confirmation` picks the one in force. - match self.active_confirmation() { - Some((card, on_cancel)) => modal(base, card, on_cancel), - None => base, + // The settings panel sits under any confirmation, matching the + // keyboard ladder: a quit armed while it is open is answered first. + if let Some((card, on_cancel)) = self.active_confirmation() { + return modal(base, card, on_cancel); } + if self.settings_open { + return modal(base, self.settings_panel(), Message::CloseSettings); + } + base } /// The focused terminal: its tab strip, then its grid drawn on a canvas. diff --git a/crates/app/src/shell/view/settings_panel.rs b/crates/app/src/shell/view/settings_panel.rs new file mode 100644 index 0000000..2bca9c7 --- /dev/null +++ b/crates/app/src/shell/view/settings_panel.rs @@ -0,0 +1,67 @@ +//! The settings panel: a card over the workspace whose picks apply live — the +//! chrome on the next frame, every terminal through the appearance sink — and +//! save themselves. No state transitions live here; those are in +//! `shell::appearance`. + +use iced::widget::{button, column, container, pick_list, row, text}; +use iced::{Element, Fill}; + +use super::sidebar_secondary_text; +use crate::settings::ThemeChoice; +use crate::shell::appearance::SchemeChoice; +use crate::shell::{Message, Shell}; +use crate::strings; + +impl Shell { + pub(super) fn settings_panel(&self) -> Element<'_, Message> { + let header = row![ + text(strings::SETTINGS_TITLE).size(16), + iced::widget::Space::new().width(Fill), + button(text(strings::SETTINGS_CLOSE).size(12)) + .on_press(Message::CloseSettings) + .style(button::text) + .padding(4), + ] + .align_y(iced::Center); + + let theme = pick_list( + ThemeChoice::ALL, + Some(self.appearance.theme), + Message::PickTheme, + ) + .text_size(12) + .width(180); + let scheme = pick_list( + SchemeChoice::all(), + SchemeChoice::of(self.appearance.colors.scheme.as_deref()), + Message::PickScheme, + ) + .text_size(12) + .width(180); + + let field = |label: &'static str, control: Element<'static, Message>| { + row![text(label).size(12).width(Fill), control] + .spacing(12) + .align_y(iced::Center) + }; + + container( + column![ + header, + field(strings::SETTINGS_THEME, theme.into()), + field(strings::SETTINGS_SCHEME, scheme.into()), + text(strings::SETTINGS_SAVED_NOTE) + .size(11) + .style(sidebar_secondary_text), + text(strings::SETTINGS_CLAUDE_NOTE) + .size(11) + .style(sidebar_secondary_text), + ] + .spacing(12), + ) + .padding(16) + .width(420) + .style(container::rounded_box) + .into() + } +} diff --git a/crates/app/src/shell/view/sidebar.rs b/crates/app/src/shell/view/sidebar.rs index 05c78d8..dff40d9 100644 --- a/crates/app/src/shell/view/sidebar.rs +++ b/crates/app/src/shell/view/sidebar.rs @@ -16,7 +16,7 @@ use iced::{Element, Fill}; use termherd_core::browser::{ProjectGroup, project_label, relative_age}; use termherd_core::{SessionRecord, SessionStatus, SidebarFold}; -use super::{clip, session_card, sidebar_secondary_text, status_color}; +use super::{clip, session_card, sidebar_secondary_text, status_dot}; use crate::shell::{Focus, Message, Shell, rename_id, search_id}; use crate::strings; @@ -132,10 +132,25 @@ impl Shell { .style(container::rounded_box), tooltip::Position::Bottom, ); + let settings = tooltip( + button(text(strings::SIDEBAR_SETTINGS).size(13)) + .on_press(Message::ToggleSettings) + .style(button::text) + .padding(0), + container(text(strings::SIDEBAR_SETTINGS_HINT).size(11)) + .padding(6) + .style(container::rounded_box), + tooltip::Position::Bottom, + ); let chrome = column![ - row![hide, iced::widget::Space::new().width(Fill), add_repo] - .spacing(6) - .align_y(iced::Center), + row![ + hide, + iced::widget::Space::new().width(Fill), + add_repo, + settings + ] + .spacing(6) + .align_y(iced::Center), search, titles_only, show_archived @@ -196,7 +211,7 @@ impl Shell { .padding(0); let mut label_row = row![].spacing(6).align_y(iced::Center); if let Some(status) = live.get(id) { - label_row = label_row.push(text("●").size(9).color(status_color(*status))); + label_row = label_row.push(status_dot(*status)); } let title = self.core.session_title(s); // The project label tells cross-project favourites apart. @@ -382,7 +397,7 @@ impl Shell { // A coloured dot marks a session already open in TermHerd and // carries its live activity (FR8). if let Some(status) = live.get(id) { - content = content.push(text("●").size(9).color(status_color(*status))); + content = content.push(status_dot(*status)); } let title = self.core.session_title(s); let renaming_this = self.renaming.as_ref().is_some_and(|(rid, _)| rid == id); diff --git a/crates/app/src/shell/view/style.rs b/crates/app/src/shell/view/style.rs index 0ba2bd2..cfacf47 100644 --- a/crates/app/src/shell/view/style.rs +++ b/crates/app/src/shell/view/style.rs @@ -5,23 +5,43 @@ //! view tracks the theme system once it lands. use iced::Color; -use iced::widget::container; +use iced::widget::text::Text; +use iced::widget::{container, text}; use termherd_core::SessionStatus; -/// The dot colour for an activity status (FR8). Shared by the tab strip's -/// chips and the sidebar's per-session dots so both stay in sync. The colour is -/// the only place the UI shows a status; the word form an agent reads lives in +/// The activity dot for a status (FR8). Shared by the tab strip's chips and +/// the sidebar's per-session dots so both stay in sync. The colour is the only +/// place the UI shows a status; the word form an agent reads lives in /// [`crate::snapshot_dto`], not on the screen. -pub(super) fn status_color(status: SessionStatus) -> Color { - match status { +pub(super) fn status_dot<'a>(status: SessionStatus) -> Text<'a> { + text("●") + .size(9) + .style(move |theme: &iced::Theme| text::Style { + color: Some(status_color(status, theme.extended_palette().is_dark)), + }) +} + +/// The dot colour for a status. The hues are tuned against a dark surface; +/// on a light one they wash out, so they are darkened by the same step to keep +/// the dot legible without changing which hue means what. +fn status_color(status: SessionStatus, dark_surface: bool) -> Color { + let hue = match status { SessionStatus::Starting => Color::from_rgb(0.55, 0.55, 0.6), SessionStatus::Busy => Color::from_rgb(0.95, 0.7, 0.2), SessionStatus::Idle => Color::from_rgb(0.3, 0.8, 0.4), SessionStatus::Attention => Color::from_rgb(0.95, 0.35, 0.35), SessionStatus::Exited => Color::from_rgb(0.5, 0.5, 0.5), + }; + if dark_surface { + hue + } else { + mix(hue, Color::BLACK, LIGHT_SURFACE_DARKEN) } } +/// How far a status hue moves toward black on a light surface. +const LIGHT_SURFACE_DARKEN: f32 = 0.4; + /// Background for the session hover card — a step away from the surrounding /// surface (the `strong` palette tier rather than the default `weak`) so the /// card reads as a distinct floating layer, with a thin border to seal it. @@ -90,3 +110,65 @@ pub(super) fn clip(s: &str, max: usize) -> String { out } } + +#[cfg(test)] +mod tests { + use super::*; + + /// WCAG relative luminance, enough to compare contrast ratios. + fn luminance(c: Color) -> f32 { + let lin = |v: f32| { + if v <= 0.039_28 { + v / 12.92 + } else { + ((v + 0.055) / 1.055).powf(2.4) + } + }; + 0.2126 * lin(c.r) + 0.7152 * lin(c.g) + 0.0722 * lin(c.b) + } + + fn contrast(a: Color, b: Color) -> f32 { + let (hi, lo) = { + let (la, lb) = (luminance(a), luminance(b)); + if la > lb { (la, lb) } else { (lb, la) } + }; + (hi + 0.05) / (lo + 0.05) + } + + const ALL: [SessionStatus; 5] = [ + SessionStatus::Starting, + SessionStatus::Busy, + SessionStatus::Idle, + SessionStatus::Attention, + SessionStatus::Exited, + ]; + + #[test] + fn every_status_dot_reads_on_every_light_chrome() { + // The WCAG floor for non-text UI components. + const FLOOR: f32 = 3.0; + for theme in [ + iced::Theme::Light, + iced::Theme::SolarizedLight, + iced::Theme::GruvboxLight, + ] { + let palette = theme.extended_palette(); + assert!(!palette.is_dark, "{theme} reads as light"); + for surface in [palette.background.base.color, palette.background.weak.color] { + for status in ALL { + let ratio = contrast(status_color(status, false), surface); + assert!(ratio >= FLOOR, "{status:?} on {theme}: {ratio:.2}"); + } + } + } + } + + #[test] + fn a_dark_surface_keeps_the_original_hues() { + for status in ALL { + let dark = status_color(status, true); + let light = status_color(status, false); + assert!(luminance(light) < luminance(dark), "{status:?} darkens"); + } + } +} diff --git a/crates/app/src/shell/view/tabs.rs b/crates/app/src/shell/view/tabs.rs index df2799b..99f02d3 100644 --- a/crates/app/src/shell/view/tabs.rs +++ b/crates/app/src/shell/view/tabs.rs @@ -10,7 +10,7 @@ use iced::widget::{button, column, container, mouse_area, row, text, text_input, use iced::{Color, Element}; use termherd_core::workspace::Tab; -use super::{card_secondary_text, card_style, clip, session_card, status_color}; +use super::{card_secondary_text, card_style, clip, session_card, status_dot}; use crate::shell::{Message, Shell, tab_rename_id}; impl Shell { @@ -53,7 +53,7 @@ impl Shell { let buffer = self.tab_rename.as_ref().map_or("", |(_, b)| b.as_str()); let mut inner = row![].spacing(6).align_y(iced::Center); if let Some(status) = self.core.tab_status(index) { - inner = inner.push(text("●").size(9).color(status_color(status))); + inner = inner.push(status_dot(status)); } inner = inner.push( text_input("", buffer) @@ -71,7 +71,7 @@ impl Shell { } else { let mut inner = row![].spacing(6).align_y(iced::Center); if let Some(status) = self.core.tab_status(index) { - inner = inner.push(text("●").size(9).color(status_color(status))); + inner = inner.push(status_dot(status)); } inner = inner.push(text(clip(tab.display_title(), 24)).size(12)); // The × lives inside the chip so it sits on the active tab's fill, diff --git a/crates/app/src/strings.rs b/crates/app/src/strings.rs index 557eb06..b68de3d 100644 --- a/crates/app/src/strings.rs +++ b/crates/app/src/strings.rs @@ -21,6 +21,8 @@ pub const SIDEBAR_HIDE: &str = "◀ Hide"; pub const SIDEBAR_ADD_REPO: &str = "+ Add a repo"; pub const SIDEBAR_ADD_REPO_HINT: &str = "Pick a folder, or drop one on the window"; pub const SIDEBAR_FORGET_REPO: &str = "Remove this repo from the sidebar"; +pub const SIDEBAR_SETTINGS: &str = "⚙"; +pub const SIDEBAR_SETTINGS_HINT: &str = "Settings"; pub const SIDEBAR_REPO_NO_SESSIONS: &str = "No sessions yet — start one with $ or 🤖"; /// Expander under a truncated session list: how many more are folded. @@ -99,3 +101,13 @@ pub fn quit_prompt(live: usize) -> String { ) } } + +// --- Settings panel --- +pub const SETTINGS_TITLE: &str = "Settings"; +pub const SETTINGS_THEME: &str = "Interface theme"; +pub const SETTINGS_SCHEME: &str = "Terminal colours"; +pub const SETTINGS_SCHEME_BUILTIN: &str = "Built-in"; +pub const SETTINGS_CLOSE: &str = "Close"; +pub const SETTINGS_SAVED_NOTE: &str = "Changes apply now and are saved to settings.json."; +pub const SETTINGS_CLAUDE_NOTE: &str = + "A running Claude session keeps its own light or dark look until it restarts."; diff --git a/crates/core/src/keymap.rs b/crates/core/src/keymap.rs index 11d76e2..e743a82 100644 --- a/crates/core/src/keymap.rs +++ b/crates/core/src/keymap.rs @@ -120,6 +120,8 @@ pub enum Action { PrevTab, FocusSearch, ToggleSidebar, + /// Open the settings panel, or close it when it is already open. + OpenSettings, Copy, Paste, /// Jump the focused terminal's viewport to the top of its scrollback. @@ -243,6 +245,11 @@ const ACTIONS: &[ActionDef] = &[ name: "toggle-sidebar", default_chords: &["mod+b"], }, + ActionDef { + action: Action::OpenSettings, + name: "open-settings", + default_chords: &["mod+,"], + }, ActionDef { action: Action::Copy, name: "copy", diff --git a/crates/pty/src/grid.rs b/crates/pty/src/grid.rs index 028ce38..f54ebeb 100644 --- a/crates/pty/src/grid.rs +++ b/crates/pty/src/grid.rs @@ -169,6 +169,15 @@ impl Palette { } } + /// Every name [`Palette::named`] resolves, in the order a picker lists + /// them. + pub const SCHEMES: [&'static str; 4] = [ + "solarized-dark", + "solarized-light", + "gruvbox-dark", + "gruvbox-light", + ]; + /// A built-in scheme by its settings name (`terminal.colors.scheme`), or /// `None` when unknown. The unnamed built-in default is /// [`Palette::default`]. Values from the published Solarized (Ethan @@ -1090,4 +1099,14 @@ mod tests { ); } } + + #[test] + fn every_listed_scheme_resolves() { + for name in Palette::SCHEMES { + assert!( + Palette::named(name).is_some(), + "{name} is listed but unknown" + ); + } + } } diff --git a/crates/pty/src/manager.rs b/crates/pty/src/manager.rs index d3d3da1..a55c2fa 100644 --- a/crates/pty/src/manager.rs +++ b/crates/pty/src/manager.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use std::io::Write; -use std::sync::{Arc, Mutex, mpsc}; +use std::sync::{Arc, Mutex, RwLock, mpsc}; use portable_pty::{CommandBuilder, PtySize, native_pty_system}; use termherd_core::ports::{PtyError, PtyHost}; @@ -22,8 +22,8 @@ use crate::launch::{ write_title_settings, }; use crate::session::{ - Session, SharedMaster, SharedWriter, TermCmd, spawn_reader, spawn_term, spawn_waiter, - spawn_watcher, + Session, SharedMaster, SharedPalette, SharedWriter, TermCmd, spawn_reader, spawn_term, + spawn_waiter, spawn_watcher, }; /// How to launch a session's shell process (FR10). Built from the user's @@ -44,8 +44,9 @@ pub struct PtyManager { sink: EventSink, /// User-configured shell; `None` falls back to the platform default. shell: Option, - /// The terminal colour scheme every session renders with. - palette: Palette, + /// The terminal colour scheme every session renders with, shared with + /// each terminal thread so [`PtyManager::set_palette`] reaches them live. + palette: SharedPalette, } impl PtyManager { @@ -58,7 +59,24 @@ impl PtyManager { sessions: Mutex::new(HashMap::new()), sink, shell, - palette, + palette: Arc::new(RwLock::new(palette)), + } + } + + /// Switch every session — running and future — to `palette`. Each live + /// terminal re-emits its screen in the new colours; a later colour query + /// (OSC 10/11) is answered from it too. A program that asked once at start + /// (Claude Code picks light or dark that way) keeps its choice until it + /// asks again. + pub fn set_palette(&self, palette: Palette) { + match self.palette.write() { + Ok(mut guard) => *guard = palette, + Err(poisoned) => *poisoned.into_inner() = palette, + } + if let Ok(map) = self.sessions.lock() { + for session in map.values() { + let _ = session.ctrl.send(TermCmd::Repaint); + } } } @@ -554,6 +572,44 @@ mod tests { mgr.kill(id).expect("kill"); } + /// A scheme change reaches a session that is already running: its next + /// screen arrives in the new colours with no respawn and no output of its + /// own to trigger it. + #[test] + fn set_palette_repaints_a_running_session() { + let next = Palette { + background: [0x12, 0x34, 0x56], + ..Palette::default() + }; + let (tx, rx) = mpsc::channel::(); + let sink: EventSink = Arc::new(move |ev| { + let _ = tx.send(ev); + }); + let mgr = PtyManager::new(sink, None, Palette::default()); + let id = sid(8); + mgr.spawn(spec(id)).expect("spawn"); + mgr.set_palette(next.clone()); + + let deadline = Instant::now() + Duration::from_secs(15); + let mut repainted = false; + while Instant::now() < deadline { + match rx.recv_timeout(Duration::from_millis(500)) { + Ok(PtyEvent::Output { screen, .. }) if screen.default_bg == next.background => { + repainted = true; + break; + } + Ok(_) | Err(mpsc::RecvTimeoutError::Timeout) => continue, + Err(_) => break, + } + } + assert!( + repainted, + "expected a screen in the palette set after spawn" + ); + + mgr.kill(id).expect("kill"); + } + #[test] fn control_methods_reject_unknown_sessions() { let sink: EventSink = Arc::new(|_| {}); diff --git a/crates/pty/src/session.rs b/crates/pty/src/session.rs index 7f95230..bd8bb7e 100644 --- a/crates/pty/src/session.rs +++ b/crates/pty/src/session.rs @@ -6,7 +6,7 @@ //! colour queries the parser raises. use std::io::{Read, Write}; -use std::sync::{Arc, Mutex, mpsc}; +use std::sync::{Arc, Mutex, RwLock, mpsc}; use std::thread::JoinHandle; use alacritty_terminal::Term; @@ -45,6 +45,20 @@ pub(crate) type SharedWriter = Arc>>; /// thread (reading the foreground process group). Neither holds it for long. pub(crate) type SharedMaster = Arc>>; +/// The palette every session renders with, shared between the manager and +/// each terminal thread so a scheme change reaches running sessions without a +/// respawn. +pub(crate) type SharedPalette = Arc>; + +/// The palette in force right now. A poisoned lock still holds a whole +/// palette — a writer cannot half-assign one — so its value is read anyway. +fn current(palette: &SharedPalette) -> Palette { + match palette.read() { + Ok(guard) => guard.clone(), + Err(poisoned) => poisoned.into_inner().clone(), + } +} + /// How often the watcher thread asks the PTY which process group owns it. Short /// enough that `wait_for_status` on an unintegrated shell settles promptly, /// long enough to be free: one `tcgetpgrp` per session per tick. @@ -61,14 +75,14 @@ const FOREGROUND_POLL: std::time::Duration = std::time::Duration::from_millis(25 #[derive(Clone)] struct PtyResponder { writer: SharedWriter, - palette: Palette, + palette: SharedPalette, } impl EventListener for PtyResponder { fn send_event(&self, event: Event) { let reply = match event { Event::PtyWrite(text) => text, - Event::ColorRequest(index, format) => match query_rgb(index, &self.palette) { + Event::ColorRequest(index, format) => match query_rgb(index, ¤t(&self.palette)) { Some([r, g, b]) => format(Rgb { r, g, b }), None => return, }, @@ -112,6 +126,8 @@ pub(crate) enum TermCmd { /// activity, or `None` when the platform cannot say. The stand-in source /// for a shell whose integration did not take (see [`crate::status`]). Foreground(Option), + /// The shared palette changed: emit a fresh screen in the new colours. + Repaint, /// The PTY reached end of file; the process is gone. `clean` carries the /// reaped exit status (see [`PtyEvent::Exited`]). Eof { clean: bool }, @@ -334,7 +350,7 @@ pub(crate) fn spawn_term( size: (u16, u16), writer: SharedWriter, sink: EventSink, - palette: Palette, + palette: SharedPalette, ) -> JoinHandle<()> { let (cols, rows) = size; let term_sink = sink.clone(); @@ -427,6 +443,9 @@ pub(crate) fn spawn_term( PointerInput::Selected => {} PointerInput::Dropped => continue, }, + // Nothing to apply: the snapshot below reads the new + // palette. + TermCmd::Repaint => {} TermCmd::CopySelection => { // Read the text from the live selection, not a snapshot, // so a fast drag's copy is exact. Commands are FIFO, so @@ -469,14 +488,14 @@ pub(crate) fn spawn_term( } term_sink(PtyEvent::Output { session, - screen: snapshot(&term, &palette), + screen: snapshot(&term, ¤t(&palette)), }); } // A final snapshot so any drained bytes reach the screen the tab // keeps showing on an unclean exit. term_sink(PtyEvent::Output { session, - screen: snapshot(&term, &palette), + screen: snapshot(&term, ¤t(&palette)), }); term_sink(PtyEvent::Exited { session, clean }); }) @@ -570,7 +589,9 @@ mod tests { (80, 24), writer, sink, - Palette::named("solarized-light").expect("known scheme"), + Arc::new(RwLock::new( + Palette::named("solarized-light").expect("known scheme"), + )), ); for chunk in chunks { ctrl.send(TermCmd::Bytes(chunk.as_bytes().to_vec())) @@ -641,7 +662,7 @@ mod tests { &TermSize::new(20, 5), PtyResponder { writer, - palette: palette.clone(), + palette: Arc::new(RwLock::new(palette.clone())), }, ); let mut parser: Processor = Processor::new(); @@ -665,7 +686,7 @@ mod tests { &TermSize::new(20, 5), PtyResponder { writer, - palette: palette.clone(), + palette: Arc::new(RwLock::new(palette.clone())), }, ); let mut parser: Processor = Processor::new(); @@ -739,7 +760,14 @@ mod tests { let sink: EventSink = Arc::new(|_| {}); let session = SessionId(std::num::NonZeroU64::new(1).expect("nonzero")); let (ctrl, ctrl_rx) = mpsc::channel(); - let term = spawn_term(session, ctrl_rx, (80, 24), writer, sink, Palette::default()); + let term = spawn_term( + session, + ctrl_rx, + (80, 24), + writer, + sink, + Arc::new(RwLock::new(Palette::default())), + ); ctrl.send(TermCmd::Bytes(b"\x1b[?1000h\x1b[?1006h".to_vec())) .expect("terminal thread alive"); ctrl.send(TermCmd::Pointer(PointerEvent::left( diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index f216b2a..857b7c3 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -14,6 +14,7 @@ - [The terminal](./workspace/terminal.md) - [Status and attention](./workspace/status.md) - [Capture and record](./workspace/capture.md) +- [The settings panel](./workspace/settings-panel.md) # Reference diff --git a/docs/src/reference/keyboard.md b/docs/src/reference/keyboard.md index e8d2864..962b3f9 100644 --- a/docs/src/reference/keyboard.md +++ b/docs/src/reference/keyboard.md @@ -67,6 +67,7 @@ across layouts. | --- | --- | --- | | `focus-search` | `mod+f` | focus the sidebar search box | | `toggle-sidebar` | `mod+b` | show / hide the sidebar | +| `open-settings` | `mod+,` | open / close the [settings panel](../workspace/settings-panel.md) | | `capture` | `mod+shift+s` | write a state dump + screenshot | | `toggle-record` | `mod+shift+r` | start / stop a GIF screencast | diff --git a/docs/src/reference/settings.md b/docs/src/reference/settings.md index 4ed3150..981e2b6 100644 --- a/docs/src/reference/settings.md +++ b/docs/src/reference/settings.md @@ -5,7 +5,8 @@ %USERPROFILE%\.termherd\settings.json (Windows) ``` -There is **no in-app settings panel yet**: edit the file and restart. +The [settings panel](../workspace/settings-panel.md) changes the appearance +live and saves it here. Everything else: edit the file and restart. The annotated template with every option, its default and its meaning is [`docs/settings.example.jsonc`](https://github.com/Termherd/termherd/blob/main/docs/settings.example.jsonc) @@ -14,7 +15,8 @@ JSON**: no comments, no trailing commas. ## How it loads -Read once at startup, and defensively: +Read once at startup — the settings panel's two keys aside — and +defensively: - Every field is optional. A missing file, a missing field, or a corrupt file falls back to built-in defaults — settings never block startup. @@ -48,7 +50,8 @@ only when named explicitly. See [Status and attention](../workspace/status.md). `"solarized-dark"`, `"solarized-light"`, `"gruvbox-dark"`, `"gruvbox-light"`. GUI chrome only — sidebar, tab strip, buttons. The terminal grid keeps its own colours: pick the matching `terminal.colors.scheme` for a chrome that blends -with it. +with it. An unknown value falls back to `"dark"` alone. Both keys are +also set live from the [settings panel](../workspace/settings-panel.md). ### `close` diff --git a/docs/src/workspace/settings-panel.md b/docs/src/workspace/settings-panel.md new file mode 100644 index 0000000..0764756 --- /dev/null +++ b/docs/src/workspace/settings-panel.md @@ -0,0 +1,40 @@ +# The settings panel + +The **⚙** at the top of the sidebar — or +Cmd/Ctrl+, (rebindable as `open-settings`) — +opens a panel over the workspace. Escape, **Close**, or a click +outside it closes it. + +```text +┌ Settings ─────────────────────────── Close ┐ +│ Interface theme [ Solarized Light ▾ ] │ +│ Terminal colours [ Solarized Light ▾ ] │ +└──────────────────────────────────────────────┘ +``` + +Today it holds the appearance. The rest of `settings.json` is still edited in +the file — see [settings.json](../reference/settings.md). + +## A pick applies at once, and saves itself + +There is no Apply button. Picking a value: + +- **Interface theme** restyles the sidebar, tab strip and buttons on the next + frame. `Dark`, `Light`, and four presets named after the terminal schemes. +- **Terminal colours** recolours every open terminal, not only new ones. + `Built-in` is the default palette. Explicit colour overrides in + `terminal.colors` still win over the scheme, as they do at startup. + +Each pick writes its own key to `settings.json` — `theme` or +`terminal.colors.scheme` — and leaves the rest of the file as you wrote it. +A file that does not parse is left alone rather than rewritten. + +Pair a preset with the scheme of the same name for a chrome that matches the +grid it frames. + +## One thing a live change cannot reach + +A program asks the terminal for its background colour when it starts, and +picks a light or a dark look from the answer. Claude Code does this. A Claude +session that was already running keeps the look it chose until it restarts; +TermHerd's own colours change at once. New sessions get it right. diff --git a/docs/src/workspace/sidebar.md b/docs/src/workspace/sidebar.md index 50ee1f4..7290b99 100644 --- a/docs/src/workspace/sidebar.md +++ b/docs/src/workspace/sidebar.md @@ -10,7 +10,7 @@ Toggle it with Cmd/Ctrl+B. ```text ┌──────────────────────────┐ -│ ◀ Hide + Add a repo │ ← add a folder the scan cannot know about +│ ◀ Hide + Add a repo ⚙ │ ← add a folder · open the settings panel │ Search… │ ← Cmd/Ctrl+F focuses this │ ☐ Titles only │ │ ☐ Show archived │ From d23ee6f51cc4122ebab674a1f92818a01c971191 Mon Sep 17 00:00:00 2001 From: Guillaume Jay Date: Mon, 5 Oct 2026 10:11:16 +0200 Subject: [PATCH 3/6] fix(settings): set an unparseable config file aside instead of overwriting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A settings.json (or metadata.json) that failed to parse was replaced by defaults with only a log line, and the next save wrote those defaults over it — one bad byte cost the whole configuration, permanently. The startup load now moves such a file to .corrupt- (never clobbering an earlier backup) and reports it in a dismissible sidebar notice. Only the startup load does this: a later read may race an editor's half-written file, which must not be moved. A write that edits keys in place already refuses a file that is not a JSON object. Closes #292 --- README.md | 13 +- crates/app/src/json_store.rs | 185 +++++++++++++++++++++++++-- crates/app/src/main.rs | 13 +- crates/app/src/metadata_store.rs | 9 +- crates/app/src/settings.rs | 7 + crates/app/src/shell.rs | 36 ++++++ crates/app/src/shell/view/sidebar.rs | 27 +++- crates/app/src/strings.rs | 13 ++ docs/src/reference/settings.md | 4 + 9 files changed, 283 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 05b2904..caf8ae7 100644 --- a/README.md +++ b/README.md @@ -115,12 +115,13 @@ cargo run -p termherd-app Optional user settings live in `~/.termherd/settings.json` (on Windows, `%USERPROFILE%\.termherd\settings.json`). The file is read at startup; if it is missing or invalid, TermHerd falls back to defaults rather than refusing -to start — out-of-range values clamp, and a single bad value (a typo'd -colour, an unknown key action) degrades alone with a logged warning instead -of resetting the rest of the file. The appearance (chrome theme and -terminal colours) can also be changed live from the settings panel — ⚙ in -the sidebar, or `Cmd`/`Ctrl`+`,` — which saves it here; everything else: -edit the file and restart. +to start (an invalid file is first set aside as `settings.json.corrupt-1`, +and the sidebar says so). Out-of-range values clamp, and a single bad value +(a typo'd colour, an unknown key action) degrades alone with a logged +warning instead of resetting the rest of the file. The appearance (chrome +theme and terminal colours) can also be changed live from the settings +panel — ⚙ in the sidebar, or `Cmd`/`Ctrl`+`,` — which saves it here; +everything else: edit the file and restart. The annotated reference template is [`docs/settings.example.jsonc`](docs/settings.example.jsonc): every option diff --git a/crates/app/src/json_store.rs b/crates/app/src/json_store.rs index aaca149..afeda67 100644 --- a/crates/app/src/json_store.rs +++ b/crates/app/src/json_store.rs @@ -2,29 +2,133 @@ //! load/save shape every file adapter shares: read the file and fall back to //! the default on any problem (missing, unreadable, corrupt — with a warning), //! create the dir and pretty-print on save, failures logged but never fatal. +//! +//! A file that exists but does not parse is **set aside** before the default +//! is adopted, so the next save cannot overwrite what the user wrote: a +//! fallback that also destroys its input turns one bad byte into a lost +//! configuration. //! Per-type concerns (DTO mapping, sanitising, legacy migration) stay in each //! store; this owns only the file plumbing they used to copy. use serde::Serialize; use serde::de::DeserializeOwned; use serde_json::{Map, Value}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use tracing::warn; +/// A config file that existed and could not be used, so its defaults are in +/// force. Reported rather than only logged: a user whose settings vanished +/// deserves to be told where they went. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LoadProblem { + /// The file's name under `~/.termherd`, e.g. `settings.json`. + pub file: String, + /// Where the unparseable original was moved, or `None` when it could not + /// be read at all (and so was left where it is). + pub kept_as: Option, +} + /// Load `~/.termherd/`; any problem (no home dir, no file, bad JSON) -/// yields the default — a config file must never block startup. +/// yields the default — a config file must never block startup. Touches +/// nothing on disk: safe for a read that may race a half-written file. #[must_use] pub fn load_json(file: &str) -> T { let Some(path) = config_path(file) else { return T::default(); }; - let Ok(raw) = std::fs::read_to_string(&path) else { - return T::default(); - }; - serde_json::from_str(&raw).unwrap_or_else(|e| { - warn!(error = %e, path = %path.display(), "invalid config file; using defaults"); - T::default() - }) + match read_at(&path) { + Read::Parsed(value) => value, + Read::Missing => T::default(), + Read::Unreadable(e) => { + warn!(error = %e, path = %path.display(), "unreadable config file; using defaults"); + T::default() + } + Read::Invalid(e) => { + warn!(error = %e, path = %path.display(), "invalid config file; using defaults"); + T::default() + } + } +} + +/// The startup load: [`load_json`], but a file that does not parse is set +/// aside (see [`set_aside`]) and reported, so the defaults adopted in its place +/// can be saved without destroying it. Startup only — a later read may see a +/// file an editor is halfway through writing, which must not be moved. +#[must_use] +pub fn load_json_checked(file: &str) -> (T, Option) { + match config_path(file) { + Some(path) => load_json_at(&path), + None => (T::default(), None), + } +} + +/// The body of [`load_json_checked`], on an explicit path. +fn load_json_at(path: &Path) -> (T, Option) { + let file = path + .file_name() + .map_or_else(String::new, |name| name.to_string_lossy().into_owned()); + match read_at(path) { + Read::Parsed(value) => (value, None), + Read::Missing => (T::default(), None), + Read::Unreadable(e) => { + warn!(error = %e, path = %path.display(), "unreadable config file; using defaults"); + ( + T::default(), + Some(LoadProblem { + file, + kept_as: None, + }), + ) + } + Read::Invalid(e) => { + let kept_as = set_aside(path); + warn!( + error = %e, + path = %path.display(), + kept_as = ?kept_as, + "invalid config file; set it aside and using defaults" + ); + (T::default(), Some(LoadProblem { file, kept_as })) + } + } +} + +/// What reading a config file found. +enum Read { + Parsed(T), + /// No file: every setting starts this way, so it is no problem. + Missing, + Unreadable(std::io::Error), + Invalid(serde_json::Error), +} + +fn read_at(path: &Path) -> Read { + match std::fs::read_to_string(path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Read::Missing, + Err(e) => Read::Unreadable(e), + Ok(raw) => match serde_json::from_str(&raw) { + Ok(value) => Read::Parsed(value), + Err(e) => Read::Invalid(e), + }, + } +} + +/// Move an unparseable file to the first free `.corrupt-` beside +/// it, so the next save starts a fresh file instead of overwriting the +/// original. `None` when no name is free or the move fails — then the file +/// stays, and [`update_json`] still refuses to write over it. +fn set_aside(path: &Path) -> Option { + let name = path.file_name()?.to_string_lossy().into_owned(); + let target = (1..=99) + .map(|n| path.with_file_name(format!("{name}.corrupt-{n}"))) + .find(|candidate| !candidate.exists())?; + match std::fs::rename(path, &target) { + Ok(()) => Some(target), + Err(e) => { + warn!(error = %e, path = %path.display(), "could not set the invalid config file aside"); + None + } + } } /// Persist `value` to `~/.termherd/`. Failures are logged, never fatal — @@ -80,3 +184,66 @@ pub fn update_json(file: &str, edit: impl FnOnce(&mut Map)) { fn config_path(file: &str) -> Option { Some(crate::paths::termherd_dir()?.join(file)) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn an_unparseable_file_is_set_aside_and_reported() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("settings.json"); + std::fs::write(&path, "{ \"theme\": ").expect("write"); + + let (value, problem) = load_json_at::(&path); + + assert_eq!(value, Value::Null, "the default is adopted"); + let kept = dir.path().join("settings.json.corrupt-1"); + assert_eq!( + problem, + Some(LoadProblem { + file: "settings.json".to_string(), + kept_as: Some(kept.clone()), + }) + ); + assert!(!path.exists(), "a later save starts a fresh file"); + assert_eq!( + std::fs::read_to_string(&kept).expect("kept"), + "{ \"theme\": ", + "the user's bytes survive verbatim" + ); + } + + #[test] + fn a_second_bad_file_does_not_overwrite_the_first_backup() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("settings.json"); + std::fs::write(dir.path().join("settings.json.corrupt-1"), "first").expect("write"); + std::fs::write(&path, "second").expect("write"); + + let (_, problem) = load_json_at::(&path); + + assert_eq!( + problem.and_then(|p| p.kept_as), + Some(dir.path().join("settings.json.corrupt-2")) + ); + assert_eq!( + std::fs::read_to_string(dir.path().join("settings.json.corrupt-1")).expect("first"), + "first" + ); + } + + #[test] + fn a_missing_file_is_no_problem_and_a_valid_one_loads() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("settings.json"); + assert_eq!(load_json_at::(&path), (Value::Null, None)); + + std::fs::write(&path, "{\"a\": 1}").expect("write"); + assert_eq!( + load_json_at::(&path), + (serde_json::json!({ "a": 1 }), None) + ); + assert!(path.exists(), "a valid file stays put"); + } +} diff --git a/crates/app/src/main.rs b/crates/app/src/main.rs index 7c380eb..d22ea2a 100644 --- a/crates/app/src/main.rs +++ b/crates/app/src/main.rs @@ -59,8 +59,9 @@ fn main() -> iced::Result { // Thin user settings (FR10): the configured shell is injected into the PTY // host, the theme into the iced shell. A corrupt file falls back to - // defaults rather than blocking startup. - let settings = settings::Settings::load(); + // defaults rather than blocking startup — set aside, and reported in the + // sidebar, so the defaults never overwrite it. + let (settings, settings_problem) = settings::Settings::load_checked(); let shell = settings.shell.as_ref().map(|s| Shell { program: s.program.clone(), args: s.args.clone(), @@ -109,8 +110,12 @@ fn main() -> iced::Result { mcp_endpoint, mcp_tokens, }; - let startup = - shell::Startup::from_settings(&settings, metadata_store::load(), collapsed_store::load()); + let (metadata, metadata_problem) = metadata_store::load(); + let mut startup = shell::Startup::from_settings(&settings, metadata, collapsed_store::load()); + startup.load_problems = settings_problem + .into_iter() + .chain(metadata_problem) + .collect(); // Terminal path candidates are checked against the real filesystem — the // one thing that tells `src/main.rs` from prose like `and/or`. let path_resolver: Arc = Arc::new(termherd_scan::FsPathResolver::new()); diff --git a/crates/app/src/metadata_store.rs b/crates/app/src/metadata_store.rs index 65e43e2..50cba7d 100644 --- a/crates/app/src/metadata_store.rs +++ b/crates/app/src/metadata_store.rs @@ -74,10 +74,13 @@ impl<'de> Deserialize<'de> for StoredOverlay { } /// Load the overlay; any problem (no file, bad JSON) yields an empty overlay — -/// metadata must never block startup. +/// metadata must never block startup. An unparseable file is set aside and +/// reported: stars, titles and added repos are the user's own work, and the +/// next save would otherwise overwrite them with the empty overlay. #[must_use] -pub fn load() -> Overlay { - from_dto(crate::json_store::load_json::(FILE).0) +pub fn load() -> (Overlay, Option) { + let (stored, problem) = crate::json_store::load_json_checked::(FILE); + (from_dto(stored.0), problem) } /// Persist the overlay. Failures are logged, never fatal. diff --git a/crates/app/src/settings.rs b/crates/app/src/settings.rs index 3467efd..2ae03dd 100644 --- a/crates/app/src/settings.rs +++ b/crates/app/src/settings.rs @@ -570,6 +570,13 @@ impl Settings { pub fn load() -> Self { crate::json_store::load_json(FILE) } + + /// The startup load: an unparseable file is set aside and reported rather + /// than silently replaced by defaults the next save would write over it. + #[must_use] + pub fn load_checked() -> (Self, Option) { + crate::json_store::load_json_checked(FILE) + } } /// One appearance key the settings panel changed. Each pick persists only its diff --git a/crates/app/src/shell.rs b/crates/app/src/shell.rs index 6b8ff59..e92211a 100644 --- a/crates/app/src/shell.rs +++ b/crates/app/src/shell.rs @@ -119,6 +119,9 @@ pub struct Startup { /// Adapter-owned config bits for the MCP `snapshot` tool's config section /// (the live font size is stamped by `core`, not carried here). pub config: ConfigInput, + /// Config files that existed but could not be used at startup, shown in + /// the sidebar until dismissed. + pub load_problems: Vec, } impl Startup { @@ -149,6 +152,7 @@ impl Startup { keymap_overrides: settings.keys.len(), }, record, + load_problems: Vec::new(), } } } @@ -198,6 +202,7 @@ pub fn run( gestures: startup.gestures, open: startup.open.clone(), config: startup.config.clone(), + load_problems: startup.load_problems.clone(), }, ); let initial_scan = shell.rescan(); @@ -289,6 +294,8 @@ struct Shell { appearance: appearance::Appearance, /// Whether the settings panel is open. settings_open: bool, + /// Config files that could not be used, until the user dismisses them. + load_problems: Vec, /// See [`Ports::appearance`]. appearance_sink: Arc, /// Configurable shortcut bindings (FR9). @@ -539,6 +546,8 @@ enum Message { ToggleSidebar, /// Open the settings panel, or close it when open. ToggleSettings, + /// Hide the notice about config files that could not be used. + DismissLoadProblems, /// Close the settings panel (its close button, the scrim, Escape). CloseSettings, /// Apply and persist a GUI chrome theme picked in the settings panel. @@ -743,6 +752,7 @@ impl Shell { colors: startup.colors, }, settings_open: false, + load_problems: startup.load_problems, appearance_sink, keymap: startup.keymap, renaming: None, @@ -1190,6 +1200,10 @@ impl Shell { self.perform(effects) } Message::ToggleSidebar => self.toggle_sidebar(), + Message::DismissLoadProblems => { + self.load_problems.clear(); + Task::none() + } Message::ToggleSettings => { self.settings_open = !self.settings_open; Task::none() @@ -1586,6 +1600,7 @@ mod key_routing { gestures: ClipboardGestures::default(), open: None, config: test_config_input(), + load_problems: Vec::new(), } } @@ -3835,6 +3850,27 @@ mod key_routing { (shell, sink) } + #[test] + fn a_startup_load_problem_is_kept_until_dismissed() { + let problem = crate::json_store::LoadProblem { + file: "settings.json".to_string(), + kept_as: Some(PathBuf::from("/home/u/.termherd/settings.json.corrupt-1")), + }; + let (_tx, rx) = iced::futures::channel::mpsc::unbounded::(); + let mut shell = Shell::new( + WindowConfig::default(), + test_ports(Arc::new(RecordingPty::default()), rx), + test_live_bridge(), + Startup { + load_problems: vec![problem.clone()], + ..test_startup() + }, + ); + assert_eq!(shell.load_problems, [problem]); + let _ = shell.update(Message::DismissLoadProblems); + assert!(shell.load_problems.is_empty()); + } + #[test] fn the_settings_chord_opens_the_panel_and_escape_closes_it() { let (mut shell, _pty) = shell_with_terminal(); diff --git a/crates/app/src/shell/view/sidebar.rs b/crates/app/src/shell/view/sidebar.rs index dff40d9..42f2d4c 100644 --- a/crates/app/src/shell/view/sidebar.rs +++ b/crates/app/src/shell/view/sidebar.rs @@ -142,7 +142,29 @@ impl Shell { .style(container::rounded_box), tooltip::Position::Bottom, ); - let chrome = column![ + let mut chrome = column![].spacing(8); + if !self.load_problems.is_empty() { + let mut notes = column![].spacing(4); + for problem in &self.load_problems { + notes = notes.push( + text(strings::config_unusable( + &problem.file, + problem.kept_as.as_deref(), + )) + .size(11), + ); + } + let dismiss = button(text(strings::DISMISS).size(11)) + .on_press(Message::DismissLoadProblems) + .style(button::text) + .padding(0); + chrome = chrome.push( + container(row![notes.width(Fill), dismiss].spacing(6)) + .padding(6) + .style(container::bordered_box), + ); + } + let chrome = chrome.extend([column![ row![ hide, iced::widget::Space::new().width(Fill), @@ -155,7 +177,8 @@ impl Shell { titles_only, show_archived ] - .spacing(8); + .spacing(8) + .into()]); container(chrome.push(scrollable(list).height(Fill)).padding(8)) .width(300) .style(container::rounded_box) diff --git a/crates/app/src/strings.rs b/crates/app/src/strings.rs index b68de3d..a736ec4 100644 --- a/crates/app/src/strings.rs +++ b/crates/app/src/strings.rs @@ -102,6 +102,19 @@ pub fn quit_prompt(live: usize) -> String { } } +/// Sidebar notice for a config file that existed and could not be used. +#[must_use] +pub fn config_unusable(file: &str, kept_as: Option<&std::path::Path>) -> String { + match kept_as { + Some(kept) => format!( + "{file} could not be read; defaults are in use. Your file was kept as {}.", + kept.display() + ), + None => format!("{file} could not be read; defaults are in use."), + } +} +pub const DISMISS: &str = "✕"; + // --- Settings panel --- pub const SETTINGS_TITLE: &str = "Settings"; pub const SETTINGS_THEME: &str = "Interface theme"; diff --git a/docs/src/reference/settings.md b/docs/src/reference/settings.md index 981e2b6..ed446e0 100644 --- a/docs/src/reference/settings.md +++ b/docs/src/reference/settings.md @@ -20,6 +20,10 @@ defensively: - Every field is optional. A missing file, a missing field, or a corrupt file falls back to built-in defaults — settings never block startup. +- A file that is not valid JSON is **set aside** as `settings.json.corrupt-1` + (then `-2`, …) and the sidebar says so, so nothing TermHerd saves later can + overwrite what you wrote. Fix the copy and move it back. `metadata.json` is + protected the same way. - Out-of-range values **clamp** instead of failing the file. - One bad value — a typo'd colour, an unknown action name — **degrades alone** with a logged warning. The rest of the file still applies. From 2b4f0232637b274260d3d1831c00c547be7c351d Mon Sep 17 00:00:00 2001 From: Guillaume Jay Date: Mon, 5 Oct 2026 10:16:31 +0200 Subject: [PATCH 4/6] feat(settings): reload settings.json on save, so no change needs a restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit settings.json was read once at startup, so every change — even a cosmetic one — cost a restart and every running session with it. The file is now watched (its directory, non-recursively, filtered to the one name, so an editor's write-temp-then-rename save is seen) and each burst of writes re-applies it through the same wide-parse-then-clamp path as startup. The chrome and every open terminal follow at once; keys, close policy, gestures, sidebar limit, open command and font size apply from the next use; the shell from the next session (PtyManager::set_shell), the record budget from the next recording. A file that does not parse mid-run — an editor's half-written save — changes nothing and is never set aside: the app does not fall back to defaults while running. Re-applying the settings in force is a no-op, so the settings panel's own writes echoing back through the watch do not recolour anything twice, and the panel now reflects an edit made to the file (or by MCP set_option) while it is open. Closes #297 --- .roadmap/features/f-settings.md | 5 +- README.md | 18 ++-- ROADMAP.md | 5 +- crates/app/src/json_store.rs | 45 +++++++++ crates/app/src/main.rs | 7 +- crates/app/src/settings.rs | 24 +++++ crates/app/src/shell.rs | 136 +++++++++++++++++++++---- crates/app/src/shell/appearance.rs | 45 ++------- crates/app/src/shell/live_settings.rs | 139 ++++++++++++++++++++++++++ crates/app/src/shell/record.rs | 7 ++ crates/pty/src/manager.rs | 23 ++++- crates/scan/src/lib.rs | 2 +- crates/scan/src/watch.rs | 84 +++++++++++++++- docs/settings.example.jsonc | 3 +- docs/src/mcp/stdio.md | 2 +- docs/src/reference/settings.md | 31 ++++-- docs/src/workspace/settings-panel.md | 5 +- 17 files changed, 490 insertions(+), 91 deletions(-) create mode 100644 crates/app/src/shell/live_settings.rs diff --git a/.roadmap/features/f-settings.md b/.roadmap/features/f-settings.md index 5268c9a..223a353 100644 --- a/.roadmap/features/f-settings.md +++ b/.roadmap/features/f-settings.md @@ -12,5 +12,6 @@ Shell select, theme, window prefs (M3): `~/.termherd/settings.json` (serde, defaults on missing/corrupt) carries a shell profile (program + args), injected into the `PtyManager` so each session launches the chosen shell, and a GUI theme (dark/light) wired to the iced chrome. (thin) Window bounds keep their -own `window.json` (FR12). The appearance is now also set live from -[F-settings-panel](#f-settings-panel); the rest stays file-based +own `window.json` (FR12). The file is reloaded on save (#297) — +every setting applies without a restart, the shell from the next session — +and the appearance is also set from [F-settings-panel](#f-settings-panel) diff --git a/README.md b/README.md index caf8ae7..c85fac8 100644 --- a/README.md +++ b/README.md @@ -113,15 +113,15 @@ cargo run -p termherd-app ## Configuration Optional user settings live in `~/.termherd/settings.json` (on Windows, -`%USERPROFILE%\.termherd\settings.json`). The file is read at startup; if it -is missing or invalid, TermHerd falls back to defaults rather than refusing -to start (an invalid file is first set aside as `settings.json.corrupt-1`, -and the sidebar says so). Out-of-range values clamp, and a single bad value -(a typo'd colour, an unknown key action) degrades alone with a logged -warning instead of resetting the rest of the file. The appearance (chrome -theme and terminal colours) can also be changed live from the settings -panel — ⚙ in the sidebar, or `Cmd`/`Ctrl`+`,` — which saves it here; -everything else: edit the file and restart. +`%USERPROFILE%\.termherd\settings.json`). A change **applies as soon as +the file is saved** — no restart; a save that does not parse changes +nothing. If the file is invalid at startup, TermHerd falls back to defaults +rather than refusing to start (the file is first set aside as +`settings.json.corrupt-1`, and the sidebar says so). Out-of-range values +clamp, and a single bad value (a typo'd colour, an unknown key action) +degrades alone with a logged warning instead of resetting the rest of the +file. The appearance (chrome theme and terminal colours) can also be set +from the settings panel — ⚙ in the sidebar, or `Cmd`/`Ctrl`+`,`. The annotated reference template is [`docs/settings.example.jsonc`](docs/settings.example.jsonc): every option diff --git a/ROADMAP.md b/ROADMAP.md index 85d6a20..a378962 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -410,8 +410,9 @@ Shell select, theme, window prefs (M3): `~/.termherd/settings.json` (serde, defaults on missing/corrupt) carries a shell profile (program + args), injected into the `PtyManager` so each session launches the chosen shell, and a GUI theme (dark/light) wired to the iced chrome. (thin) Window bounds keep their -own `window.json` (FR12). The appearance is now also set live from -[F-settings-panel](#f-settings-panel); the rest stays file-based +own `window.json` (FR12). The file is reloaded on save (#297) — +every setting applies without a restart, the shell from the next session — +and the appearance is also set from [F-settings-panel](#f-settings-panel) diff --git a/crates/app/src/json_store.rs b/crates/app/src/json_store.rs index afeda67..632c6b3 100644 --- a/crates/app/src/json_store.rs +++ b/crates/app/src/json_store.rs @@ -50,6 +50,34 @@ pub fn load_json(file: &str) -> T { } } +/// Re-read `~/.termherd/` while the app runs: `Some` only when it parses. +/// A missing, unreadable or invalid file — an editor's half-written save +/// included — is `None`, so the caller keeps what it has rather than falling +/// back to defaults mid-run. Touches nothing on disk. +#[must_use] +pub fn reload_json(file: &str) -> Option { + reload_at(&config_path(file)?) +} + +/// The body of [`reload_json`], on an explicit path. +fn reload_at(path: &Path) -> Option { + match read_at(path) { + Read::Parsed(value) => Some(value), + Read::Missing => { + warn!(path = %path.display(), "config file gone; keeping the running values"); + None + } + Read::Unreadable(e) => { + warn!(error = %e, path = %path.display(), "unreadable config file; keeping the running values"); + None + } + Read::Invalid(e) => { + warn!(error = %e, path = %path.display(), "invalid config file; keeping the running values"); + None + } + } +} + /// The startup load: [`load_json`], but a file that does not parse is set /// aside (see [`set_aside`]) and reported, so the defaults adopted in its place /// can be saved without destroying it. Startup only — a later read may see a @@ -233,6 +261,23 @@ mod tests { ); } + #[test] + fn a_reload_keeps_the_running_values_on_anything_but_a_parse_and_moves_nothing() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("settings.json"); + assert_eq!(reload_at::(&path), None, "missing"); + + std::fs::write(&path, "{ \"theme\": ").expect("write"); + assert_eq!(reload_at::(&path), None, "half-written"); + assert!(path.exists(), "a mid-run read never sets a file aside"); + + std::fs::write(&path, "{\"theme\": \"light\"}").expect("write"); + assert_eq!( + reload_at::(&path), + Some(serde_json::json!({ "theme": "light" })) + ); + } + #[test] fn a_missing_file_is_no_problem_and_a_valid_one_loads() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/app/src/main.rs b/crates/app/src/main.rs index d22ea2a..c29d422 100644 --- a/crates/app/src/main.rs +++ b/crates/app/src/main.rs @@ -29,7 +29,7 @@ mod window_geometry; use std::sync::Arc; use termherd_core::ports::{PathResolver, ProjectScanner, ScanError}; -use termherd_pty::{EventSink, PtyEvent, PtyManager, Shell}; +use termherd_pty::{EventSink, PtyEvent, PtyManager}; use termherd_scan::FsScanner; use tracing::{info, warn}; @@ -62,10 +62,7 @@ fn main() -> iced::Result { // defaults rather than blocking startup — set aside, and reported in the // sidebar, so the defaults never overwrite it. let (settings, settings_problem) = settings::Settings::load_checked(); - let shell = settings.shell.as_ref().map(|s| Shell { - program: s.program.clone(), - args: s.args.clone(), - }); + let shell = settings.shell_profile(); // PTY output flows from the reader threads through this channel into the // iced subscription (M2). The manager is built here and injected as a diff --git a/crates/app/src/settings.rs b/crates/app/src/settings.rs index 2ae03dd..0db5988 100644 --- a/crates/app/src/settings.rs +++ b/crates/app/src/settings.rs @@ -571,6 +571,23 @@ impl Settings { crate::json_store::load_json(FILE) } + /// Re-read the file while the app runs. `None` when it does not parse, + /// so the running settings stay as they are. + #[must_use] + pub fn reload() -> Option { + crate::json_store::reload_json(FILE) + } + + /// The shell to launch, as the PTY adapter takes it; `None` for the + /// platform default login shell. + #[must_use] + pub fn shell_profile(&self) -> Option { + self.shell.as_ref().map(|s| termherd_pty::Shell { + program: s.program.clone(), + args: s.args.clone(), + }) + } + /// The startup load: an unparseable file is set aside and reported rather /// than silently replaced by defaults the next save would write over it. #[must_use] @@ -632,6 +649,13 @@ fn apply_appearance( }; } +/// Where the settings file lives, for the live-reload watch. `None` without a +/// home directory. +#[must_use] +pub fn path() -> Option { + Some(crate::paths::termherd_dir()?.join(FILE)) +} + /// `~/.termherd/settings.json` — the app data dir from the PRD (§7). const FILE: &str = "settings.json"; diff --git a/crates/app/src/shell.rs b/crates/app/src/shell.rs index e92211a..470aa91 100644 --- a/crates/app/src/shell.rs +++ b/crates/app/src/shell.rs @@ -42,6 +42,7 @@ mod geometry; mod ime; mod input; mod launch; +mod live_settings; mod orchestrate; mod record; mod repos; @@ -145,18 +146,26 @@ impl Startup { close: settings.close, gestures: settings.clipboard_gestures(), open: settings.open_command(), - config: ConfigInput { - terminal_scheme: settings.terminal.colors.scheme.clone(), - record_fps: record.fps, - record_scale: record.scale, - keymap_overrides: settings.keys.len(), - }, + config: config_input(settings), record, load_problems: Vec::new(), } } } +/// The settings the MCP `snapshot` tool reports that `core` cannot read — +/// built in one place for startup and for a live reload, so the two cannot +/// report different things. +fn config_input(settings: &crate::settings::Settings) -> ConfigInput { + let record = settings.record_config(); + ConfigInput { + terminal_scheme: settings.terminal.colors.scheme.clone(), + record_fps: record.fps, + record_scale: record.scale, + keymap_overrides: settings.keys.len(), + } +} + pub fn run( scanner: Arc, watch_root: Option, @@ -186,7 +195,7 @@ pub fn run( path_resolver: path_resolver.clone(), pty: pty.clone(), pty_output: pty_output.clone(), - appearance: Arc::new(appearance::LiveAppearance(pty.clone())), + settings: Arc::new(live_settings::LiveSettings(pty.clone())), }, live_bridge.clone(), Startup { @@ -296,8 +305,8 @@ struct Shell { settings_open: bool, /// Config files that could not be used, until the user dismisses them. load_problems: Vec, - /// See [`Ports::appearance`]. - appearance_sink: Arc, + /// See [`Ports::settings`]. + settings_sink: Arc, /// Configurable shortcut bindings (FR9). keymap: Keymap, /// In-progress inline rename: `(session id, edit buffer)` (F-session-metadata). @@ -548,6 +557,8 @@ enum Message { ToggleSettings, /// Hide the notice about config files that could not be used. DismissLoadProblems, + /// `settings.json` changed on disk: reload and apply it. + SettingsFileChanged, /// Close the settings panel (its close button, the scrim, Escape). CloseSettings, /// Apply and persist a GUI chrome theme picked in the settings panel. @@ -703,9 +714,9 @@ pub(crate) struct Ports { pub(crate) path_resolver: Arc, pub(crate) pty: Arc, pub(crate) pty_output: PtyOutput, - /// Where the settings panel's picks go: the live terminal palette and - /// `settings.json`. - pub(crate) appearance: Arc, + /// Where a setting goes beyond the shell: the live terminal palette, the + /// next session's shell, and `settings.json`. + pub(crate) settings: Arc, } impl Shell { @@ -716,7 +727,7 @@ impl Shell { path_resolver, pty, pty_output, - appearance: appearance_sink, + settings: settings_sink, } = ports; let LiveBridge { requests: bridge_requests, @@ -753,7 +764,7 @@ impl Shell { }, settings_open: false, load_problems: startup.load_problems, - appearance_sink, + settings_sink, keymap: startup.keymap, renaming: None, tab_rename: None, @@ -1200,6 +1211,7 @@ impl Shell { self.perform(effects) } Message::ToggleSidebar => self.toggle_sidebar(), + Message::SettingsFileChanged => self.reload_settings(), Message::DismissLoadProblems => { self.load_problems.clear(); Task::none() @@ -1446,6 +1458,12 @@ impl Shell { subs.push(Subscription::run_with(root.clone(), watch_stream)); } subs.push(Subscription::run_with(self.pty_output.clone(), pty_stream)); + if let Some(file) = crate::settings::path() { + subs.push(Subscription::run_with( + file, + live_settings::settings_watch_stream, + )); + } subs.push(Subscription::run_with( self.bridge_requests.clone(), bridge::request_stream, @@ -1639,7 +1657,7 @@ mod key_routing { path_resolver: Arc::new(termherd_scan::FsPathResolver::new()), pty, pty_output: PtyOutput::new(rx), - appearance: Arc::new(()), + settings: Arc::new(()), } } @@ -3825,13 +3843,21 @@ mod key_routing { struct RecordingAppearance { repainted: std::sync::Mutex>, persisted: std::sync::Mutex>, + shells: std::sync::Mutex>>, } - impl appearance::AppearanceSink for RecordingAppearance { + impl live_settings::SettingsSink for RecordingAppearance { fn repaint(&self, palette: termherd_pty::Palette) { self.repainted.lock().expect("lock").push(palette); } + fn set_shell(&self, shell: Option) { + self.shells + .lock() + .expect("lock") + .push(shell.map(|s| s.program)); + } + fn persist(&self, change: &crate::settings::AppearanceChange) { self.persisted.lock().expect("lock").push(change.clone()); } @@ -3841,7 +3867,7 @@ mod key_routing { let sink = Arc::new(RecordingAppearance::default()); let (_tx, rx) = iced::futures::channel::mpsc::unbounded::(); let mut ports = test_ports(Arc::new(RecordingPty::default()), rx); - ports.appearance = sink.clone(); + ports.settings = sink.clone(); let startup = Startup { colors, ..test_startup() @@ -3850,6 +3876,82 @@ mod key_routing { (shell, sink) } + #[test] + fn a_reloaded_file_applies_every_live_setting() { + let (mut shell, sink) = shell_with_appearance(ColorSettings::default()); + let settings: crate::settings::Settings = serde_json::from_str( + r#"{ + "theme": "gruvbox-light", + "shell": { "program": "fish" }, + "keys": { "toggle-sidebar": "ctrl+alt+b" }, + "close": { "tab": "noConfirmation" }, + "sidebar": { "session_limit": 2 }, + "terminal": { + "font_size": 20, + "copy_on_select": true, + "colors": { "scheme": "gruvbox-light" } + }, + "record": { "fps": 12 } + }"#, + ) + .expect("valid settings"); + + let _ = shell.apply_settings(&settings); + + assert_eq!(shell.theme(), iced::Theme::GruvboxLight); + assert_eq!( + *sink.repainted.lock().expect("lock"), + [termherd_pty::Palette::named("gruvbox-light").expect("known scheme")] + ); + assert_eq!( + *sink.shells.lock().expect("lock"), + [Some("fish".to_string())] + ); + assert_eq!( + shell + .keymap + .lookup(&KeyChord::parse("ctrl+alt+b").expect("a test chord parses")), + Some(termherd_core::Action::ToggleSidebar) + ); + assert_eq!( + shell.close_confirm.tab, + crate::settings::ConfirmClose::NoConfirmation + ); + assert!(shell.gestures.copy_on_select); + assert_eq!(shell.core.sidebar.session_limit, 2); + assert!((shell.core.font_size() - 20.0).abs() < f32::EPSILON); + assert_eq!( + shell.config.terminal_scheme.as_deref(), + Some("gruvbox-light") + ); + assert_eq!(shell.config.record_fps, 12); + assert!( + sink.persisted.lock().expect("lock").is_empty(), + "a reload reads the file; writing it back would loop" + ); + } + + #[test] + fn reapplying_the_settings_in_force_does_not_repaint() { + // The settings panel's own write comes back through the watch: it must + // be a no-op, not a second recolour of every terminal. + let (mut shell, sink) = shell_with_appearance(ColorSettings::default()); + let _ = shell.update(Message::PickScheme(appearance::SchemeChoice(Some( + "solarized-dark", + )))); + let echoed: crate::settings::Settings = + serde_json::from_str(r#"{ "terminal": { "colors": { "scheme": "solarized-dark" } } }"#) + .expect("valid settings"); + + let _ = shell.apply_settings(&echoed); + + assert_eq!( + sink.repainted.lock().expect("lock").len(), + 1, + "only the pick repainted" + ); + } + #[test] fn a_startup_load_problem_is_kept_until_dismissed() { let problem = crate::json_store::LoadProblem { diff --git a/crates/app/src/shell/appearance.rs b/crates/app/src/shell/appearance.rs index 7c73b49..d25f680 100644 --- a/crates/app/src/shell/appearance.rs +++ b/crates/app/src/shell/appearance.rs @@ -1,45 +1,15 @@ //! The settings panel's appearance controls: the chrome theme and the //! terminal scheme, each applied the moment it is picked and then persisted. //! The chrome follows on the next frame, since [`Shell::theme`] reads it every -//! frame; the terminals and the file follow through [`AppearanceSink`]. +//! frame; the terminals and the file follow through +//! [`super::live_settings::SettingsSink`]. use std::fmt; -use std::sync::Arc; -use termherd_pty::{Palette, PtyManager}; +use termherd_pty::Palette; use super::Shell; -use crate::settings::{AppearanceChange, ColorSettings, ThemeChoice, save_appearance}; - -/// Where a picked appearance goes beyond the shell's own state. A port of its -/// own rather than a [`termherd_core::ports::PtyHost`] method: a palette is -/// RGB, which `core` never sees. Injected so a shell under test neither -/// recolours real terminals nor rewrites the user's `settings.json`. -pub(crate) trait AppearanceSink: Send + Sync { - /// Recolour every terminal, running and future. - fn repaint(&self, palette: Palette); - /// Persist one picked key. - fn persist(&self, change: &AppearanceChange); -} - -/// The real sink: the PTY manager's live palette and `settings.json`. -pub(crate) struct LiveAppearance(pub(crate) Arc); - -impl AppearanceSink for LiveAppearance { - fn repaint(&self, palette: Palette) { - self.0.set_palette(palette); - } - - fn persist(&self, change: &AppearanceChange) { - save_appearance(change); - } -} - -/// Goes nowhere — the double for a shell whose test does not look. -impl AppearanceSink for () { - fn repaint(&self, _palette: Palette) {} - fn persist(&self, _change: &AppearanceChange) {} -} +use crate::settings::{AppearanceChange, ColorSettings, ThemeChoice}; /// What the settings panel edits. pub(super) struct Appearance { @@ -99,8 +69,7 @@ impl Shell { /// Apply a chrome theme and persist it. pub(super) fn pick_theme(&mut self, theme: ThemeChoice) { self.appearance.theme = theme; - self.appearance_sink - .persist(&AppearanceChange::Theme(theme)); + self.settings_sink.persist(&AppearanceChange::Theme(theme)); } /// Apply a terminal scheme to every session and persist it. @@ -108,9 +77,9 @@ impl Shell { let name = scheme.0.map(str::to_owned); self.appearance.colors.scheme.clone_from(&name); self.config.terminal_scheme = name; - self.appearance_sink + self.settings_sink .repaint(self.appearance.colors.to_palette()); - self.appearance_sink.persist(&AppearanceChange::Scheme( + self.settings_sink.persist(&AppearanceChange::Scheme( self.appearance.colors.scheme.clone(), )); } diff --git a/crates/app/src/shell/live_settings.rs b/crates/app/src/shell/live_settings.rs new file mode 100644 index 0000000..a8125cf --- /dev/null +++ b/crates/app/src/shell/live_settings.rs @@ -0,0 +1,139 @@ +//! `settings.json`, live: the port that carries a setting beyond the shell's +//! own state (the terminals' palette, the next session's shell, the file), +//! and the reload that re-applies the file when it changes on disk — whoever +//! wrote it: a text editor, an MCP `set_option`, the settings panel itself. +//! +//! A reload goes through the same wide-parse-then-clamp path as startup, so a +//! bad value degrades exactly as it does there. A file that does not parse at +//! all — typically one an editor is halfway through writing — leaves the +//! running settings untouched: the app never falls back to defaults mid-run. + +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; + +use iced::Task; +use iced::futures::{SinkExt, Stream, StreamExt}; +use termherd_pty::{Palette, PtyManager}; + +use super::{Message, Shell}; +use crate::settings::{AppearanceChange, Settings, save_appearance}; + +/// Quiet period before a burst of writes to the file triggers one reload — an +/// editor's save is often several events. +const RELOAD_DEBOUNCE: Duration = Duration::from_millis(200); + +/// Where a setting goes beyond the shell's own state. A port of its own rather +/// than [`termherd_core::ports::PtyHost`] methods: a palette is RGB and a shell +/// profile is adapter config, neither of which `core` sees. Injected so a shell +/// under test neither recolours real terminals nor rewrites the user's +/// `settings.json`. +pub(crate) trait SettingsSink: Send + Sync { + /// Recolour every terminal, running and future. + fn repaint(&self, palette: Palette); + /// Launch this shell from the next session on. + fn set_shell(&self, shell: Option); + /// Persist one key the settings panel picked. + fn persist(&self, change: &AppearanceChange); +} + +/// The real sink: the PTY manager and `settings.json`. +pub(crate) struct LiveSettings(pub(crate) Arc); + +impl SettingsSink for LiveSettings { + fn repaint(&self, palette: Palette) { + self.0.set_palette(palette); + } + + fn set_shell(&self, shell: Option) { + self.0.set_shell(shell); + } + + fn persist(&self, change: &AppearanceChange) { + save_appearance(change); + } +} + +/// Goes nowhere — the double for a shell whose test does not look. +impl SettingsSink for () { + fn repaint(&self, _palette: Palette) {} + fn set_shell(&self, _shell: Option) {} + fn persist(&self, _change: &AppearanceChange) {} +} + +impl Shell { + /// Re-read `settings.json` after it changed on disk and apply it. A file + /// that does not parse changes nothing. + pub(super) fn reload_settings(&mut self) -> Task { + match Settings::reload() { + Some(settings) => self.apply_settings(&settings), + None => Task::none(), + } + } + + /// Bring every live setting in line with `settings`. Idempotent: applying + /// the values already in force changes nothing visible, which is what + /// makes the settings panel's own writes — echoed back by the watch — + /// harmless. + pub(super) fn apply_settings(&mut self, settings: &Settings) -> Task { + self.appearance.theme = settings.theme; + let palette = settings.palette(); + if palette != self.appearance.colors.to_palette() { + self.settings_sink.repaint(palette); + } + self.appearance.colors = settings.terminal.colors.clone(); + self.keymap = settings.keymap(); + self.close_confirm = settings.close; + self.gestures = settings.clipboard_gestures(); + self.record.set_config(settings.record_config()); + self.config = super::config_input(settings); + self.settings_sink.set_shell(settings.shell_profile()); + + let font_before = self.core.font_size(); + for event in [ + termherd_core::Event::SessionLimitLoaded(settings.session_limit()), + termherd_core::Event::FontSizeLoaded(settings.font_size()), + termherd_core::Event::OpenCommandLoaded(settings.open_command()), + ] { + self.core.apply(event); + } + // A new base size re-derives every grid, as a zoom does. + if self.core.font_size() == font_before { + Task::none() + } else { + self.resize_panes() + } + } +} + +/// Watch `settings.json` and emit [`Message::SettingsFileChanged`] once per +/// burst of writes. If the watch cannot start, the file is simply read at the +/// next launch, as before (logged, not fatal). +// `&PathBuf` is imposed by `Subscription::run_with`, which passes `&D` to a +// plain fn pointer — `&Path` would not match `for<'a> fn(&'a D)`. +#[allow(clippy::ptr_arg)] +pub(super) fn settings_watch_stream(file: &PathBuf) -> impl Stream + use<> { + let file = file.clone(); + iced::stream::channel( + 4, + |mut output: iced::futures::channel::mpsc::Sender| async move { + let (tx, mut rx) = iced::futures::channel::mpsc::unbounded::<()>(); + match termherd_scan::watch_file(file, RELOAD_DEBOUNCE, move || { + let _ = tx.unbounded_send(()); + }) { + Ok(handle) => { + while rx.next().await.is_some() { + if output.send(Message::SettingsFileChanged).await.is_err() { + break; + } + } + drop(handle); + } + Err(error) => { + tracing::warn!(%error, "settings watch unavailable; changes apply at restart"); + iced::futures::future::pending::<()>().await; + } + } + }, + ) +} diff --git a/crates/app/src/shell/record.rs b/crates/app/src/shell/record.rs index 88f3161..fe1b2ac 100644 --- a/crates/app/src/shell/record.rs +++ b/crates/app/src/shell/record.rs @@ -55,6 +55,13 @@ impl RecordState { } } + /// Replace the budget from a reloaded `settings.json`. It governs the next + /// recording: one in progress keeps the throttle and encoder it started + /// with. + pub(super) fn set_config(&mut self, config: RecordConfig) { + self.config = config; + } + /// The frame cap `core` needs to decide the auto-stop. pub(super) fn max_frames(&self) -> u32 { self.config.max_frames() diff --git a/crates/pty/src/manager.rs b/crates/pty/src/manager.rs index a55c2fa..f4556c7 100644 --- a/crates/pty/src/manager.rs +++ b/crates/pty/src/manager.rs @@ -43,7 +43,9 @@ pub struct PtyManager { sessions: Mutex>, sink: EventSink, /// User-configured shell; `None` falls back to the platform default. - shell: Option, + /// Read when a session spawns, so [`PtyManager::set_shell`] reaches the + /// next session and leaves the running ones alone. + shell: RwLock>, /// The terminal colour scheme every session renders with, shared with /// each terminal thread so [`PtyManager::set_palette`] reaches them live. palette: SharedPalette, @@ -58,11 +60,20 @@ impl PtyManager { Self { sessions: Mutex::new(HashMap::new()), sink, - shell, + shell: RwLock::new(shell), palette: Arc::new(RwLock::new(palette)), } } + /// Launch `shell` (or the platform default for `None`) from the next + /// spawned session on. A running session keeps the program it started. + pub fn set_shell(&self, shell: Option) { + match self.shell.write() { + Ok(mut guard) => *guard = shell, + Err(poisoned) => *poisoned.into_inner() = shell, + } + } + /// Switch every session — running and future — to `palette`. Each live /// terminal re-emits its screen in the new colours; a later colour query /// (OSC 10/11) is answered from it too. A program that asked once at start @@ -124,7 +135,11 @@ impl PtyHost for PtyManager { // project directory with a sane TERM. Resuming a real Claude session // lands in a later slice; the id flows through so the adapter never has // to invent one (Q6). - let mut cmd = match &self.shell { + let configured = match self.shell.read() { + Ok(guard) => guard.clone(), + Err(poisoned) => poisoned.into_inner().clone(), + }; + let mut cmd = match &configured { Some(shell) => { let mut c = CommandBuilder::new(&shell.program); for arg in &shell.args { @@ -140,7 +155,7 @@ impl PtyHost for PtyManager { apply_terminal_env(&mut cmd); // Which shell is about to run decides the integration recipe; the // default program only resolves to a name here, so ask the builder. - let program = match &self.shell { + let program = match &configured { Some(shell) => shell.program.clone(), None => cmd.get_shell(), }; diff --git a/crates/scan/src/lib.rs b/crates/scan/src/lib.rs index 68a296e..e727254 100644 --- a/crates/scan/src/lib.rs +++ b/crates/scan/src/lib.rs @@ -49,7 +49,7 @@ use walk::scan_root; pub use paths::FsPathResolver; pub use repo::{normalize_repo_path, repo_root, sidebar_key}; -pub use watch::{WatchHandle, watch_changes}; +pub use watch::{WatchHandle, watch_changes, watch_file}; /// Scanner over a projects root (normally `~/.claude/projects`). pub struct FsScanner { diff --git a/crates/scan/src/watch.rs b/crates/scan/src/watch.rs index fa5ddb7..7be32d5 100644 --- a/crates/scan/src/watch.rs +++ b/crates/scan/src/watch.rs @@ -2,7 +2,7 @@ //! independent leaf: the notify/coalesce seam, with no dependency on the walk //! or its cache. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::Duration; use notify::{RecursiveMode, Watcher}; @@ -22,17 +22,64 @@ pub struct WatchHandle { pub fn watch_changes( root: PathBuf, debounce: Duration, + on_change: impl FnMut() + Send + 'static, +) -> Result { + watch( + &root, + RecursiveMode::Recursive, + |_| true, + debounce, + on_change, + ) +} + +/// Watch one file and invoke `on_change` once per debounced burst of events +/// that name it. The parent directory is what is watched, not the file: an +/// editor that saves by writing a temporary and renaming it over the original +/// replaces the inode a file watch would hold, and the file may not exist yet. +/// Events about its neighbours are ignored. +pub fn watch_file( + file: PathBuf, + debounce: Duration, + on_change: impl FnMut() + Send + 'static, +) -> Result { + let dir = file + .parent() + .map(Path::to_path_buf) + .ok_or_else(|| ScanError::Unreadable(format!("{}: no parent", file.display())))?; + let name = file.file_name().map(std::ffi::OsStr::to_os_string); + watch( + &dir, + RecursiveMode::NonRecursive, + move |event| { + event + .paths + .iter() + .any(|path| path.file_name().map(std::ffi::OsStr::to_os_string) == name) + }, + debounce, + on_change, + ) +} + +/// The shared body: watch `root` in `mode`, keep the events `relevant` admits, +/// and coalesce them into one `on_change` per quiet `debounce`. +fn watch( + root: &Path, + mode: RecursiveMode, + relevant: impl Fn(¬ify::Event) -> bool + Send + 'static, + debounce: Duration, mut on_change: impl FnMut() + Send + 'static, ) -> Result { let (tx, rx) = std::sync::mpsc::channel::<()>(); let mut watcher = notify::recommended_watcher(move |event: notify::Result| { - if event.is_ok() { + if event.is_ok_and(|event| relevant(&event)) { let _ = tx.send(()); } }) .map_err(|e| ScanError::Unreadable(format!("watcher: {e}")))?; watcher - .watch(&root, RecursiveMode::Recursive) + .watch(root, mode) .map_err(|e| ScanError::Unreadable(format!("{}: {e}", root.display())))?; std::thread::Builder::new() @@ -89,4 +136,35 @@ mod tests { extra + 1 ); } + + #[test] + fn a_file_watch_fires_for_its_file_and_ignores_its_neighbours() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("settings.json"); + let (tx, rx) = std::sync::mpsc::channel::<()>(); + let _handle = watch_file( + file.clone(), + std::time::Duration::from_millis(100), + move || { + let _ = tx.send(()); + }, + ) + .unwrap(); + + fs::write(tmp.path().join("metadata.json"), "{}").unwrap(); + assert!( + rx.recv_timeout(std::time::Duration::from_millis(800)) + .is_err(), + "a neighbour's write must not fire" + ); + + // Written as an editor saves: a temporary renamed over the target. + let staged = tmp.path().join("settings.json.tmp"); + fs::write(&staged, "{}").unwrap(); + fs::rename(&staged, &file).unwrap(); + assert!( + rx.recv_timeout(std::time::Duration::from_secs(10)).is_ok(), + "no change signal for the watched file within 10s" + ); + } } diff --git a/docs/settings.example.jsonc b/docs/settings.example.jsonc index 47ac444..ae8b1a3 100644 --- a/docs/settings.example.jsonc +++ b/docs/settings.example.jsonc @@ -10,7 +10,8 @@ // settings.json and strip the comments. // // ── Load behaviour ───────────────────────────────────────────────────────── -// The file is read once at startup (edit + restart; no in-app panel yet). +// The file is read at startup and again whenever it is saved: a change +// applies without a restart (a half-written save changes nothing). // Every field is optional: a missing file, a missing field, or a corrupt // file falls back to the built-in defaults — settings never block startup. // Out-of-range values clamp instead of failing the file, and one bad value diff --git a/docs/src/mcp/stdio.md b/docs/src/mcp/stdio.md index 96b8a1d..da07fd0 100644 --- a/docs/src/mcp/stdio.md +++ b/docs/src/mcp/stdio.md @@ -29,7 +29,7 @@ Add it to your `mcpServers` config, pointing `command` at the built binary: | Tool | Args | Does | | --- | --- | --- | | `list_options` | — | lists the configurable options with their current values | -| `set_option` | `id`, `value` | sets one **writable** option; the change lands in `settings.json` and applies on restart | +| `set_option` | `id`, `value` | sets one **writable** option; the change lands in `settings.json` and applies at once | Both speak the option **id** — a stable, dotted name: diff --git a/docs/src/reference/settings.md b/docs/src/reference/settings.md index ed446e0..804982f 100644 --- a/docs/src/reference/settings.md +++ b/docs/src/reference/settings.md @@ -5,8 +5,9 @@ %USERPROFILE%\.termherd\settings.json (Windows) ``` -The [settings panel](../workspace/settings-panel.md) changes the appearance -live and saves it here. Everything else: edit the file and restart. +**Changes apply as soon as the file is saved** — no restart, no session +lost. The [settings panel](../workspace/settings-panel.md) changes the +appearance from inside the app and saves it here. The annotated template with every option, its default and its meaning is [`docs/settings.example.jsonc`](https://github.com/Termherd/termherd/blob/main/docs/settings.example.jsonc) @@ -15,8 +16,9 @@ JSON**: no comments, no trailing commas. ## How it loads -Read once at startup — the settings panel's two keys aside — and -defensively: +Read at startup, then again each time the file changes on disk — whoever +wrote it: your editor, an MCP `set_option`, the settings panel. Defensively, +both times: - Every field is optional. A missing file, a missing field, or a corrupt file falls back to built-in defaults — settings never block startup. @@ -27,6 +29,23 @@ defensively: - Out-of-range values **clamp** instead of failing the file. - One bad value — a typo'd colour, an unknown action name — **degrades alone** with a logged warning. The rest of the file still applies. +- While the app runs, a file that does not parse — a save your editor has + only half written, a missing brace — **changes nothing**: the running + settings stay until the next valid save. Nothing falls back to defaults + mid-run, and nothing is set aside. + +What a live change reaches: + +| Setting | Applies to | +| --- | --- | +| `theme`, `terminal.colors` | the chrome and **every open terminal**, at once | +| `keys`, `close`, `sidebar`, `open`, `mcp`, `terminal.copy_on_select`, `terminal.paste_on_right_click` | the next keypress, click or call | +| `terminal.font_size` | every terminal, at once (your zoom steps are kept on top) | +| `shell` | the **next session** you open — a running one keeps its program | +| `record` | the **next recording** — one in progress keeps its budget | + +A program that read the terminal's colours when it started — Claude Code +picks its light or dark look that way — keeps its choice until it restarts. Two neighbouring files are TermHerd's, not yours to edit: `window.json` (size and position — a position left off every connected monitor is dropped, so the @@ -191,8 +210,8 @@ launch, so an agent may read them but never set them. The `close`, `sidebar`, `terminal.copy_on_select` and `terminal.paste_on_right_click` blocks are file-only for now; `keys` is published as a read-only resource. -A `set_option` write lands in `settings.json` and **applies on restart**, like -any other edit to the file. +A `set_option` write lands in `settings.json` and applies **as soon as it is +written**, like any other edit to the file. ## A complete example diff --git a/docs/src/workspace/settings-panel.md b/docs/src/workspace/settings-panel.md index 0764756..22de6b4 100644 --- a/docs/src/workspace/settings-panel.md +++ b/docs/src/workspace/settings-panel.md @@ -12,8 +12,9 @@ outside it closes it. └──────────────────────────────────────────────┘ ``` -Today it holds the appearance. The rest of `settings.json` is still edited in -the file — see [settings.json](../reference/settings.md). +Today it holds the appearance. The rest of `settings.json` is edited in the +file, which applies on save too — see [settings.json](../reference/settings.md). +An edit to the file while the panel is open shows in it at once. ## A pick applies at once, and saves itself From ae47d42bfbe31d5162a2895ccc3382b11139c0dc Mon Sep 17 00:00:00 2001 From: Guillaume Jay Date: Mon, 5 Oct 2026 10:21:50 +0200 Subject: [PATCH 5/6] feat(mcp): carry list_options and set_option on the live bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session termherd launched — the only kind it launches — reached the live bridge's seventeen tools and none about settings, so "switch me to a light theme" needed the stdio server registered by hand. The bridge now carries both tools over the stdio slice's pure catalogue (resolve_options / set_option in crates/mcp): one definition of the options, two transports. #[tool] accepts only a literal description, so the text is spelled on the bridge and pinned to the shared constants by a test. The write lands in settings.json, which the running shell's settings watch applies at once. The options tools take their file from the handler, so tests never reach the user's settings. Neither surface now writes over a settings.json that does not parse: the bridge goes through json_store::edit_json_at, which refuses a non-object file, and the stdio server refuses a write when its read failed to parse, where it used to set the one option on an empty object and discard the rest of the file. Closes #298 --- .roadmap/features/f-mcp-control-surface.md | 2 + .roadmap/features/f-mcp-options-bridge.md | 20 +++ AGENTS.md | 8 +- Cargo.lock | 1 + README.md | 12 +- ROADMAP.md | 20 +++ crates/app/Cargo.toml | 1 + crates/app/src/json_store.rs | 112 ++++++++---- crates/app/src/mcp/handler.rs | 198 ++++++++++++++++++++- crates/mcp/src/lib.rs | 12 +- crates/mcp/src/main.rs | 65 +++++-- docs/src/mcp/index.md | 15 +- docs/src/mcp/live-bridge.md | 14 ++ docs/src/mcp/stdio.md | 9 +- 14 files changed, 429 insertions(+), 60 deletions(-) create mode 100644 .roadmap/features/f-mcp-options-bridge.md diff --git a/.roadmap/features/f-mcp-control-surface.md b/.roadmap/features/f-mcp-control-surface.md index a1790b1..771fcb6 100644 --- a/.roadmap/features/f-mcp-control-surface.md +++ b/.roadmap/features/f-mcp-control-surface.md @@ -57,3 +57,5 @@ shippable: terminal half: a mouse event inside a session. Unblocked #155, now fixed. - [ ] [F-mcp-pointer-chrome](#f-mcp-pointer-chrome) — The pointer rung, chrome half: click and drag termherd's own interface. +- [x] [F-mcp-options-bridge](#f-mcp-options-bridge) — `list_options` and + `set_option` on the live bridge too, over the stdio slice's catalogue. diff --git a/.roadmap/features/f-mcp-options-bridge.md b/.roadmap/features/f-mcp-options-bridge.md new file mode 100644 index 0000000..19d4d8d --- /dev/null +++ b/.roadmap/features/f-mcp-options-bridge.md @@ -0,0 +1,20 @@ ++++ +id = "F-mcp-options-bridge" +type = "feature" +area = ["mcp"] +status = "done" +target = ["Could"] ++++ + +`list_options` and `set_option` on the live bridge, not only the stdio slice. + +Shipped as #298. A session termherd launched — the only kind it launches — +saw seventeen tools, none about settings, so "switch me to a light theme" +needed the stdio server registered by hand. The bridge now carries both over +the same pure catalogue in `crates/mcp` (one implementation, two transports; +the tool descriptions are pinned equal by a test, since `#[tool]` takes only +a literal). Neither surface writes over a `settings.json` that does not +parse. Paired with the settings hot reload (#297): this makes the change +reachable from a session, that one makes it visible without a restart. Rung +of [F-mcp-control-surface](#f-mcp-control-surface), after +[F-mcp-config-write](#f-mcp-config-write). diff --git a/AGENTS.md b/AGENTS.md index 12d5d3e..37fa2b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -149,13 +149,17 @@ into its `mcpServers` at spawn (loopback, per-session token) — so it can read and drive the workspace it runs in. This is the richer sibling of the capture dump above: same `WorkspaceSnapshot` model, live instead of a file. -**Settled.** Seventeen tools: `list_sessions` + `snapshot` +**Settled.** Nineteen tools: `list_sessions` + `snapshot` (perception), `open_session` / `split_pane` / `focus_pane` / `rename_tab` / `close_pane` / `run_in_session` / `mouse_in_session` (action), `wait_for_status` + `read_terminal` (synchronisation), `screenshot` (pixels), `press_keys` + `run_action` (the app's own keyboard), `add_repo` + `forget_repo` (membership — what the sidebar *contains*, as against what the -window draws), and `prompt_in_session` (the loop below, composed). The loop +window draws), `prompt_in_session` (the loop below, composed), and +`list_options` + `set_option` (settings — the stdio server's two tools over +its own pure catalogue in `crates/mcp`, writing `settings.json`, which the +shell's settings watch then applies live; the one pair that touches a file +rather than `core`, so it needs no bridge round trip). The loop they exist to serve is **act → wait → observe**: `run_in_session` returns immediately, so synchronise with `wait_for_status` and then `read_terminal`. Do **not** poll `snapshot` in a loop — it races the transition you are diff --git a/Cargo.lock b/Cargo.lock index b0c2e2d..516249f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3913,6 +3913,7 @@ dependencies = [ "tempfile", "termherd-claude", "termherd-core", + "termherd-mcp", "termherd-pty", "termherd-scan", "thiserror 2.0.18", diff --git a/README.md b/README.md index c85fac8..0629ba9 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,7 @@ nothing to configure. It exposes the running workspace: | `mouse_in_session` | a mouse event at a cell of a terminal — forwarded to a program reading the mouse, else the terminal's own selection | | `add_repo` · `forget_repo` | put a repository in the sidebar before it has any session, and drop that addition | | `prompt_in_session` | type, wait and read in one round trip — prompting another Claude session is opt-in | +| `list_options` · `set_option` | read and change `settings.json` — the same catalogue as the stdio server; a running TermHerd applies a change at once | The loop that makes it useful is **act → wait → observe**: `run_in_session`, then `wait_for_status`, then `read_terminal`. Sessions are addressed by a @@ -278,11 +279,12 @@ the sidebar, tabs or gutters. `termherd-mcp` is a separate small binary that exposes termherd's own **configuration** — so you can ask "what can I configure here?", or "switch me -to a light theme", from any Claude session. Two tools, `list_options` (read) -and `set_option` (write), plus the option **schema** as a resource, all -reflecting `~/.termherd/settings.json`. The two options that name what -termherd executes — `shell.program`, `shell.args` — are readable but never -writable over MCP. +to a light theme", from a Claude session termherd did **not** launch (one it +launched already has the same tools on the live bridge). Two tools, +`list_options` (read) and `set_option` (write), plus the option **schema** as +a resource, all reflecting `~/.termherd/settings.json`. The two options that +name what termherd executes — `shell.program`, `shell.args` — are readable but +never writable over MCP. It speaks JSON-RPC over stdio. Register it with Claude Code by adding it to your `mcpServers` config (point `command` at the built binary): diff --git a/ROADMAP.md b/ROADMAP.md index a378962..f6a4969 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -99,6 +99,7 @@ issues #18–#29. | [F-mcp-config-write](#f-mcp-config-write) | feature | mcp | ✅ | `set_option` and `keys` on the stateless stdio slice. | | [F-mcp-keys](#f-mcp-keys) | feature | mcp, keymap | ✅ | The keyboard rung: drive the app by key chords through the real keymap. | | [F-mcp-live-bridge](#f-mcp-live-bridge) | feature | mcp | ✅ | The gate: an in-process MCP server on loopback, reaching the live `core::App`. | +| [F-mcp-options-bridge](#f-mcp-options-bridge) | feature | mcp | ✅ | `list_options` and `set_option` on the live bridge, not only the stdio slice. | | [F-mcp-orchestration](#f-mcp-orchestration) | feature | mcp, workspace | ✅ | The action rung: six mutating tools, each over an existing `core::App` event. | | [F-mcp-pointer-terminal](#f-mcp-pointer-terminal) | feature | mcp, terminal | ✅ | The pointer rung, terminal half: place a mouse event inside a session. | | [F-mcp-screenshot](#f-mcp-screenshot) | feature | mcp, workspace | ✅ | The pixel rung: the window as a PNG, for what text cannot answer. | @@ -932,6 +933,8 @@ shippable: terminal half: a mouse event inside a session. Unblocked #155, now fixed. - [ ] [F-mcp-pointer-chrome](#f-mcp-pointer-chrome) — The pointer rung, chrome half: click and drag termherd's own interface. +- [x] [F-mcp-options-bridge](#f-mcp-options-bridge) — `list_options` and + `set_option` on the live bridge too, over the stdio slice's catalogue. @@ -1252,6 +1255,23 @@ Apache-2.0 (cargo-deny allows it), edition 2024, tokio-native (reuses the #192 runtime, now `enable_all` for the listener); the http stack is `hyper`/`hyper-util` (frugal base — no axum). Unblocks #194/#195/#196 + + +### F-mcp-options-bridge + +`list_options` and `set_option` on the live bridge, not only the stdio slice. + +Shipped as #298. A session termherd launched — the only kind it launches — +saw seventeen tools, none about settings, so "switch me to a light theme" +needed the stdio server registered by hand. The bridge now carries both over +the same pure catalogue in `crates/mcp` (one implementation, two transports; +the tool descriptions are pinned equal by a test, since `#[tool]` takes only +a literal). Neither surface writes over a `settings.json` that does not +parse. Paired with the settings hot reload (#297): this makes the change +reachable from a session, that one makes it visible without a restart. Rung +of [F-mcp-control-surface](#f-mcp-control-surface), after +[F-mcp-config-write](#f-mcp-config-write). + ### F-mcp-orchestration diff --git a/crates/app/Cargo.toml b/crates/app/Cargo.toml index c1f19f7..d465be6 100644 --- a/crates/app/Cargo.toml +++ b/crates/app/Cargo.toml @@ -57,6 +57,7 @@ termherd-core = { path = "../core" } termherd-claude = { path = "../claude" } termherd-scan = { path = "../scan" } termherd-pty = { path = "../pty" } +termherd-mcp = { path = "../mcp" } tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] } single-instance = "0.3" diff --git a/crates/app/src/json_store.rs b/crates/app/src/json_store.rs index 632c6b3..690c99d 100644 --- a/crates/app/src/json_store.rs +++ b/crates/app/src/json_store.rs @@ -165,47 +165,65 @@ pub fn save_json(file: &str, value: &T) { let Some(path) = config_path(file) else { return; }; - if let Some(dir) = path.parent() - && let Err(e) = std::fs::create_dir_all(dir) - { - warn!(error = %e, "could not create config dir"); - return; - } - match serde_json::to_string_pretty(value) { - Ok(json) => { - if let Err(e) = std::fs::write(&path, json) { - warn!(error = %e, path = %path.display(), "could not save config file"); - } - } - Err(e) => warn!(error = %e, path = %path.display(), "could not serialise config"), + if let Err(e) = write_json_at(&path, value) { + warn!(error = %e, "could not save config file"); } } /// Rewrite some keys of the JSON object in `~/.termherd/`, leaving every /// other key as found — including ones this build does not understand. A -/// missing file starts from an empty object; a file that does not parse as an -/// object is left alone (with a warning), since rewriting it would replace the -/// user's whole configuration with the few keys being set. +/// failure is logged, never fatal; see [`edit_json_at`] for what refuses. pub fn update_json(file: &str, edit: impl FnOnce(&mut Map)) { let Some(path) = config_path(file) else { return; }; - let mut root = match std::fs::read_to_string(&path) { - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Map::new(), - Err(e) => { - warn!(error = %e, path = %path.display(), "could not read config file; not updating it"); - return; + let result = edit_json_at(&path, |mut root| { + if let Value::Object(map) = &mut root { + edit(map); } - Ok(raw) => match serde_json::from_str(&raw) { - Ok(Value::Object(root)) => root, - _ => { - warn!(path = %path.display(), "config file is not a JSON object; not updating it"); - return; - } - }, - }; - edit(&mut root); - save_json(file, &root); + Ok(root) + }); + if let Err(e) = result { + warn!(error = %e, path = %path.display(), "could not update config file"); + } +} + +/// Read the JSON object at `path`, pass it through `edit`, and write back what +/// `edit` returns. A missing file starts from an empty object. A file that +/// exists but is not a JSON object is refused rather than rewritten: writing +/// the few keys being set onto an empty object would replace the user's whole +/// configuration. `edit` may refuse too; nothing is written then. +pub fn edit_json_at( + path: &Path, + edit: impl FnOnce(Value) -> Result, +) -> Result<(), String> { + let root = read_object_at(path)?; + let edited = edit(root)?; + write_json_at(path, &edited) +} + +/// The JSON object at `path`; an empty one when the file does not exist. +pub fn read_object_at(path: &Path) -> Result { + match read_at::(path) { + Read::Missing => Ok(Value::Object(Map::new())), + Read::Parsed(value) if value.is_object() => Ok(value), + Read::Parsed(_) | Read::Invalid(_) => Err(format!( + "{} is not a valid JSON object; fix it first", + path.display() + )), + Read::Unreadable(e) => Err(format!("could not read {}: {e}", path.display())), + } +} + +/// Pretty-print `value` to `path`, creating its directory. +fn write_json_at(path: &Path, value: &T) -> Result<(), String> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir) + .map_err(|e| format!("could not create {}: {e}", dir.display()))?; + } + let json = serde_json::to_string_pretty(value) + .map_err(|e| format!("could not encode {}: {e}", path.display()))?; + std::fs::write(path, json).map_err(|e| format!("could not write {}: {e}", path.display())) } /// `~/.termherd/` — the app data dir from the PRD (§7). @@ -278,6 +296,38 @@ mod tests { ); } + #[test] + fn an_edit_refuses_a_file_that_is_not_an_object_and_writes_nothing() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("settings.json"); + std::fs::write(&path, "{ broken").expect("write"); + + let result = edit_json_at(&path, |_| Ok(serde_json::json!({ "theme": "light" }))); + + assert!(result.is_err()); + assert_eq!(std::fs::read_to_string(&path).expect("read"), "{ broken"); + } + + #[test] + fn an_edit_starts_a_missing_file_and_an_edit_that_refuses_writes_nothing() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("settings.json"); + + let refused = edit_json_at(&path, |_| Err("no".to_string())); + assert_eq!(refused, Err("no".to_string())); + assert!(!path.exists()); + + edit_json_at(&path, |mut root| { + root["theme"] = serde_json::json!("light"); + Ok(root) + }) + .expect("written"); + assert_eq!( + read_object_at(&path).expect("object"), + serde_json::json!({ "theme": "light" }) + ); + } + #[test] fn a_missing_file_is_no_problem_and_a_valid_one_loads() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/app/src/mcp/handler.rs b/crates/app/src/mcp/handler.rs index b72c72d..2d6cc8e 100644 --- a/crates/app/src/mcp/handler.rs +++ b/crates/app/src/mcp/handler.rs @@ -6,7 +6,13 @@ //! the timeout-bounded [`Reply`], and shapes it back into an MCP result. No state //! and no `core` access live here; the shell owns `core::App` and answers on the //! bridge, so a tool can never touch it directly. +//! +//! The two options tools are the exception that proves it: `settings.json` is +//! a file, not `core` state, so they read and write it directly — through the +//! same pure catalogue as the stdio server — and the running shell picks the +//! write up through its settings watch like any other edit. +use std::path::PathBuf; use std::time::Duration; use rmcp::{ @@ -72,6 +78,9 @@ const READ_BACK_TIMEOUT: Duration = Duration::from_millis(250); #[derive(Clone)] pub struct TermherdMcp { bridge: BridgeHandle, + /// The `settings.json` the options tools read and write; `None` without a + /// home directory. + settings_file: Option, tool_router: ToolRouter, } @@ -82,10 +91,68 @@ impl TermherdMcp { pub fn new(bridge: BridgeHandle) -> Self { Self { bridge, + settings_file: crate::settings::path(), tool_router: Self::tool_router(), } } + /// Point the options tools at another file — a test's own, so a call never + /// reaches the user's settings. + #[cfg(test)] + fn with_settings_file(mut self, file: PathBuf) -> Self { + self.settings_file = Some(file); + self + } + + /// Every configurable option with its current value — the same catalogue + /// and resolution as the stdio server's tool of the same name. + #[tool( + name = "list_options", + description = "List termherd's configurable options with their current values." + )] + async fn list_options(&self) -> Result { + let file = self.settings_file()?; + let settings = crate::json_store::read_object_at(file) + .map_err(|reason| ErrorData::internal_error(reason, None))?; + structured(serde_json::json!({ + "options": termherd_mcp::resolve_options(&settings), + })) + } + + /// Set one writable option. Validated by the shared catalogue; the running + /// shell applies the write when its settings watch sees it. + #[tool( + name = "set_option", + description = "Set one writable termherd option by id (see `writable` in list_options); the change lands in settings.json, which a running termherd applies at once. A read-only option or an out-of-shape value is refused, nothing written." + )] + async fn set_option( + &self, + Parameters(args): Parameters, + ) -> Result { + let file = self.settings_file()?; + // A refusal by the catalogue is the caller's to fix; anything else is + // the file's. Told apart so a model reads which one it is. + let mut refused = None; + crate::json_store::edit_json_at(file, |settings| { + termherd_mcp::set_option(&settings, &args.id, &args.value).map_err(|error| { + let reason = error.to_string(); + refused = Some(reason.clone()); + reason + }) + }) + .map_err(|reason| match refused.take() { + Some(reason) => ErrorData::invalid_params(reason, None), + None => ErrorData::internal_error(reason, None), + })?; + structured(serde_json::json!({ "id": args.id, "value": args.value })) + } + + fn settings_file(&self) -> Result<&PathBuf, ErrorData> { + self.settings_file.as_ref().ok_or_else(|| { + ErrorData::internal_error("no home directory; there is no settings.json", None) + }) + } + /// The spike tool: list the live terminal sessions termherd is hosting, each /// with its stable handle, so an MCP client can address them. #[tool( @@ -1213,6 +1280,16 @@ fn unknown_action(name: &str) -> ErrorData { ) } +/// Arguments for `set_option`. +#[derive(Debug, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct SetOptionArgs { + /// The option id, e.g. "theme" — see `list_options`. + id: String, + /// The new value, typed per the option's `kind`; `null` unsets it. + value: serde_json::Value, +} + /// Arguments for `read_terminal`. #[derive(Debug, Default, Deserialize, JsonSchema)] #[schemars(crate = "rmcp::schemars")] @@ -1382,6 +1459,19 @@ mod tests { type SweepCall<'a> = std::pin::Pin> + 'a>>; + /// The sweep's options tools: they read and write a file, so they need no + /// reply from the shell. + fn options_sweep_case<'a>(mcp: &'a TermherdMcp, tool: &str) -> Option> { + match tool { + "list_options" => Some(Box::pin(mcp.list_options())), + "set_option" => Some(Box::pin(mcp.set_option(Parameters(SetOptionArgs { + id: "theme".into(), + value: serde_json::json!("light"), + })))), + _ => None, + } + } + /// The replies the shell must give for `tool`, and the call that drives it. /// /// One `match` states the tool list once, so a tool the sweep does not know @@ -1389,6 +1479,10 @@ mod tests { fn sweep_case<'a>(mcp: &'a TermherdMcp, tool: &str) -> (Vec, SweepCall<'a>) { use crate::shell::bridge::{ActionOutcome, ShotResult, TerminalRead, WaitOutcome}; + if let Some(call) = options_sweep_case(mcp, tool) { + return (Vec::new(), call); + } + let acted = || Reply::Acted(ActionOutcome::applied(Some("1".into()))); match tool { "list_sessions" => ( @@ -1548,9 +1642,11 @@ mod tests { .collect(); assert!(!tools.is_empty(), "an empty router would pass vacuously"); + // The options tools touch a file: never the user's. + let dir = tempfile::tempdir().expect("tempdir"); for tool in tools { let (handle, requests) = channel(); - let mcp = TermherdMcp::new(handle); + let mcp = TermherdMcp::new(handle).with_settings_file(dir.path().join("settings.json")); let (replies, call) = sweep_case(&mcp, &tool); let _shell = spawn_test_shell_seq(requests, replies.into_iter().map(Some).collect::>()); @@ -1567,6 +1663,106 @@ mod tests { } } + #[test] + fn the_options_tools_say_what_the_stdio_server_says() { + // `#[tool]` takes only a literal, so the text is spelled here and in + // `termherd_mcp`; this keeps the two surfaces from drifting apart. + let router = TermherdMcp::tool_router(); + for (name, expected) in [ + ("list_options", termherd_mcp::LIST_OPTIONS_DESCRIPTION), + ("set_option", termherd_mcp::SET_OPTION_DESCRIPTION), + ] { + let tool = router + .list_all() + .into_iter() + .find(|tool| tool.name == name) + .unwrap_or_else(|| panic!("{name} is on the bridge")); + assert_eq!(tool.description.as_deref(), Some(expected), "{name}"); + } + } + + fn options_mcp(dir: &tempfile::TempDir) -> TermherdMcp { + TermherdMcp::new(channel().0).with_settings_file(dir.path().join("settings.json")) + } + + #[tokio::test] + async fn set_option_writes_one_key_and_list_options_reads_it_back() { + let dir = tempfile::tempdir().expect("tempdir"); + let file = dir.path().join("settings.json"); + std::fs::write(&file, r#"{ "keys": { "copy": "ctrl+c" } }"#).expect("write"); + let mcp = options_mcp(&dir); + + mcp.set_option(Parameters(SetOptionArgs { + id: "terminal.colors.scheme".into(), + value: serde_json::json!("solarized-light"), + })) + .await + .expect("a writable option is set"); + + let written: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&file).expect("read")).expect("json"); + assert_eq!( + written, + serde_json::json!({ + "keys": { "copy": "ctrl+c" }, + "terminal": { "colors": { "scheme": "solarized-light" } } + }), + "the rest of the file is kept" + ); + let listed = mcp + .list_options() + .await + .expect("listed") + .structured_content + .expect("structured"); + let scheme = listed["options"] + .as_array() + .expect("array") + .iter() + .find(|o| o["id"] == "terminal.colors.scheme") + .expect("scheme option"); + assert_eq!(scheme["value"], "solarized-light"); + } + + #[tokio::test] + async fn set_option_refuses_what_the_catalogue_refuses_and_writes_nothing() { + let dir = tempfile::tempdir().expect("tempdir"); + let mcp = options_mcp(&dir); + for (id, value) in [ + ("theme", serde_json::json!("banana")), + ("shell.program", serde_json::json!("/bin/evil")), + ("no.such", serde_json::json!(1)), + ] { + let error = mcp + .set_option(Parameters(SetOptionArgs { + id: id.into(), + value, + })) + .await + .expect_err("refused"); + assert_eq!(error.code, rmcp::model::ErrorCode::INVALID_PARAMS, "{id}"); + } + assert!(!dir.path().join("settings.json").exists()); + } + + #[tokio::test] + async fn set_option_will_not_write_over_a_file_it_cannot_parse() { + let dir = tempfile::tempdir().expect("tempdir"); + let file = dir.path().join("settings.json"); + std::fs::write(&file, "{ broken").expect("write"); + + let error = options_mcp(&dir) + .set_option(Parameters(SetOptionArgs { + id: "theme".into(), + value: serde_json::json!("light"), + })) + .await + .expect_err("refused"); + + assert_eq!(error.code, rmcp::model::ErrorCode::INTERNAL_ERROR); + assert_eq!(std::fs::read_to_string(&file).expect("read"), "{ broken"); + } + #[tokio::test] async fn list_sessions_tool_surfaces_a_bridge_failure_as_an_error() { // No shell answering: the bridge closes, and the tool must report an diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 99792b4..41f33d9 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -46,6 +46,14 @@ pub struct OptionSpec { pub writable: bool, } +/// What `list_options` does, worded once for both surfaces that carry it (this +/// stdio server and the live bridge). +pub const LIST_OPTIONS_DESCRIPTION: &str = + "List termherd's configurable options with their current values."; + +/// What `set_option` does, worded once for both surfaces that carry it. +pub const SET_OPTION_DESCRIPTION: &str = "Set one writable termherd option by id (see `writable` in list_options); the change lands in settings.json, which a running termherd applies at once. A read-only option or an out-of-shape value is refused, nothing written."; + /// The option catalog — the single source of what the control surface exposes. /// Kept small for this first draft; `keys` (the keymap overrides) and the /// orchestration surface are deferred. @@ -362,12 +370,12 @@ fn tools_list_result() -> Value { "tools": [ { "name": "list_options", - "description": "List termherd's configurable options with their current values.", + "description": LIST_OPTIONS_DESCRIPTION, "inputSchema": { "type": "object", "properties": {}, "additionalProperties": false }, }, { "name": "set_option", - "description": "Set one writable termherd option by id (see `writable` in list_options); the change lands in settings.json and applies on restart. A read-only option or an out-of-shape value is refused, nothing written.", + "description": SET_OPTION_DESCRIPTION, "inputSchema": { "type": "object", "properties": { diff --git a/crates/mcp/src/main.rs b/crates/mcp/src/main.rs index ed4108e..226f50b 100644 --- a/crates/mcp/src/main.rs +++ b/crates/mcp/src/main.rs @@ -30,8 +30,8 @@ fn main() { continue; }; // Read settings fresh per request so a live edit is reflected without a - // restart; a missing or invalid file reads as "no options set". - let settings = load_settings(); + // restart; a missing file reads as "no options set". + let (settings, writable) = load_settings(); let reply = termherd_mcp::handle_message(&message, &settings); // Perform the write effect the pure layer described, before replying, // so a caller that reads back sees its own change. A write that fails @@ -40,7 +40,7 @@ fn main() { // exists to avoid). let mut response = reply.response; if let Some(new_settings) = reply.write_settings - && let Err(err) = store_settings(&new_settings) + && let Err(err) = writable.and_then(|()| store_settings(&new_settings)) { eprintln!("termherd-mcp: {err}"); response = write_error_response(&message, &err); @@ -70,14 +70,31 @@ fn write_error_response(message: &Value, detail: &str) -> Option { /// Read `~/.termherd/settings.json` into a JSON value, falling back to an empty /// object when it is missing or unreadable — `list_options` then reports every -/// option as unset rather than failing. -fn load_settings() -> Value { - let Some(path) = settings_path() else { - return json!({}); - }; - match std::fs::read_to_string(&path) { - Ok(raw) => serde_json::from_str(&raw).unwrap_or_else(|_| json!({})), - Err(_) => json!({}), +/// option as unset rather than failing. The second half says whether a write +/// may replace the file: not when it exists but does not parse, since writing +/// the one option set onto an empty object would discard everything else the +/// user wrote. +fn load_settings() -> (Value, Result<(), String>) { + match settings_path() { + Some(path) => load_settings_at(&path), + None => (json!({}), Ok(())), + } +} + +/// The body of [`load_settings`], on an explicit path. +fn load_settings_at(path: &std::path::Path) -> (Value, Result<(), String>) { + match std::fs::read_to_string(path) { + Ok(raw) => match serde_json::from_str::(&raw) { + Ok(value) if value.is_object() => (value, Ok(())), + _ => ( + json!({}), + Err(format!( + "{} is not a valid JSON object; fix it before setting options", + path.display() + )), + ), + }, + Err(_) => (json!({}), Ok(())), } } @@ -107,6 +124,32 @@ fn settings_path() -> Option { mod tests { use super::*; + #[test] + fn a_file_that_does_not_parse_is_read_as_empty_but_never_written_over() { + let dir = std::env::temp_dir().join(format!("termherd-mcp-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("dir"); + let path = dir.join("settings.json"); + + std::fs::write(&path, "{ broken").expect("write"); + let (settings, writable) = load_settings_at(&path); + assert_eq!(settings, json!({})); + assert!(writable.is_err(), "a write would discard the user's file"); + + std::fs::write(&path, r#"{ "theme": "dark" }"#).expect("write"); + assert_eq!( + load_settings_at(&path), + (json!({ "theme": "dark" }), Ok(())) + ); + + std::fs::remove_file(&path).expect("remove"); + assert_eq!( + load_settings_at(&path), + (json!({}), Ok(())), + "missing is writable" + ); + let _ = std::fs::remove_dir(&dir); + } + #[test] fn a_failed_write_becomes_an_error_for_the_same_request() { let req = json!({ diff --git a/docs/src/mcp/index.md b/docs/src/mcp/index.md index ace7a6f..566d8b1 100644 --- a/docs/src/mcp/index.md +++ b/docs/src/mcp/index.md @@ -13,8 +13,8 @@ depends on how the session started. loopback │ └───────────▲────────────┘ │ ┌──────┼──────────────┘ │ │ │ the LIVE BRIDGE │ - │ │ 17 tools · the running │ - │ │ workspace │ + │ │ 19 tools · the running │ + │ │ workspace + settings.json │ │ └──────────────────────────────┘ │ │ wired in at spawn, per-session token @@ -33,11 +33,11 @@ depends on how the session started. | | [The live bridge](./live-bridge.md) | [The stdio server](./stdio.md) | | --- | --- | --- | -| Reaches | the **running** workspace | the **settings file** | +| Reaches | the **running** workspace and the settings file | the **settings file** | | Setup | none — wired in at spawn | you register `termherd-mcp` yourself | | Available to | sessions launched from TermHerd | any Claude session | | Transport | in-process, loopback, per-session bearer token | JSON-RPC over stdio | -| Surface | 17 tools | 2 tools + 2 resources | +| Surface | 19 tools | 2 tools + 2 resources | | Needs the app running | ✅ yes | ❌ no | ## The gap between them @@ -56,8 +56,11 @@ tools. - *"Split this pane and run the tests"* → **live bridge**. It only exists inside a session TermHerd started. -- *"Switch me to a light theme"* → **stdio server**. It edits the file; the - change applies on restart. +- *"Switch me to a light theme"* → either. Both carry `list_options` and + `set_option`, over one shared catalogue; the write lands in `settings.json` + and a running TermHerd applies it at once. From a session TermHerd started, + the bridge already has them — register the stdio server only for a session + started elsewhere. ## Design notes diff --git a/docs/src/mcp/live-bridge.md b/docs/src/mcp/live-bridge.md index 36bb579..36d7e02 100644 --- a/docs/src/mcp/live-bridge.md +++ b/docs/src/mcp/live-bridge.md @@ -156,6 +156,20 @@ current one, and `timed_out` is `true`. And a session that **exits** settles the wait whatever you asked for — it can no longer reach your target. Both behaviours exist so a wait can never silently park you. +### Settings + +| Tool | Args | Returns | +| --- | --- | --- | +| `list_options` | — | `{ options }` — every configurable option with its current value | +| `set_option` | `id`, `value` | `{ id, value }` — one writable option, written to `settings.json` | + +The same two tools as [the stdio server](./stdio.md), over the same catalogue — +the option ids, which ones are writable, and what each accepts are defined +once and read by both. The write lands in `settings.json`, and the running app +applies it the moment the file changes, as it does for any edit to the file. +A refused value is an `invalid_params` error; a `settings.json` that does not +parse is refused too, rather than overwritten. + ### The keyboard `press_keys` and `run_action` drive TermHerd's own interface — see diff --git a/docs/src/mcp/stdio.md b/docs/src/mcp/stdio.md index da07fd0..5060cad 100644 --- a/docs/src/mcp/stdio.md +++ b/docs/src/mcp/stdio.md @@ -6,7 +6,10 @@ to a light theme", from *any* Claude session, whether or not TermHerd is running. It speaks JSON-RPC over stdio and is stateless: it reads and writes -`~/.termherd/settings.json`, nothing else. +`~/.termherd/settings.json`, nothing else. A session TermHerd launched does not +need it — [the live bridge](./live-bridge.md#settings) carries the same two +tools over the same catalogue. Register this one for a session started +elsewhere. ## Registering it @@ -51,7 +54,9 @@ carry the `writable` flag per id, so a model can tell before it tries. `set_option` **refuses** rather than degrades: a read-only id, an unknown id, or a value that does not fit the option's kind (a non-array palette, a theme -outside `dark`/`light`) answers a JSON-RPC error and writes nothing. `null` is +outside the listed choices) answers a JSON-RPC error and writes nothing. So +does a `settings.json` that does not parse: writing the one option onto an +empty object would discard the rest of the file. `null` is always accepted on a writable id — it unsets the option. That is the whole write surface today. The `close`, `sidebar`, `record`, From 1931c976f53eb90050b13dde397ed26787e3b2d0 Mon Sep 17 00:00:00 2001 From: Guillaume Jay Date: Mon, 5 Oct 2026 20:10:18 +0200 Subject: [PATCH 6/6] refactor(settings): one settings.json writer, one path from file to shell - termherd_mcp::file owns the guarded read-modify-write of settings.json; the stdio server, the live bridge and the settings panel all set options through it, so the never-overwrite-an-unparseable-file rule exists once. - The panel persists through the catalogue setter instead of a hand-written JSON walk; tests pin every theme and scheme it offers as accepted. - Startup carries the parsed Settings and adopts them through the same adopt_settings a live reload uses, so no setting can apply at one and not the other. - PoisonError::into_inner instead of four hand-written lock matches; the settings path resolved once; no allocation per watch event; the contrast test derives its light themes from ThemeChoice::ALL. --- crates/app/src/json_store.rs | 81 +-------------- crates/app/src/main.rs | 14 ++- crates/app/src/mcp/handler.rs | 21 ++-- crates/app/src/settings.rs | 127 +++++++---------------- crates/app/src/shell.rs | 140 ++++++-------------------- crates/app/src/shell/appearance.rs | 9 +- crates/app/src/shell/live_settings.rs | 27 +++-- crates/app/src/shell/view/style.rs | 11 +- crates/mcp/src/file.rs | 133 ++++++++++++++++++++++++ crates/mcp/src/lib.rs | 5 + crates/mcp/src/main.rs | 41 ++------ crates/pty/src/manager.rs | 21 ++-- crates/pty/src/session.rs | 10 +- crates/scan/src/watch.rs | 2 +- 14 files changed, 273 insertions(+), 369 deletions(-) create mode 100644 crates/mcp/src/file.rs diff --git a/crates/app/src/json_store.rs b/crates/app/src/json_store.rs index 690c99d..160c4f7 100644 --- a/crates/app/src/json_store.rs +++ b/crates/app/src/json_store.rs @@ -12,7 +12,6 @@ use serde::Serialize; use serde::de::DeserializeOwned; -use serde_json::{Map, Value}; use std::path::{Path, PathBuf}; use tracing::warn; @@ -144,7 +143,7 @@ fn read_at(path: &Path) -> Read { /// Move an unparseable file to the first free `.corrupt-` beside /// it, so the next save starts a fresh file instead of overwriting the /// original. `None` when no name is free or the move fails — then the file -/// stays, and [`update_json`] still refuses to write over it. +/// stays, and the settings writer still refuses to write over it. fn set_aside(path: &Path) -> Option { let name = path.file_name()?.to_string_lossy().into_owned(); let target = (1..=99) @@ -170,51 +169,6 @@ pub fn save_json(file: &str, value: &T) { } } -/// Rewrite some keys of the JSON object in `~/.termherd/`, leaving every -/// other key as found — including ones this build does not understand. A -/// failure is logged, never fatal; see [`edit_json_at`] for what refuses. -pub fn update_json(file: &str, edit: impl FnOnce(&mut Map)) { - let Some(path) = config_path(file) else { - return; - }; - let result = edit_json_at(&path, |mut root| { - if let Value::Object(map) = &mut root { - edit(map); - } - Ok(root) - }); - if let Err(e) = result { - warn!(error = %e, path = %path.display(), "could not update config file"); - } -} - -/// Read the JSON object at `path`, pass it through `edit`, and write back what -/// `edit` returns. A missing file starts from an empty object. A file that -/// exists but is not a JSON object is refused rather than rewritten: writing -/// the few keys being set onto an empty object would replace the user's whole -/// configuration. `edit` may refuse too; nothing is written then. -pub fn edit_json_at( - path: &Path, - edit: impl FnOnce(Value) -> Result, -) -> Result<(), String> { - let root = read_object_at(path)?; - let edited = edit(root)?; - write_json_at(path, &edited) -} - -/// The JSON object at `path`; an empty one when the file does not exist. -pub fn read_object_at(path: &Path) -> Result { - match read_at::(path) { - Read::Missing => Ok(Value::Object(Map::new())), - Read::Parsed(value) if value.is_object() => Ok(value), - Read::Parsed(_) | Read::Invalid(_) => Err(format!( - "{} is not a valid JSON object; fix it first", - path.display() - )), - Read::Unreadable(e) => Err(format!("could not read {}: {e}", path.display())), - } -} - /// Pretty-print `value` to `path`, creating its directory. fn write_json_at(path: &Path, value: &T) -> Result<(), String> { if let Some(dir) = path.parent() { @@ -234,6 +188,7 @@ fn config_path(file: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use serde_json::Value; #[test] fn an_unparseable_file_is_set_aside_and_reported() { @@ -296,38 +251,6 @@ mod tests { ); } - #[test] - fn an_edit_refuses_a_file_that_is_not_an_object_and_writes_nothing() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("settings.json"); - std::fs::write(&path, "{ broken").expect("write"); - - let result = edit_json_at(&path, |_| Ok(serde_json::json!({ "theme": "light" }))); - - assert!(result.is_err()); - assert_eq!(std::fs::read_to_string(&path).expect("read"), "{ broken"); - } - - #[test] - fn an_edit_starts_a_missing_file_and_an_edit_that_refuses_writes_nothing() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("settings.json"); - - let refused = edit_json_at(&path, |_| Err("no".to_string())); - assert_eq!(refused, Err("no".to_string())); - assert!(!path.exists()); - - edit_json_at(&path, |mut root| { - root["theme"] = serde_json::json!("light"); - Ok(root) - }) - .expect("written"); - assert_eq!( - read_object_at(&path).expect("object"), - serde_json::json!({ "theme": "light" }) - ); - } - #[test] fn a_missing_file_is_no_problem_and_a_valid_one_loads() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/app/src/main.rs b/crates/app/src/main.rs index c29d422..569cb92 100644 --- a/crates/app/src/main.rs +++ b/crates/app/src/main.rs @@ -108,11 +108,15 @@ fn main() -> iced::Result { mcp_tokens, }; let (metadata, metadata_problem) = metadata_store::load(); - let mut startup = shell::Startup::from_settings(&settings, metadata, collapsed_store::load()); - startup.load_problems = settings_problem - .into_iter() - .chain(metadata_problem) - .collect(); + let startup = shell::Startup { + settings, + metadata, + collapsed: collapsed_store::load(), + load_problems: settings_problem + .into_iter() + .chain(metadata_problem) + .collect(), + }; // Terminal path candidates are checked against the real filesystem — the // one thing that tells `src/main.rs` from prose like `and/or`. let path_resolver: Arc = Arc::new(termherd_scan::FsPathResolver::new()); diff --git a/crates/app/src/mcp/handler.rs b/crates/app/src/mcp/handler.rs index 2d6cc8e..8eca1bc 100644 --- a/crates/app/src/mcp/handler.rs +++ b/crates/app/src/mcp/handler.rs @@ -33,6 +33,7 @@ use termherd_core::{ Action as KeymapAction, KeyChord, PointerButton, PointerEvent, PointerKind, PointerRoute, Section, SessionStatus, SnapshotFilter, TerminalScope, }; +use termherd_mcp::file::SetAtError; use crate::shell::bridge::{ Action, ActionDetail, BridgeHandle, CallError, Press, PressStep, Reply, Request, SessionInfo, @@ -112,7 +113,7 @@ impl TermherdMcp { )] async fn list_options(&self) -> Result { let file = self.settings_file()?; - let settings = crate::json_store::read_object_at(file) + let settings = termherd_mcp::file::read_object(file) .map_err(|reason| ErrorData::internal_error(reason, None))?; structured(serde_json::json!({ "options": termherd_mcp::resolve_options(&settings), @@ -132,18 +133,12 @@ impl TermherdMcp { let file = self.settings_file()?; // A refusal by the catalogue is the caller's to fix; anything else is // the file's. Told apart so a model reads which one it is. - let mut refused = None; - crate::json_store::edit_json_at(file, |settings| { - termherd_mcp::set_option(&settings, &args.id, &args.value).map_err(|error| { - let reason = error.to_string(); - refused = Some(reason.clone()); - reason - }) - }) - .map_err(|reason| match refused.take() { - Some(reason) => ErrorData::invalid_params(reason, None), - None => ErrorData::internal_error(reason, None), - })?; + termherd_mcp::file::set_option_at(file, &args.id, &args.value).map_err( + |error| match error { + SetAtError::Refused(_) => ErrorData::invalid_params(error.to_string(), None), + SetAtError::File(_) => ErrorData::internal_error(error.to_string(), None), + }, + )?; structured(serde_json::json!({ "id": args.id, "value": args.value })) } diff --git a/crates/app/src/settings.rs b/crates/app/src/settings.rs index 0db5988..7d2fa44 100644 --- a/crates/app/src/settings.rs +++ b/crates/app/src/settings.rs @@ -607,46 +607,30 @@ pub enum AppearanceChange { Scheme(Option), } -/// Persist one appearance change to `settings.json`, keeping the rest of the -/// file — explicit colour overrides included — as the user wrote it. -pub fn save_appearance(change: &AppearanceChange) { - crate::json_store::update_json(FILE, |root| apply_appearance(root, change)); -} - -/// The pure half of [`save_appearance`]. -fn apply_appearance( - root: &mut serde_json::Map, - change: &AppearanceChange, -) { - use serde_json::{Map, Value}; - let scheme = match change { - AppearanceChange::Theme(theme) => { - if let Ok(theme) = serde_json::to_value(theme) { - root.insert("theme".into(), theme); +impl AppearanceChange { + /// The catalogue option this change sets, and its value: the panel writes + /// through the same validated setter as both MCP surfaces. + fn option(&self) -> (&'static str, serde_json::Value) { + match self { + AppearanceChange::Theme(theme) => ("theme", serde_json::json!(theme)), + AppearanceChange::Scheme(scheme) => { + ("terminal.colors.scheme", serde_json::json!(scheme)) } - return; - } - AppearanceChange::Scheme(scheme) => scheme, - }; - // Descend into `terminal.colors`, replacing a level that is not an object: - // it could not have parsed as one, so nothing readable is lost. - let mut level = root; - for key in ["terminal", "colors"] { - let entry = level - .entry(key) - .or_insert_with(|| Value::Object(Map::new())); - if !entry.is_object() { - *entry = Value::Object(Map::new()); } - let Value::Object(next) = entry else { - return; - }; - level = next; } - match scheme { - Some(name) => level.insert("scheme".into(), Value::String(name.clone())), - None => level.remove("scheme"), +} + +/// Persist one appearance change to `settings.json`, keeping the rest of the +/// file — explicit colour overrides included — as the user wrote it. A +/// failure is logged, never fatal. +pub fn save_appearance(change: &AppearanceChange) { + let Some(path) = path() else { + return; }; + let (id, value) = change.option(); + if let Err(e) = termherd_mcp::file::set_option_at(&path, id, &value) { + warn!(error = %e, path = %path.display(), "could not save the appearance"); + } } /// Where the settings file lives, for the live-reload watch. `None` without a @@ -1046,68 +1030,29 @@ mod tests { } } - #[test] - fn saving_the_appearance_keeps_every_other_key() { - let mut root = serde_json::json!({ - "theme": "dark", - "shell": { "program": "pwsh" }, - "terminal": { "font_size": 16, "colors": { "scheme": "gruvbox-dark", "cursor": "#ff0000" } }, - "future-key": [1, 2] - }); - let map = root.as_object_mut().expect("object"); - apply_appearance(map, &AppearanceChange::Theme(ThemeChoice::SolarizedLight)); - apply_appearance( - map, - &AppearanceChange::Scheme(Some("solarized-light".to_string())), - ); - assert_eq!( - root, - serde_json::json!({ - "theme": "solarized-light", - "shell": { "program": "pwsh" }, - "terminal": { "font_size": 16, "colors": { "scheme": "solarized-light", "cursor": "#ff0000" } }, - "future-key": [1, 2] - }) - ); - } - - #[test] - fn a_theme_change_leaves_the_scheme_alone_and_the_reverse() { - let mut root = serde_json::json!({ - "theme": "light", - "terminal": { "colors": { "scheme": "gruvbox-dark" } } - }); - let map = root.as_object_mut().expect("object"); - apply_appearance(map, &AppearanceChange::Theme(ThemeChoice::Dark)); - assert_eq!(map["terminal"]["colors"]["scheme"], "gruvbox-dark"); - apply_appearance(map, &AppearanceChange::Scheme(None)); - assert_eq!(map["theme"], "dark"); - assert_eq!(map["terminal"]["colors"], serde_json::json!({})); + /// What the panel would write, applied by the catalogue's own setter. + fn saved(change: &AppearanceChange) -> Settings { + let (id, value) = change.option(); + let written = termherd_mcp::set_option(&serde_json::json!({}), id, &value) + .expect("the catalogue accepts every panel choice"); + serde_json::from_value(written).expect("valid settings") } #[test] - fn a_scheme_reaches_a_file_whose_terminal_block_was_not_an_object() { - let mut root = serde_json::Map::new(); - root.insert("terminal".into(), serde_json::json!("not an object")); - apply_appearance( - &mut root, - &AppearanceChange::Scheme(Some("gruvbox-light".to_string())), - ); - assert_eq!( - serde_json::Value::Object(root), - serde_json::json!({ "terminal": { "colors": { "scheme": "gruvbox-light" } } }) - ); + fn every_saved_theme_reads_back_as_what_was_picked() { + for theme in ThemeChoice::ALL { + assert_eq!(saved(&AppearanceChange::Theme(theme)).theme, theme); + } } #[test] - fn the_saved_theme_reads_back_as_what_was_picked() { - for theme in ThemeChoice::ALL { - let mut root = serde_json::Map::new(); - apply_appearance(&mut root, &AppearanceChange::Theme(theme)); - let s: Settings = - serde_json::from_value(serde_json::Value::Object(root)).expect("valid settings"); - assert_eq!(s.theme, theme); + fn every_saved_scheme_reads_back_as_what_was_picked() { + for scheme in Palette::SCHEMES { + let s = saved(&AppearanceChange::Scheme(Some(scheme.to_string()))); + assert_eq!(s.terminal.colors.scheme.as_deref(), Some(scheme)); } + let s = saved(&AppearanceChange::Scheme(None)); + assert_eq!(s.terminal.colors.scheme, None, "built-in clears the scheme"); } #[test] diff --git a/crates/app/src/shell.rs b/crates/app/src/shell.rs index 470aa91..288777d 100644 --- a/crates/app/src/shell.rs +++ b/crates/app/src/shell.rs @@ -31,7 +31,7 @@ use termherd_pty::{PtyEvent, Screen}; use crate::docs::DocEntry; use crate::record_config::RecordConfig; -use crate::settings::{ClipboardGestures, CloseSettings, ColorSettings, ThemeChoice}; +use crate::settings::{ClipboardGestures, CloseSettings, ThemeChoice}; use crate::window_config::WindowConfig; mod appearance; @@ -96,63 +96,19 @@ fn home_dir() -> String { /// Resolved user configuration handed to the shell at startup: the theme, /// keymap and metadata overlay built from `settings.json` / `metadata.json`. /// Bundled so the composition root passes one value, not a long argument list. +#[derive(Clone)] pub struct Startup { - pub theme: ThemeChoice, - /// The terminal colour settings, kept whole so a scheme picked in the - /// settings panel still honours the user's explicit overrides. - pub colors: ColorSettings, - pub keymap: Keymap, + /// The parsed `settings.json`, adopted through the same path a live + /// reload takes, so a setting cannot apply at startup and not on reload. + pub settings: crate::settings::Settings, pub metadata: Overlay, /// Folded project paths restored from disk. pub collapsed: HashSet, - /// GIF screencast budget from settings. - pub record: RecordConfig, - /// Sidebar session limit from settings; `0` shows every session. - pub session_limit: usize, - /// Terminal base font size from settings. - pub font_size: f32, - /// Close-confirmation policy for tab close and app quit. - pub close: CloseSettings, - /// Which mouse gestures reach the clipboard. - pub gestures: ClipboardGestures, - /// The editor command from settings, or `None` for the OS default handler. - pub open: Option, - /// Adapter-owned config bits for the MCP `snapshot` tool's config section - /// (the live font size is stamped by `core`, not carried here). - pub config: ConfigInput, /// Config files that existed but could not be used at startup, shown in /// the sidebar until dismissed. pub load_problems: Vec, } -impl Startup { - /// Bundle the sanitised settings with the other persisted state, so the - /// composition root passes one value instead of fanning fields by hand. - #[must_use] - pub fn from_settings( - settings: &crate::settings::Settings, - metadata: Overlay, - collapsed: HashSet, - ) -> Self { - let record = settings.record_config(); - Self { - theme: settings.theme, - colors: settings.terminal.colors.clone(), - keymap: settings.keymap(), - metadata, - collapsed, - session_limit: settings.session_limit(), - font_size: settings.font_size(), - close: settings.close, - gestures: settings.clipboard_gestures(), - open: settings.open_command(), - config: config_input(settings), - record, - load_problems: Vec::new(), - } - } -} - /// The settings the MCP `snapshot` tool reports that `core` cannot read — /// built in one place for startup and for a live reload, so the two cannot /// report different things. @@ -198,21 +154,7 @@ pub fn run( settings: Arc::new(live_settings::LiveSettings(pty.clone())), }, live_bridge.clone(), - Startup { - theme: startup.theme, - colors: startup.colors.clone(), - keymap: startup.keymap.clone(), - metadata: startup.metadata.clone(), - collapsed: startup.collapsed.clone(), - record: startup.record, - session_limit: startup.session_limit, - font_size: startup.font_size, - close: startup.close, - gestures: startup.gestures, - open: startup.open.clone(), - config: startup.config.clone(), - load_problems: startup.load_problems.clone(), - }, + startup.clone(), ); let initial_scan = shell.rescan(); (shell, initial_scan) @@ -266,6 +208,8 @@ struct Shell { bounds: WindowConfig, scanner: Arc, watch_root: Option, + /// The `settings.json` the live-reload watch follows; resolved once. + settings_file: Option, scan_error: Option, /// Checks whether a path-shaped run of terminal text names a real file. /// The one thing that tells `src/main.rs` from `and/or`. @@ -737,16 +681,14 @@ impl Shell { let mut core = termherd_core::App::new(); core.apply(termherd_core::Event::MetadataLoaded(startup.metadata)); core.apply(termherd_core::Event::CollapsedLoaded(startup.collapsed)); - core.apply(termherd_core::Event::SessionLimitLoaded( - startup.session_limit, - )); - core.apply(termherd_core::Event::FontSizeLoaded(startup.font_size)); - core.apply(termherd_core::Event::OpenCommandLoaded(startup.open)); - Self { + // Every settings-derived field starts at its default and is then + // adopted from the file below, exactly as a reload adopts it. + let mut shell = Self { core, bounds, scanner, watch_root, + settings_file: crate::settings::path(), scan_error: None, path_resolver, pty, @@ -758,14 +700,11 @@ impl Shell { screens: HashMap::new(), waiters: Vec::new(), focus: Focus::Search, - appearance: appearance::Appearance { - theme: startup.theme, - colors: startup.colors, - }, + appearance: appearance::Appearance::default(), settings_open: false, load_problems: startup.load_problems, settings_sink, - keymap: startup.keymap, + keymap: Keymap::defaults(), renaming: None, tab_rename: None, // Populated by the first scan's `refresh_docs` — `discover` does @@ -775,17 +714,19 @@ impl Shell { rescan_pending: false, open_doc: None, closing: None, - close_confirm: startup.close, - gestures: startup.gestures, + close_confirm: CloseSettings::default(), + gestures: ClipboardGestures::default(), archiving: None, closing_window: None, link_modifier: false, shift_modifier: false, tab_drag: None, exiting: false, - record: RecordState::new(startup.record), - config: startup.config, - } + record: RecordState::new(RecordConfig::default()), + config: config_input(&crate::settings::Settings::default()), + }; + shell.adopt_settings(&startup.settings); + shell } /// The GUI chrome theme (FR10); the terminal grid keeps its own colours. @@ -1458,9 +1399,9 @@ impl Shell { subs.push(Subscription::run_with(root.clone(), watch_stream)); } subs.push(Subscription::run_with(self.pty_output.clone(), pty_stream)); - if let Some(file) = crate::settings::path() { + if let Some(file) = &self.settings_file { subs.push(Subscription::run_with( - file, + file.clone(), live_settings::settings_watch_stream, )); } @@ -1488,7 +1429,7 @@ impl Shell { mod key_routing { use super::routing::KeyboardOwner; use super::*; - use crate::settings::ConfirmClose; + use crate::settings::{ColorSettings, ConfirmClose}; use iced::keyboard::key::{Named, NativeCode, Physical}; use iced::keyboard::{Key, Location, Modifiers}; use std::sync::Mutex as StdMutex; @@ -1606,33 +1547,13 @@ mod key_routing { /// The default startup payload the test shells boot with. fn test_startup() -> Startup { Startup { - theme: ThemeChoice::default(), - colors: ColorSettings::default(), - keymap: Keymap::defaults(), + settings: crate::settings::Settings::default(), metadata: Overlay::default(), collapsed: HashSet::new(), - record: RecordConfig::default(), - session_limit: 0, - font_size: 14.0, - close: CloseSettings::default(), - gestures: ClipboardGestures::default(), - open: None, - config: test_config_input(), load_problems: Vec::new(), } } - /// A neutral config input for test shells — the snapshot tool's config - /// section is exercised in the `core` builder tests, not here. - fn test_config_input() -> ConfigInput { - ConfigInput { - terminal_scheme: None, - record_fps: 8, - record_scale: 0.5, - keymap_overrides: 0, - } - } - /// Live-bridge wiring for a test shell: a fresh request channel and no MCP /// server (endpoint `None`), so tests that don't exercise the bridge are /// unaffected. Tests that do set `shell.mcp_endpoint` afterwards. @@ -1686,7 +1607,10 @@ mod key_routing { WindowConfig::default(), test_ports(Arc::new(RecordingPty::default()), rx), test_live_bridge(), - Startup::from_settings(&settings, Overlay::default(), HashSet::new()), + Startup { + settings, + ..test_startup() + }, ); let request = termherd_core::PathRequest { @@ -3868,10 +3792,8 @@ mod key_routing { let (_tx, rx) = iced::futures::channel::mpsc::unbounded::(); let mut ports = test_ports(Arc::new(RecordingPty::default()), rx); ports.settings = sink.clone(); - let startup = Startup { - colors, - ..test_startup() - }; + let mut startup = test_startup(); + startup.settings.terminal.colors = colors; let shell = Shell::new(WindowConfig::default(), ports, test_live_bridge(), startup); (shell, sink) } diff --git a/crates/app/src/shell/appearance.rs b/crates/app/src/shell/appearance.rs index d25f680..b51eb7c 100644 --- a/crates/app/src/shell/appearance.rs +++ b/crates/app/src/shell/appearance.rs @@ -12,6 +12,7 @@ use super::Shell; use crate::settings::{AppearanceChange, ColorSettings, ThemeChoice}; /// What the settings panel edits. +#[derive(Default)] pub(super) struct Appearance { pub(super) theme: ThemeChoice, /// Kept whole, so a picked scheme still honours the user's explicit @@ -36,13 +37,7 @@ impl SchemeChoice { /// built-in answers to, so the picker shows no selection rather than a /// wrong one. pub(super) fn of(scheme: Option<&str>) -> Option { - match scheme { - None => Some(Self(None)), - Some(name) => Palette::SCHEMES - .into_iter() - .find(|known| *known == name) - .map(|known| Self(Some(known))), - } + Self::all().into_iter().find(|choice| choice.0 == scheme) } } diff --git a/crates/app/src/shell/live_settings.rs b/crates/app/src/shell/live_settings.rs index a8125cf..7ae2f68 100644 --- a/crates/app/src/shell/live_settings.rs +++ b/crates/app/src/shell/live_settings.rs @@ -76,20 +76,33 @@ impl Shell { /// makes the settings panel's own writes — echoed back by the watch — /// harmless. pub(super) fn apply_settings(&mut self, settings: &Settings) -> Task { - self.appearance.theme = settings.theme; let palette = settings.palette(); if palette != self.appearance.colors.to_palette() { self.settings_sink.repaint(palette); } + self.settings_sink.set_shell(settings.shell_profile()); + let font_before = self.core.font_size(); + self.adopt_settings(settings); + // A new base size re-derives every grid, as a zoom does. + if self.core.font_size() == font_before { + Task::none() + } else { + self.resize_panes() + } + } + + /// The shell's own share of `settings`: what it holds itself and what it + /// tells `core`. Startup takes this path too — there the terminals + /// already start with the file's palette and shell, so the sink is not + /// told again. + pub(super) fn adopt_settings(&mut self, settings: &Settings) { + self.appearance.theme = settings.theme; self.appearance.colors = settings.terminal.colors.clone(); self.keymap = settings.keymap(); self.close_confirm = settings.close; self.gestures = settings.clipboard_gestures(); self.record.set_config(settings.record_config()); self.config = super::config_input(settings); - self.settings_sink.set_shell(settings.shell_profile()); - - let font_before = self.core.font_size(); for event in [ termherd_core::Event::SessionLimitLoaded(settings.session_limit()), termherd_core::Event::FontSizeLoaded(settings.font_size()), @@ -97,12 +110,6 @@ impl Shell { ] { self.core.apply(event); } - // A new base size re-derives every grid, as a zoom does. - if self.core.font_size() == font_before { - Task::none() - } else { - self.resize_panes() - } } } diff --git a/crates/app/src/shell/view/style.rs b/crates/app/src/shell/view/style.rs index cfacf47..e8ae7b3 100644 --- a/crates/app/src/shell/view/style.rs +++ b/crates/app/src/shell/view/style.rs @@ -147,13 +147,12 @@ mod tests { fn every_status_dot_reads_on_every_light_chrome() { // The WCAG floor for non-text UI components. const FLOOR: f32 = 3.0; - for theme in [ - iced::Theme::Light, - iced::Theme::SolarizedLight, - iced::Theme::GruvboxLight, - ] { + let light = crate::settings::ThemeChoice::ALL + .map(crate::settings::ThemeChoice::to_iced) + .into_iter() + .filter(|theme| !theme.extended_palette().is_dark); + for theme in light { let palette = theme.extended_palette(); - assert!(!palette.is_dark, "{theme} reads as light"); for surface in [palette.background.base.color, palette.background.weak.color] { for status in ALL { let ratio = contrast(status_color(status, false), surface); diff --git a/crates/mcp/src/file.rs b/crates/mcp/src/file.rs new file mode 100644 index 0000000..3ce0223 --- /dev/null +++ b/crates/mcp/src/file.rs @@ -0,0 +1,133 @@ +//! The one writer of `settings.json` options, shared by every surface that sets +//! one — this stdio server, the live bridge and the GUI settings panel — so the +//! rule that guards the user's file is stated once. +//! +//! The rule: a file that exists but is not a JSON object is never written +//! over. Setting one option onto an empty object in its place would discard +//! everything else the user wrote. + +use std::path::Path; + +use serde_json::{Map, Value}; + +use crate::SetError; + +/// Why [`set_option_at`] wrote nothing. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SetAtError { + /// The catalogue refused the id or the value: the caller's to fix. + Refused(SetError), + /// The file could not be read, parsed as an object, or written. + File(String), +} + +impl std::fmt::Display for SetAtError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SetAtError::Refused(error) => error.fmt(f), + SetAtError::File(reason) => f.write_str(reason), + } + } +} + +impl std::error::Error for SetAtError {} + +/// The JSON object at `path`; an empty one when the file does not exist. A +/// file that is unreadable or not a JSON object is an error. +pub fn read_object(path: &Path) -> Result { + let raw = match std::fs::read_to_string(path) { + Ok(raw) => raw, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return Ok(Value::Object(Map::new())); + } + Err(e) => return Err(format!("could not read {}: {e}", path.display())), + }; + match serde_json::from_str::(&raw) { + Ok(value) if value.is_object() => Ok(value), + _ => Err(format!( + "{} is not a valid JSON object; fix it first", + path.display() + )), + } +} + +/// Pretty-print `value` to `path`, creating its directory. +pub fn write(path: &Path, value: &Value) -> Result<(), String> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir) + .map_err(|e| format!("could not create {}: {e}", dir.display()))?; + } + let json = serde_json::to_string_pretty(value) + .map_err(|e| format!("could not encode {}: {e}", path.display()))?; + std::fs::write(path, json + "\n") + .map_err(|e| format!("could not write {}: {e}", path.display())) +} + +/// Set one catalogue option in the file at `path`, leaving every other key as +/// found. Nothing is written on any refusal. +pub fn set_option_at(path: &Path, id: &str, value: &Value) -> Result<(), SetAtError> { + let settings = read_object(path).map_err(SetAtError::File)?; + let edited = crate::set_option(&settings, id, value).map_err(SetAtError::Refused)?; + write(path, &edited).map_err(SetAtError::File) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn temp_file(name: &str) -> std::path::PathBuf { + let dir = + std::env::temp_dir().join(format!("termherd-mcp-file-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("dir"); + dir.join("settings.json") + } + + #[test] + fn a_missing_file_reads_as_empty_and_a_set_creates_it() { + let path = temp_file("missing"); + assert_eq!(read_object(&path), Ok(json!({}))); + + set_option_at(&path, "theme", &json!("light")).expect("written"); + assert_eq!(read_object(&path), Ok(json!({ "theme": "light" }))); + } + + #[test] + fn a_file_that_is_not_an_object_is_never_written_over() { + let path = temp_file("broken"); + std::fs::write(&path, "{ broken").expect("write"); + + let result = set_option_at(&path, "theme", &json!("light")); + + assert!(matches!(result, Err(SetAtError::File(_)))); + assert_eq!(std::fs::read_to_string(&path).expect("read"), "{ broken"); + } + + #[test] + fn a_refused_value_writes_nothing() { + let path = temp_file("refused"); + + let result = set_option_at(&path, "theme", &json!("mauve")); + + assert!(matches!(result, Err(SetAtError::Refused(_)))); + assert!(!path.exists()); + } + + #[test] + fn other_keys_survive_a_set() { + let path = temp_file("siblings"); + std::fs::write(&path, r#"{ "future": 1, "terminal": { "font_size": 14 } }"#) + .expect("write"); + + set_option_at(&path, "terminal.colors.scheme", &json!("gruvbox-light")).expect("written"); + + assert_eq!( + read_object(&path), + Ok(json!({ + "future": 1, + "terminal": { "font_size": 14, "colors": { "scheme": "gruvbox-light" } } + })) + ); + } +} diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 41f33d9..d23a7d0 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -16,6 +16,11 @@ //! no I/O, no globals — so the protocol is unit-testable. A write is *described* //! (the returned [`Reply::write_settings`]) here and *performed* by the thin //! stdio loop in `main.rs`, keeping the same Event→Effect split as `core`. +//! +//! The one exception is [`file`]: the guarded read-modify-write of +//! `settings.json`, shared with the GUI so its rule lives in one place. + +pub mod file; use serde_json::{Value, json}; diff --git a/crates/mcp/src/main.rs b/crates/mcp/src/main.rs index 226f50b..7c5131a 100644 --- a/crates/mcp/src/main.rs +++ b/crates/mcp/src/main.rs @@ -68,12 +68,10 @@ fn write_error_response(message: &Value, detail: &str) -> Option { })) } -/// Read `~/.termherd/settings.json` into a JSON value, falling back to an empty -/// object when it is missing or unreadable — `list_options` then reports every -/// option as unset rather than failing. The second half says whether a write -/// may replace the file: not when it exists but does not parse, since writing -/// the one option set onto an empty object would discard everything else the -/// user wrote. +/// Read `~/.termherd/settings.json`, falling back to an empty object when it +/// cannot be used — `list_options` then reports every option as unset rather +/// than failing. The second half says whether a write may replace the file: +/// not when it exists but is not a JSON object (see [`termherd_mcp::file`]). fn load_settings() -> (Value, Result<(), String>) { match settings_path() { Some(path) => load_settings_at(&path), @@ -83,35 +81,18 @@ fn load_settings() -> (Value, Result<(), String>) { /// The body of [`load_settings`], on an explicit path. fn load_settings_at(path: &std::path::Path) -> (Value, Result<(), String>) { - match std::fs::read_to_string(path) { - Ok(raw) => match serde_json::from_str::(&raw) { - Ok(value) if value.is_object() => (value, Ok(())), - _ => ( - json!({}), - Err(format!( - "{} is not a valid JSON object; fix it before setting options", - path.display() - )), - ), - }, - Err(_) => (json!({}), Ok(())), + match termherd_mcp::file::read_object(path) { + Ok(value) => (value, Ok(())), + Err(reason) => (json!({}), Err(reason)), } } -/// Persist the mutated settings to `~/.termherd/settings.json`, pretty-printed -/// like the file the GUI writes. Creates the `~/.termherd` directory if needed. -/// Returns an error message (not a panic — the server stays up) so the caller -/// can report the failure instead of a false success. +/// Persist the mutated settings to `~/.termherd/settings.json`. Returns an +/// error message (not a panic — the server stays up) so the caller can report +/// the failure instead of a false success. fn store_settings(settings: &Value) -> Result<(), String> { let path = settings_path().ok_or("no home directory; cannot write settings")?; - if let Some(dir) = path.parent() { - std::fs::create_dir_all(dir) - .map_err(|err| format!("could not create {}: {err}", dir.display()))?; - } - let raw = serde_json::to_string_pretty(settings) - .map_err(|err| format!("could not encode settings: {err}"))?; - std::fs::write(&path, raw + "\n") - .map_err(|err| format!("could not write {}: {err}", path.display())) + termherd_mcp::file::write(&path, settings) } /// `~/.termherd/settings.json` — the same file the GUI shell reads. diff --git a/crates/pty/src/manager.rs b/crates/pty/src/manager.rs index f4556c7..e2d9cb2 100644 --- a/crates/pty/src/manager.rs +++ b/crates/pty/src/manager.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use std::io::Write; -use std::sync::{Arc, Mutex, RwLock, mpsc}; +use std::sync::{Arc, Mutex, PoisonError, RwLock, mpsc}; use portable_pty::{CommandBuilder, PtySize, native_pty_system}; use termherd_core::ports::{PtyError, PtyHost}; @@ -68,10 +68,7 @@ impl PtyManager { /// Launch `shell` (or the platform default for `None`) from the next /// spawned session on. A running session keeps the program it started. pub fn set_shell(&self, shell: Option) { - match self.shell.write() { - Ok(mut guard) => *guard = shell, - Err(poisoned) => *poisoned.into_inner() = shell, - } + *self.shell.write().unwrap_or_else(PoisonError::into_inner) = shell; } /// Switch every session — running and future — to `palette`. Each live @@ -80,10 +77,7 @@ impl PtyManager { /// (Claude Code picks light or dark that way) keeps its choice until it /// asks again. pub fn set_palette(&self, palette: Palette) { - match self.palette.write() { - Ok(mut guard) => *guard = palette, - Err(poisoned) => *poisoned.into_inner() = palette, - } + *self.palette.write().unwrap_or_else(PoisonError::into_inner) = palette; if let Ok(map) = self.sessions.lock() { for session in map.values() { let _ = session.ctrl.send(TermCmd::Repaint); @@ -135,10 +129,11 @@ impl PtyHost for PtyManager { // project directory with a sane TERM. Resuming a real Claude session // lands in a later slice; the id flows through so the adapter never has // to invent one (Q6). - let configured = match self.shell.read() { - Ok(guard) => guard.clone(), - Err(poisoned) => poisoned.into_inner().clone(), - }; + let configured = self + .shell + .read() + .unwrap_or_else(PoisonError::into_inner) + .clone(); let mut cmd = match &configured { Some(shell) => { let mut c = CommandBuilder::new(&shell.program); diff --git a/crates/pty/src/session.rs b/crates/pty/src/session.rs index bd8bb7e..9247098 100644 --- a/crates/pty/src/session.rs +++ b/crates/pty/src/session.rs @@ -6,7 +6,7 @@ //! colour queries the parser raises. use std::io::{Read, Write}; -use std::sync::{Arc, Mutex, RwLock, mpsc}; +use std::sync::{Arc, Mutex, PoisonError, RwLock, mpsc}; use std::thread::JoinHandle; use alacritty_terminal::Term; @@ -53,10 +53,10 @@ pub(crate) type SharedPalette = Arc>; /// The palette in force right now. A poisoned lock still holds a whole /// palette — a writer cannot half-assign one — so its value is read anyway. fn current(palette: &SharedPalette) -> Palette { - match palette.read() { - Ok(guard) => guard.clone(), - Err(poisoned) => poisoned.into_inner().clone(), - } + palette + .read() + .unwrap_or_else(PoisonError::into_inner) + .clone() } /// How often the watcher thread asks the PTY which process group owns it. Short diff --git a/crates/scan/src/watch.rs b/crates/scan/src/watch.rs index 7be32d5..0013d9d 100644 --- a/crates/scan/src/watch.rs +++ b/crates/scan/src/watch.rs @@ -55,7 +55,7 @@ pub fn watch_file( event .paths .iter() - .any(|path| path.file_name().map(std::ffi::OsStr::to_os_string) == name) + .any(|path| path.file_name() == name.as_deref()) }, debounce, on_change,