From 64313a58a1f750f47b05b29ca30538b4cc220910 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 14:24:03 -0400 Subject: [PATCH 1/8] feat(profile): add terminal profiles and unify color resolution A screenshot and a color assertion disagreed about what a cell was painted. `render/svg.rs` carried a private sixteen-color table and `assert/color.rs` carried a different one, so `expect --fg "#800000"` passed on a cell the screenshot drew `#e88388`. Both tables are deleted here and both callers resolve through one profile, which is what makes them agree by construction rather than by coincidence. The palette had to become configurable to fix it anyway: the two tables could only be collapsed by choosing which one was right, and that choice belongs to the user rather than to whichever module was read first. The shipped default is the VGA/xterm palette that `TERM=xterm-256color` already promises, which is what the assertion side used. A profile is read from `shell-use.toml` and sets scrollback and colors. Only the sixteen ANSI slots and the three defaults are configurable; indices 16-255 are the xterm color cube and gray ramp, which are fixed by the spec, so a config that could move them would let two sessions disagree about what `--fg 196` means. Profiles are named, and `--profile` selects one. The file is looked up nearest first, project before user, so a repository can pin the terminal its tests expect. Resolution happens in the CLI rather than the daemon: the daemon is long-lived and shared, so it has no single working directory to resolve a project-local config against, and a resolved profile travels on `Request::Open` the same way timeouts already do. Absent settings take the default, and the field is `#[serde(default)]`, so a client that predates this keeps the behavior it had. Scrollback moves from a hardcoded 5,000 to a configurable 10,000, matching alacritty's own default. Two things are deliberately errors rather than silent fallbacks: an unknown profile name, which reports the ones that exist, and a config file that does not parse, which would otherwise run the session with settings nobody asked for. A *missing* file stays fine, since running without one is normal. Screenshots will look different: the default background is now black rather than the previous dark blue-gray, and the palette is saturated rather than muted. Both are recoverable in a profile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- Cargo.lock | 83 +++ Cargo.toml | 1 + README.md | 50 +- SKILL.md | 24 +- crates/shell-use-cli/src/cli.rs | 26 + crates/shell-use-cli/src/main.rs | 4 + .../shell-use-cli/tests/session_lifecycle.rs | 92 +++ crates/shell-use/Cargo.toml | 1 + crates/shell-use/src/assert/color.rs | 138 +++-- crates/shell-use/src/engine.rs | 24 +- crates/shell-use/src/lib.rs | 1 + crates/shell-use/src/profile.rs | 522 ++++++++++++++++++ crates/shell-use/src/protocol.rs | 7 + crates/shell-use/src/render/svg.rs | 121 ++-- crates/shell-use/src/session.rs | 7 +- crates/shell-use/tests/runtime.rs | 3 + 16 files changed, 940 insertions(+), 164 deletions(-) create mode 100644 crates/shell-use/src/profile.rs diff --git a/Cargo.lock b/Cargo.lock index e00cc8e..1572b43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -392,6 +392,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" +[[package]] +name = "equivalent" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" + [[package]] name = "errno" version = "0.3.14" @@ -544,6 +550,12 @@ dependencies = [ "wasi", ] +[[package]] +name = "hashbrown" +version = "0.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" + [[package]] name = "heck" version = "0.5.0" @@ -565,6 +577,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "indexmap" +version = "2.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" +dependencies = [ + "equivalent", + "hashbrown", +] + [[package]] name = "interprocess" version = "2.4.2" @@ -1117,6 +1139,15 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_spanned" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" +dependencies = [ + "serde_core", +] + [[package]] name = "serial2" version = "0.2.37" @@ -1164,6 +1195,7 @@ dependencies = [ "serde", "serde_json", "sha2", + "toml", "ttf-parser", ] @@ -1352,6 +1384,45 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "toml" +version = "0.9.12+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf92845e79fc2e2def6a5d828f0801e29a2f8acc037becc5ab08595c7d5e9863" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned", + "toml_datetime", + "toml_parser", + "toml_writer", + "winnow 0.7.15", +] + +[[package]] +name = "toml_datetime" +version = "0.7.5+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92e1cfed4a3038bc5a127e35a2d360f145e1f4b971b551a2ba5fd7aedf7e1347" +dependencies = [ + "serde_core", +] + +[[package]] +name = "toml_parser" +version = "1.1.3+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" +dependencies = [ + "winnow 1.0.4", +] + +[[package]] +name = "toml_writer" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" + [[package]] name = "ttf-parser" version = "0.25.1" @@ -1530,6 +1601,18 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "winnow" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" + +[[package]] +name = "winnow" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" + [[package]] name = "winreg" version = "0.10.1" diff --git a/Cargo.toml b/Cargo.toml index 89f1c8e..c13d9a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ regex = "1.12.4" serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.150" sha2 = "0.10.9" +toml = "0.9" shell-use = { path = "crates/shell-use" } ttf-parser = { version = "0.25.1", default-features = false, features = ["std"] } diff --git a/README.md b/README.md index ac6b08a..27a36a2 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ prints a session's effective timeouts. | Command | Description | | ------------------------------------------------------------ | ------------------------------------------- | -| `open [--shell S] [--cols N --rows N] [--cwd D] [--env K=V] [--timeout- MS]` | Spawn a shell session. | +| `open [--shell S] [--cols N --rows N] [--cwd D] [--env K=V] [--config F] [--profile P] [--timeout- MS]` | Spawn a shell session. | | `run [args...]` | Spawn a session running a program directly. | | `sessions` | List active sessions. | | `close [--all]` | Close the current session (or all). | @@ -295,6 +295,54 @@ Every command returns a stable exit code so an agent can branch on the failure c With `--json`, failures also carry a `"kind"` field (`assertion`/`usage`/`no_session`/`internal`). +## Configuration + +Settings live in a `shell-use.toml` with named profiles. Everything is +optional, so a file only states what it changes: + +```toml +[profiles.default] +scrollback = 10000 # rows kept beyond the visible screen + +[profiles.default.colors] +background = "#000000" +foreground = "#c0c0c0" +cursor = "#c0c0c0" +red = "#800000" # any of the 16 ANSI slots, by name + +[profiles.ci] +scrollback = 500 # inherits the default palette +``` + +```bash +shell-use open # profile "default" +shell-use open --profile ci +shell-use open --config ./other.toml --profile ci +``` + +Looked up nearest first: `./shell-use.toml`, then +`~/.shell-use/shell-use.toml`. `--config` or `SHELL_USE_CONFIG` replaces the +search. Running without a config file is normal; a file that fails to parse is +an error rather than a silent fallback. + +Resolution happens in the CLI, not the daemon — the daemon is long-lived and +shared, so it has no working directory to resolve a project-local config +against. + +### Colors + +A terminal grid stores colour *indices*, not colours. What index 1 looks like +is the profile's choice, and shell-use needs that choice twice: to draw a +screenshot, and to answer `expect --fg "#rrggbb"`. **Both read the same table**, +so a colour an assertion matches is the colour a screenshot paints. + +Only the 16 ANSI slots and the three defaults are configurable. Indices 16-255 +are the xterm colour cube and grey ramp, fixed by the spec, so `--fg 196` means +the same thing in every profile. + +The shipped palette is the classic VGA/xterm one that `TERM=xterm-256color` +promises. + ## Supported shells - bash diff --git a/SKILL.md b/SKILL.md index a0084da..fc185db 100644 --- a/SKILL.md +++ b/SKILL.md @@ -60,7 +60,7 @@ without parsing text: | Command | Description | | --- | --- | -| `open [--shell S] [--cols N] [--rows N] [--cwd D] [--env K=V]...` | Spawn a shell session (auto-starts the daemon). `--env` is repeatable. | +| `open [--shell S] [--cols N] [--rows N] [--cwd D] [--env K=V] [--config F] [--profile P]...` | Spawn a shell session (auto-starts the daemon). `--env` is repeatable. | | `run [args...] [--cols N] [--rows N] [--cwd D] [--env K=V]...` | Spawn a session running a program directly (no shell). | | `sessions` | List active sessions. | | `close [--all]` | Close the current session (or every session with `--all`). | @@ -299,6 +299,28 @@ of `ShellUseError`. On its first call a client also checks that the daemon's version matches the package and raises `VersionMismatchError` if they differ; stop the daemon (`daemon_stop`) so it restarts on the matching binary. +## Configuration + +`shell-use.toml` holds named profiles; `--profile NAME` selects one and +`--config PATH` picks the file. Looked up nearest first: `./shell-use.toml` +then `~/.shell-use/shell-use.toml`. No file is fine; an unparseable one errors. + +```toml +[profiles.ci] +scrollback = 500 + +[profiles.ci.colors] +red = "#ff0000" +``` + +A profile sets `scrollback` (default 10000) and colors: `foreground`, +`background`, `cursor`, and the 16 ANSI slots by name (`red`, `bright_red`, +...). Indices 16-255 are spec-defined and not configurable, so `--fg 196` is +stable across profiles. + +The palette is what a screenshot paints **and** what `expect --fg/--bg` matches +a `#rrggbb` against, so the two always agree. + ## Supported shells & integration `open --shell S` accepts: `bash`, `zsh`, `fish`, `powershell`, `pwsh`, `cmd`, diff --git a/crates/shell-use-cli/src/cli.rs b/crates/shell-use-cli/src/cli.rs index e8b5538..a405e6d 100644 --- a/crates/shell-use-cli/src/cli.rs +++ b/crates/shell-use-cli/src/cli.rs @@ -34,6 +34,28 @@ impl From for Shell { } } +/// Which terminal profile a session runs with. +#[derive(Args, Clone, Default)] +pub struct ProfileArgs { + /// Config file to read (default: ./shell-use.toml, then + /// ~/.shell-use/shell-use.toml). + #[arg(long, value_name = "PATH")] + pub config: Option, + /// Named profile from the config file (default: `default`). + #[arg(long, value_name = "NAME")] + pub profile: Option, +} + +impl ProfileArgs { + /// Resolve to concrete settings. Done here, in the client, because the + /// daemon is long-lived and shared and so has no working directory to + /// resolve a project-local config against. + pub fn resolve(&self) -> anyhow::Result { + let cwd = std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")); + shell_use::profile::resolve(self.config.as_deref(), self.profile.as_deref(), &cwd) + } +} + /// Per-class default timeouts for a session, in milliseconds. #[derive(Args, Clone, Copy, Default)] pub struct TimeoutArgs { @@ -114,6 +136,8 @@ pub enum Command { #[arg(long, conflicts_with = "wait_ready")] no_wait_ready: bool, #[command(flatten)] + profile: ProfileArgs, + #[command(flatten)] timeouts: TimeoutArgs, }, /// Spawn a session running a program directly. @@ -143,6 +167,8 @@ pub enum Command { #[arg(long, conflicts_with = "wait_ready")] no_wait_ready: bool, #[command(flatten)] + profile: ProfileArgs, + #[command(flatten)] timeouts: TimeoutArgs, }, /// Close the current session (or all sessions). diff --git a/crates/shell-use-cli/src/main.rs b/crates/shell-use-cli/src/main.rs index 245e401..4575398 100644 --- a/crates/shell-use-cli/src/main.rs +++ b/crates/shell-use-cli/src/main.rs @@ -132,10 +132,12 @@ fn build_request(command: Command) -> anyhow::Result { env, wait_ready, no_wait_ready, + profile, timeouts, } => Request::Open { shell: shell.map(Into::into), program: None, + profile: profile.resolve()?, cols, rows, cwd, @@ -152,6 +154,7 @@ fn build_request(command: Command) -> anyhow::Result { env, wait_ready, no_wait_ready, + profile, timeouts, } => { let mut prog = vec![program]; @@ -159,6 +162,7 @@ fn build_request(command: Command) -> anyhow::Result { Request::Open { shell: None, program: Some(prog), + profile: profile.resolve()?, cols, rows, cwd, diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index 7e2e6f0..67e95e7 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -251,6 +251,98 @@ fn a_session_timeout_default_applies_to_later_commands() { ); } +/// The color a screenshot paints is the color an assertion matches. +/// +/// These came from two separate hardcoded tables that disagreed on every ANSI +/// slot, so `expect --fg "#800000"` passed on a cell the screenshot painted +/// `#e88388`. Both now resolve through the session profile, and this drives the +/// whole path — daemon, renderer, assertion — rather than the resolver alone. +#[test] +fn a_screenshot_and_an_assertion_agree_on_a_color() { + let sandbox = Sandbox::new("palette-agree"); + // Printed lowercase so the match is the output, not the echoed command. + let print_red = r#"printf "\033[31m%s\033[0m\n" "$(echo QRSX | tr A-Z a-z)""#; + sandbox.ok(&["run", "--cols", "44", "--", "bash", "--norc"]); + sandbox.ok(&["submit", print_red]); + sandbox.ok(&["wait", "command"]); + + // The default profile is the VGA palette, so slot 1 is #800000. + sandbox.ok(&["expect", "text", "qrsx", "--fg", "#800000"]); + + let svg = sandbox.home.join("shot.svg"); + let path = svg.to_str().expect("utf-8 path"); + sandbox.ok(&["screenshot", "--out", path]); + let drawing = std::fs::read_to_string(&svg).expect("read screenshot"); + assert!( + drawing.contains("fill=\"#800000\""), + "the screenshot must paint the color the assertion matched" + ); +} + +/// A profile's palette drives both, so recoloring a slot moves the screenshot +/// and the assertion together. +#[test] +fn a_custom_profile_recolors_screenshots_and_assertions_together() { + let sandbox = Sandbox::new("palette-profile"); + let config = sandbox.home.join("custom.toml"); + std::fs::write(&config, "[profiles.neon.colors]\nred = \"#ff00ff\"\n").expect("write config"); + let config_path = config.to_str().expect("utf-8 path"); + + let print_red = r#"printf "\033[31m%s\033[0m\n" "$(echo QRSX | tr A-Z a-z)""#; + sandbox.ok(&[ + "run", + "--config", + config_path, + "--profile", + "neon", + "--cols", + "44", + "--", + "bash", + "--norc", + ]); + sandbox.ok(&["submit", print_red]); + sandbox.ok(&["wait", "command"]); + + sandbox.ok(&["expect", "text", "qrsx", "--fg", "#ff00ff"]); + let out = sandbox.run(&["expect", "text", "qrsx", "--fg", "#800000"]); + assert!( + !out.status.success(), + "the profile replaced the default red, so the default must no longer match" + ); + + let svg = sandbox.home.join("neon.svg"); + let path = svg.to_str().expect("utf-8 path"); + sandbox.ok(&["screenshot", "--out", path]); + let drawing = std::fs::read_to_string(&svg).expect("read screenshot"); + assert!( + drawing.contains("fill=\"#ff00ff\""), + "the screenshot follows the profile too" + ); +} + +/// A profile that does not exist is an error naming the ones that do, rather +/// than a session that silently ran with the defaults. +#[test] +fn an_unknown_profile_is_rejected() { + let sandbox = Sandbox::new("palette-unknown"); + let config = sandbox.home.join("c.toml"); + std::fs::write(&config, "[profiles.ci]\n").expect("write config"); + let out = sandbox.run(&[ + "open", + "--config", + config.to_str().expect("utf-8 path"), + "--profile", + "nope", + ]); + assert!(!out.status.success(), "an unknown profile must not open"); + let msg = String::from_utf8_lossy(&out.stderr) + String::from_utf8_lossy(&out.stdout); + assert!( + msg.contains("ci"), + "the error should name the real profile: {msg}" + ); +} + #[test] fn state_reports_effective_timeouts() { let sandbox = Sandbox::new("state-timeouts"); diff --git a/crates/shell-use/Cargo.toml b/crates/shell-use/Cargo.toml index 4871ebb..2caea64 100644 --- a/crates/shell-use/Cargo.toml +++ b/crates/shell-use/Cargo.toml @@ -24,4 +24,5 @@ regex.workspace = true serde.workspace = true serde_json.workspace = true sha2.workspace = true +toml.workspace = true ttf-parser.workspace = true diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 8ebf828..56105ef 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -1,6 +1,7 @@ //! Color parsing and comparison for `expect --fg/--bg`. use super::super::terminal::cell::Color; +use crate::profile::Colors; /// The spelling of [`Expected::Default`], on the command line and in messages. pub const DEFAULT: &str = "default"; @@ -66,80 +67,35 @@ fn parse_hex(hex: &str) -> anyhow::Result<(u8, u8, u8)> { /// Does a cell's resolved color match the expected color? /// /// A cell that set no color of its own matches only `default`. It cannot match -/// a concrete value, because which value it paints is the viewer's theme's -/// choice and not something the grid knows. -pub fn matches(cell: Option, expected: &Expected) -> bool { +/// a concrete value: which value it paints is the profile's choice, and the +/// grid only records that the cell chose nothing. +/// +/// A concrete `#rrggbb` is resolved through the session profile, the same table +/// the screenshot renderer draws with. These used to be two separate hardcoded +/// tables that disagreed on every ANSI slot, so `expect --fg "#800000"` passed +/// on a cell a screenshot painted `#e88388`. +pub fn matches(cell: Option, expected: &Expected, colors: &Colors) -> bool { let Some(cell) = cell else { return matches!(expected, Expected::Default); }; match expected { Expected::Default => false, Expected::Ansi256(n) => cell.to_index() == *n, - Expected::Hex(er, eg, eb) | Expected::Rgb(er, eg, eb) => rgb_of(cell) == (*er, *eg, *eb), - } -} - -fn rgb_of(c: Color) -> (u8, u8, u8) { - match c { - Color::Rgb(r, g, b) => (r, g, b), - c => ansi256_to_rgb(c.to_index()), + Expected::Hex(er, eg, eb) | Expected::Rgb(er, eg, eb) => { + let got = colors.resolve(Some(cell), true); + (got.r, got.g, got.b) == (*er, *eg, *eb) + } } } /// Render a cell's color in the same space as the expected value, for messages. -pub fn describe_cell(cell: Option, expected: &Expected) -> String { +pub fn describe_cell(cell: Option, expected: &Expected, colors: &Colors) -> String { let Some(cell) = cell else { return DEFAULT.to_string(); }; match expected { Expected::Default | Expected::Ansi256(_) => cell.to_index().to_string(), - _ => { - let (r, g, b) = rgb_of(cell); - format!("#{r:02x}{g:02x}{b:02x}") - } - } -} - -const ANSI16: [(u8, u8, u8); 16] = [ - (0, 0, 0), - (128, 0, 0), - (0, 128, 0), - (128, 128, 0), - (0, 0, 128), - (128, 0, 128), - (0, 128, 128), - (192, 192, 192), - (128, 128, 128), - (255, 0, 0), - (0, 255, 0), - (255, 255, 0), - (0, 0, 255), - (255, 0, 255), - (0, 255, 255), - (255, 255, 255), -]; - -pub fn ansi256_to_rgb(n: u8) -> (u8, u8, u8) { - match n { - 0..=15 => ANSI16[n as usize], - 16..=231 => { - let i = n as u16 - 16; - let r = (i / 36) % 6; - let g = (i / 6) % 6; - let b = i % 6; - let conv = |c: u16| -> u8 { - if c == 0 { - 0 - } else { - (c * 40 + 55) as u8 - } - }; - (conv(r), conv(g), conv(b)) - } - 232..=255 => { - let v = (n as u16 - 232) * 10 + 8; - (v as u8, v as u8, v as u8) - } + _ => colors.resolve(Some(cell), true).to_hex(), } } @@ -188,13 +144,15 @@ mod tests { #[test] fn matches_palette_and_default() { + let c = Colors::default(); let idx = |i| Some(Color::from_index(i)); - assert!(matches(idx(9), &Expected::Ansi256(9))); - assert!(!matches(idx(2), &Expected::Ansi256(9))); - assert!(matches(idx(196), &Expected::Ansi256(196))); + assert!(matches(idx(9), &Expected::Ansi256(9), &c)); + assert!(!matches(idx(2), &Expected::Ansi256(9), &c)); + assert!(matches(idx(196), &Expected::Ansi256(196), &c)); assert!(matches( Some(Color::Rgb(255, 0, 0)), - &Expected::Rgb(255, 0, 0) + &Expected::Rgb(255, 0, 0), + &c )); } @@ -203,17 +161,19 @@ mod tests { /// `default` keyword, which is the way to assert on it. #[test] fn default_color_matches_only_default() { - assert!(!matches(None, &Expected::Ansi256(0))); - assert!(!matches(None, &Expected::Hex(0, 0, 0))); - assert!(matches(None, &Expected::Default)); - assert_eq!(describe_cell(None, &Expected::Ansi256(0)), "default"); + let c = Colors::default(); + assert!(!matches(None, &Expected::Ansi256(0), &c)); + assert!(!matches(None, &Expected::Hex(0, 0, 0), &c)); + assert!(matches(None, &Expected::Default, &c)); + assert_eq!(describe_cell(None, &Expected::Ansi256(0), &c), "default"); } #[test] fn a_colored_cell_is_not_default() { + let c = Colors::default(); let red = Some(Color::from_index(1)); - assert!(!matches(red, &Expected::Default)); - assert!(matches(red, &Expected::Ansi256(1))); + assert!(!matches(red, &Expected::Default, &c)); + assert!(matches(red, &Expected::Ansi256(1), &c)); assert!(matches!( Expected::parse("default").unwrap(), Expected::Default @@ -222,12 +182,44 @@ mod tests { Expected::parse("DEFAULT").unwrap(), Expected::Default )); - assert_eq!(describe_cell(red, &Expected::Default), "1"); + assert_eq!(describe_cell(red, &Expected::Default, &c), "1"); + } + + /// The regression test for the bug this module used to carry: the color a + /// screenshot paints and the color an assertion matches are now the same + /// value for every slot, because both come from the profile. + #[test] + fn an_assertion_matches_the_color_a_screenshot_paints() { + let colors = Colors::default(); + for index in 0u8..=255 { + let cell = Some(Color::from_index(index)); + let painted = colors.resolve(cell, true); + assert!( + matches( + cell, + &Expected::Hex(painted.r, painted.g, painted.b), + &colors + ), + "slot {index} paints {} but does not match it", + painted.to_hex() + ); + } } + /// A profile's palette is what an assertion compares against, so two + /// profiles genuinely disagree rather than sharing one hardcoded table. #[test] - fn ansi256_cube_roundtrip() { - let (r, g, b) = ansi256_to_rgb(196); - assert_eq!((r, g, b), (255, 0, 0)); + fn a_recolored_profile_moves_what_an_assertion_matches() { + let colors = Colors { + red: crate::profile::Rgb::new(1, 2, 3), + ..Default::default() + }; + let red = Some(Color::from_index(1)); + assert!(matches(red, &Expected::Hex(1, 2, 3), &colors)); + assert!(!matches(red, &Expected::Hex(128, 0, 0), &colors)); + assert!( + matches(red, &Expected::Ansi256(1), &colors), + "the index is unaffected by the palette" + ); } } diff --git a/crates/shell-use/src/engine.rs b/crates/shell-use/src/engine.rs index 33d418f..34d55ff 100644 --- a/crates/shell-use/src/engine.rs +++ b/crates/shell-use/src/engine.rs @@ -48,6 +48,7 @@ fn req_summary(req: &Request) -> String { Request::Open { shell, program, + profile, cols, rows, cwd, @@ -55,7 +56,8 @@ fn req_summary(req: &Request) -> String { wait_ready, timeouts, } => format!( - "Open {{ shell: {shell:?}, program: {program:?}, {cols}x{rows}, cwd: {cwd:?}, wait_ready: {wait_ready:?}, timeouts: {timeouts:?}, env: <{} vars> }}", + "Open {{ shell: {shell:?}, program: {program:?}, scrollback: {}, {cols}x{rows}, cwd: {cwd:?}, wait_ready: {wait_ready:?}, timeouts: {timeouts:?}, env: <{} vars> }}", + profile.scrollback, env.len() ), other => format!("{other:?}"), @@ -88,6 +90,7 @@ impl Engine { Request::Open { shell, program, + profile, cols, rows, cwd, @@ -95,7 +98,9 @@ impl Engine { wait_ready, timeouts, } => ( - self.open(shell, program, cols, rows, cwd, env, wait_ready, timeouts), + self.open( + shell, program, profile, cols, rows, cwd, env, wait_ready, timeouts, + ), false, ), Request::Close => { @@ -115,6 +120,7 @@ impl Engine { &self, shell: Option, program: Option>, + profile: crate::profile::Profile, cols: u16, rows: u16, cwd: Option, @@ -129,6 +135,7 @@ impl Engine { match Session::open( shell, program.clone(), + profile, cols, rows, cwd, @@ -720,7 +727,7 @@ fn expect_text( let ok = poll_until( || match locator::find(&grid(s, full), &pattern, strict) { Ok(Some(cells)) if !cells.is_empty() => { - if let Some(err) = check_colors(&cells, &fg, &bg, not) { + if let Some(err) = check_colors(&cells, &fg, &bg, not, &s.profile.colors) { last_err = Some(err); false } else { @@ -750,17 +757,18 @@ fn check_colors( fg: &Option, bg: &Option, not: bool, + colors: &crate::profile::Colors, ) -> Option { let want = !not; if let Some(spec) = fg { let expected = Expected::parse(spec).ok()?; for c in cells { - if color::matches(c.cell.fg, &expected) != want { + if color::matches(c.cell.fg, &expected, colors) != want { return Some(format!( "expected fg {} {}, found {} in cell '{}' at {},{}", if not { "absent" } else { "present" }, expected.describe(), - color::describe_cell(c.cell.fg, &expected), + color::describe_cell(c.cell.fg, &expected, colors), c.cell.ch, c.x, c.y @@ -771,12 +779,12 @@ fn check_colors( if let Some(spec) = bg { let expected = Expected::parse(spec).ok()?; for c in cells { - if color::matches(c.cell.bg, &expected) != want { + if color::matches(c.cell.bg, &expected, colors) != want { return Some(format!( "expected bg {} {}, found {} in cell '{}' at {},{}", if not { "absent" } else { "present" }, expected.describe(), - color::describe_cell(c.cell.bg, &expected), + color::describe_cell(c.cell.bg, &expected, colors), c.cell.ch, c.x, c.y @@ -862,7 +870,7 @@ fn screenshot(s: &Session, full: bool, path: Option) -> Response { let rows = grid(s, full); match path { Some(path) => { - let svg = crate::render::svg::render_svg(&rows, s.cols); + let svg = crate::render::svg::render_svg(&rows, s.cols, &s.profile.colors); match std::fs::write(&path, svg) { Ok(()) => Response::with(json!({ "path": path })), Err(e) => Response::internal(e.to_string()), diff --git a/crates/shell-use/src/lib.rs b/crates/shell-use/src/lib.rs index b9d041b..4053554 100644 --- a/crates/shell-use/src/lib.rs +++ b/crates/shell-use/src/lib.rs @@ -3,6 +3,7 @@ pub mod config; pub mod engine; pub mod input; pub mod logger; +pub mod profile; pub mod protocol; pub mod render; pub mod runtime; diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs new file mode 100644 index 0000000..ec98799 --- /dev/null +++ b/crates/shell-use/src/profile.rs @@ -0,0 +1,522 @@ +//! Terminal profiles: the settings a session runs with. +//! +//! A profile is chosen when a session opens and fixed for its lifetime. It is +//! read from a TOML file so a project can commit the terminal its tests expect, +//! rather than depending on whatever the machine happens to default to. +//! +//! # Colors are resolved here, not by the emulator +//! +//! A terminal grid stores color *indices*, not colors: a cell painted with +//! `SGR 31` records palette slot 1, and what that looks like is the viewer's +//! choice. Nothing in the emulator needs a palette — xterm.js's `theme` option +//! is inert in a headless terminal, and alacritty has no palette at all. +//! +//! shell-use has to make that choice twice: once to draw a screenshot, and once +//! to answer `expect --fg "#rrggbb"`. Those answers have to agree. They used to +//! come from two separate hardcoded tables that disagreed on all sixteen ANSI +//! slots, so `expect --fg "#800000"` passed on a cell the screenshot painted +//! `#e88388`. [`Colors`] is the single table both now read. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Deserializer, Serialize, Serializer}; + +use crate::terminal::cell::{Color, NamedColor}; + +/// Rows of scrollback a profile retains when it does not say otherwise. +/// +/// The emulators do not agree on their own defaults (alacritty 10,000, +/// xterm.js 1,000), so this is always set explicitly rather than inherited. +pub const DEFAULT_SCROLLBACK: usize = 10_000; + +/// The file a profile is read from, under the config directory. +pub const CONFIG_FILE: &str = "shell-use.toml"; + +/// The profile used when none is named. +pub const DEFAULT_PROFILE: &str = "default"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Rgb { + pub r: u8, + pub g: u8, + pub b: u8, +} + +impl Rgb { + pub const fn new(r: u8, g: u8, b: u8) -> Self { + Rgb { r, g, b } + } + + pub fn to_hex(self) -> String { + format!("#{:02x}{:02x}{:02x}", self.r, self.g, self.b) + } + + /// Parse `#rgb` or `#rrggbb`. The leading `#` is optional so a TOML value + /// that lost it to a stray quote still reads sensibly. + pub fn parse(s: &str) -> Result { + let hex = s.trim().trim_start_matches('#'); + let read = |i: usize, n: usize| -> Result { + u8::from_str_radix(&hex[i..i + n], 16) + .map(|v| if n == 1 { v * 17 } else { v }) + .map_err(|_| format!("invalid hex color {s:?}")) + }; + match hex.len() { + 3 => Ok(Rgb::new(read(0, 1)?, read(1, 1)?, read(2, 1)?)), + 6 => Ok(Rgb::new(read(0, 2)?, read(2, 2)?, read(4, 2)?)), + _ => Err(format!("color must be #rgb or #rrggbb (got {s:?})")), + } + } +} + +impl Serialize for Rgb { + fn serialize(&self, s: S) -> Result { + s.serialize_str(&self.to_hex()) + } +} + +impl<'de> Deserialize<'de> for Rgb { + fn deserialize>(d: D) -> Result { + let raw = String::deserialize(d)?; + Rgb::parse(&raw).map_err(serde::de::Error::custom) + } +} + +/// The colors a session paints with. +/// +/// Only the sixteen ANSI slots are configurable. Indices 16-255 are the xterm +/// color cube and gray ramp, which are defined by the spec rather than by a +/// theme, so [`Colors::rgb`] computes them instead of storing them. A config +/// that could override them would let two sessions disagree about what +/// `--fg 196` means. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct Colors { + /// The color text takes when a cell set none of its own. + pub foreground: Rgb, + /// The color an unpainted cell takes. + pub background: Rgb, + /// Tracked for `OSC 12`; nothing draws a cursor yet. + pub cursor: Rgb, + + pub black: Rgb, + pub red: Rgb, + pub green: Rgb, + pub yellow: Rgb, + pub blue: Rgb, + pub magenta: Rgb, + pub cyan: Rgb, + pub white: Rgb, + pub bright_black: Rgb, + pub bright_red: Rgb, + pub bright_green: Rgb, + pub bright_yellow: Rgb, + pub bright_blue: Rgb, + pub bright_magenta: Rgb, + pub bright_cyan: Rgb, + pub bright_white: Rgb, +} + +impl Default for Colors { + /// The classic VGA/xterm palette, which is what `TERM=xterm-256color` + /// promises and what the assertion layer already compared against. + fn default() -> Self { + Colors { + foreground: Rgb::new(192, 192, 192), + background: Rgb::new(0, 0, 0), + cursor: Rgb::new(192, 192, 192), + + black: Rgb::new(0, 0, 0), + red: Rgb::new(128, 0, 0), + green: Rgb::new(0, 128, 0), + yellow: Rgb::new(128, 128, 0), + blue: Rgb::new(0, 0, 128), + magenta: Rgb::new(128, 0, 128), + cyan: Rgb::new(0, 128, 128), + white: Rgb::new(192, 192, 192), + bright_black: Rgb::new(128, 128, 128), + bright_red: Rgb::new(255, 0, 0), + bright_green: Rgb::new(0, 255, 0), + bright_yellow: Rgb::new(255, 255, 0), + bright_blue: Rgb::new(0, 0, 255), + bright_magenta: Rgb::new(255, 0, 255), + bright_cyan: Rgb::new(0, 255, 255), + bright_white: Rgb::new(255, 255, 255), + } + } +} + +impl Colors { + /// The sixteen ANSI slots, in palette order. + pub fn ansi(&self) -> [Rgb; 16] { + [ + self.black, + self.red, + self.green, + self.yellow, + self.blue, + self.magenta, + self.cyan, + self.white, + self.bright_black, + self.bright_red, + self.bright_green, + self.bright_yellow, + self.bright_blue, + self.bright_magenta, + self.bright_cyan, + self.bright_white, + ] + } + + /// The name a slot goes by in the config file. + pub fn slot_name(index: u8) -> Option<&'static str> { + Some(match NamedColor::from_index(index)? { + NamedColor::Black => "black", + NamedColor::Red => "red", + NamedColor::Green => "green", + NamedColor::Yellow => "yellow", + NamedColor::Blue => "blue", + NamedColor::Magenta => "magenta", + NamedColor::Cyan => "cyan", + NamedColor::White => "white", + NamedColor::BrightBlack => "bright_black", + NamedColor::BrightRed => "bright_red", + NamedColor::BrightGreen => "bright_green", + NamedColor::BrightYellow => "bright_yellow", + NamedColor::BrightBlue => "bright_blue", + NamedColor::BrightMagenta => "bright_magenta", + NamedColor::BrightCyan => "bright_cyan", + NamedColor::BrightWhite => "bright_white", + }) + } + + /// Resolve any 256-color index. + /// + /// Slots 0-15 come from the profile. The color cube (16-231) and gray ramp + /// (232-255) are fixed by the xterm spec and identical under every profile. + pub fn rgb(&self, index: u8) -> Rgb { + match index { + 0..=15 => self.ansi()[index as usize], + 16..=231 => { + let i = index as u16 - 16; + let level = |c: u16| -> u8 { + if c == 0 { + 0 + } else { + (c * 40 + 55) as u8 + } + }; + Rgb::new(level((i / 36) % 6), level((i / 6) % 6), level(i % 6)) + } + 232..=255 => { + let v = ((index as u16 - 232) * 10 + 8) as u8; + Rgb::new(v, v, v) + } + } + } + + /// Resolve a cell's color, where `None` is the terminal default. + /// + /// This is the one function both the screenshot renderer and `expect + /// --fg/--bg` call, which is what keeps them agreeing. + pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { + match color { + None => { + if is_fg { + self.foreground + } else { + self.background + } + } + Some(Color::Named(n)) => self.rgb(n.index()), + Some(Color::Idx(i)) => self.rgb(i), + Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), + } + } +} + +/// The settings a session runs with. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct Profile { + /// Rows retained beyond the visible screen. + pub scrollback: usize, + pub colors: Colors, +} + +impl Default for Profile { + fn default() -> Self { + Profile { + scrollback: DEFAULT_SCROLLBACK, + colors: Colors::default(), + } + } +} + +/// A parsed config file: named profiles, and nothing else. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct ConfigFile { + pub profiles: BTreeMap, +} + +impl ConfigFile { + pub fn parse(toml_text: &str) -> anyhow::Result { + Ok(toml::from_str(toml_text)?) + } + + pub fn load(path: &Path) -> anyhow::Result { + let text = std::fs::read_to_string(path) + .map_err(|e| anyhow::anyhow!("could not read {}: {e}", path.display()))?; + Self::parse(&text).map_err(|e| anyhow::anyhow!("{}: {e}", path.display())) + } + + /// The named profile, or the built-in defaults when nothing is named and + /// the file defines no `default`. + pub fn profile(&self, name: Option<&str>) -> anyhow::Result { + match name { + Some(name) => self.profiles.get(name).copied().ok_or_else(|| { + let known: Vec<&str> = self.profiles.keys().map(String::as_str).collect(); + if known.is_empty() { + anyhow::anyhow!("no profile {name:?}; the config file defines none") + } else { + anyhow::anyhow!("no profile {name:?}; found: {}", known.join(", ")) + } + }), + None => Ok(self + .profiles + .get(DEFAULT_PROFILE) + .copied() + .unwrap_or_default()), + } + } +} + +/// Where a config file is looked for, nearest first. +/// +/// A project-local file wins so a repository can pin the terminal its tests +/// expect. `SHELL_USE_CONFIG` overrides both, which is also how a test suite +/// pins a config without depending on the working directory. +pub fn search_paths(cwd: &Path) -> Vec { + if let Ok(explicit) = std::env::var("SHELL_USE_CONFIG") { + return vec![PathBuf::from(explicit)]; + } + vec![ + cwd.join(CONFIG_FILE), + crate::config::home_dir().join(CONFIG_FILE), + ] +} + +/// Resolve a profile: an explicit file if given, else the first file found on +/// the search path, else the built-in defaults. +/// +/// A missing file is not an error — shell-use runs without one. A file that +/// exists but does not parse *is* an error, because silently ignoring it would +/// run the session with settings the user did not ask for. +pub fn resolve( + explicit_config: Option<&Path>, + profile_name: Option<&str>, + cwd: &Path, +) -> anyhow::Result { + if let Some(path) = explicit_config { + return ConfigFile::load(path)?.profile(profile_name); + } + for path in search_paths(cwd) { + if path.is_file() { + return ConfigFile::load(&path)?.profile(profile_name); + } + } + match profile_name { + Some(name) => anyhow::bail!("no profile {name:?}: no config file found"), + None => Ok(Profile::default()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hex_colors_round_trip() { + for raw in ["#000000", "#ffffff", "#800000", "#c0c0c0"] { + assert_eq!(Rgb::parse(raw).unwrap().to_hex(), raw); + } + assert_eq!(Rgb::parse("#f00").unwrap(), Rgb::new(255, 0, 0)); + assert_eq!(Rgb::parse("800000").unwrap(), Rgb::new(128, 0, 0)); + } + + #[test] + fn a_bad_color_says_what_it_wanted() { + for raw in ["", "#12", "#1234567", "nope", "#gggggg"] { + let err = Rgb::parse(raw).unwrap_err(); + assert!( + err.contains("color") || err.contains("hex"), + "{raw:?}: {err}" + ); + } + } + + /// A profile that says nothing is the built-in default, so a config file is + /// never required. + #[test] + fn an_empty_config_yields_the_defaults() { + let cfg = ConfigFile::parse("").unwrap(); + assert_eq!(cfg.profile(None).unwrap(), Profile::default()); + assert_eq!(Profile::default().scrollback, 10_000); + } + + /// Every field is individually optional, so a profile can set one color + /// without restating the palette. + #[test] + fn a_partial_profile_keeps_the_other_defaults() { + let cfg = ConfigFile::parse( + r##" + [profiles.ci] + scrollback = 50 + + [profiles.ci.colors] + red = "#ff0000" + "##, + ) + .unwrap(); + let p = cfg.profile(Some("ci")).unwrap(); + assert_eq!(p.scrollback, 50); + assert_eq!(p.colors.red, Rgb::new(255, 0, 0), "the override applies"); + assert_eq!( + p.colors.green, + Colors::default().green, + "an unset slot keeps its default" + ); + assert_eq!( + p.colors.background, + Colors::default().background, + "an unset default color is untouched" + ); + } + + #[test] + fn an_unknown_profile_names_the_ones_that_exist() { + let cfg = ConfigFile::parse("[profiles.ci]\n[profiles.demo]\n").unwrap(); + let err = cfg.profile(Some("nope")).unwrap_err().to_string(); + assert!(err.contains("ci") && err.contains("demo"), "{err}"); + } + + /// A typo in a key is an error rather than a setting that silently does + /// nothing. + #[test] + fn an_unknown_key_is_rejected() { + let err = ConfigFile::parse("[profiles.ci]\nscrollbacks = 10\n") + .unwrap_err() + .to_string(); + assert!(err.contains("scrollbacks"), "{err}"); + } + + /// Above the sixteen configurable slots the palette is spec, not + /// preference, so profiles cannot disagree about what `--fg 196` means. + #[test] + fn the_color_cube_ignores_the_profile() { + let recolored = Colors { + red: Rgb::new(1, 2, 3), + ..Default::default() + }; + for n in 16u8..=255 { + assert_eq!(recolored.rgb(n), Colors::default().rgb(n), "index {n}"); + } + assert_eq!(Colors::default().rgb(196), Rgb::new(255, 0, 0)); + assert_eq!(Colors::default().rgb(232), Rgb::new(8, 8, 8)); + assert_eq!(recolored.rgb(1), Rgb::new(1, 2, 3), "but slot 1 follows it"); + } + + /// Every configurable slot is reachable by the name the file uses, so the + /// documented key set and the resolver cannot drift apart. + #[test] + fn every_ansi_slot_has_a_config_key() { + for i in 0u8..16 { + let name = Colors::slot_name(i).unwrap_or_else(|| panic!("slot {i} unnamed")); + let toml = format!("[profiles.p.colors]\n{name} = \"#010203\"\n"); + let p = ConfigFile::parse(&toml) + .unwrap() + .profile(Some("p")) + .unwrap(); + assert_eq!( + p.colors.rgb(i), + Rgb::new(1, 2, 3), + "setting {name:?} must move slot {i}" + ); + } + assert_eq!(Colors::slot_name(16), None, "only 0-15 are configurable"); + } + + #[test] + fn a_cell_that_set_no_color_takes_the_profile_default() { + let c = Colors::default(); + assert_eq!(c.resolve(None, true), c.foreground); + assert_eq!(c.resolve(None, false), c.background); + assert_eq!( + c.resolve(Some(Color::Rgb(1, 2, 3)), true), + Rgb::new(1, 2, 3), + "a true-color cell is itself whatever the profile says" + ); + } + + /// A project-local file wins over the user's, so a repository can pin the + /// terminal its tests expect. `SHELL_USE_CONFIG` overrides both. + #[test] + fn the_search_order_puts_the_project_first() { + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _guard = ENV_LOCK.lock().unwrap(); + + let old = std::env::var_os("SHELL_USE_CONFIG"); + std::env::remove_var("SHELL_USE_CONFIG"); + let cwd = Path::new("/tmp/some-project"); + let result = std::panic::catch_unwind(|| { + let paths = search_paths(cwd); + assert_eq!(paths.len(), 2); + assert_eq!(paths[0], cwd.join(CONFIG_FILE), "the project file is first"); + assert!( + paths[1].ends_with(CONFIG_FILE) && paths[1] != paths[0], + "the user file is second: {:?}", + paths[1] + ); + + std::env::set_var("SHELL_USE_CONFIG", "/tmp/pinned.toml"); + let pinned = search_paths(cwd); + assert_eq!( + pinned, + vec![PathBuf::from("/tmp/pinned.toml")], + "an explicit config replaces the search entirely" + ); + }); + std::env::remove_var("SHELL_USE_CONFIG"); + if let Some(value) = old { + std::env::set_var("SHELL_USE_CONFIG", value); + } + result.unwrap(); + } + + /// Running without a config file is normal, so a missing one is not an + /// error. A file that exists but does not parse is, because ignoring it + /// would silently run with settings nobody asked for. + #[test] + fn a_missing_config_defaults_but_a_broken_one_fails() { + let dir = std::env::temp_dir().join(format!("su-profile-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + + let missing = dir.join("absent.toml"); + assert!( + resolve(Some(&missing), None, &dir).is_err(), + "named-but-absent is an error" + ); + + let broken = dir.join("broken.toml"); + std::fs::write(&broken, "[profiles.ci]\nscrollback = \"lots\"\n").unwrap(); + let err = resolve(Some(&broken), None, &dir).unwrap_err().to_string(); + assert!( + err.contains("broken.toml"), + "the error names the file: {err}" + ); + + std::fs::remove_dir_all(&dir).ok(); + } +} diff --git a/crates/shell-use/src/protocol.rs b/crates/shell-use/src/protocol.rs index 78d96df..8d1e154 100644 --- a/crates/shell-use/src/protocol.rs +++ b/crates/shell-use/src/protocol.rs @@ -35,6 +35,12 @@ pub enum Request { Open { shell: Option, program: Option>, + /// Terminal settings, already resolved from the config file by the + /// client. The daemon never reads that file: it is long-lived and + /// shared, so it has no single working directory to resolve a + /// project-local config against. + #[serde(default)] + profile: crate::profile::Profile, cols: u16, rows: u16, cwd: Option, @@ -293,6 +299,7 @@ mod tests { Request::Open { shell: None, program: None, + profile: Default::default(), cols: 80, rows: 30, cwd: None, diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index 9278220..ddce6a1 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -11,7 +11,8 @@ use std::fmt::Write; use super::nerd_font::NerdFont; -use crate::terminal::cell::{Attrs, Color, EmuCell}; +use crate::profile::{Colors, Rgb}; +use crate::terminal::cell::{Attrs, EmuCell}; const CELL_W: f32 = 10.0; const CELL_H: f32 = 21.0; @@ -24,63 +25,13 @@ const DOT_R: f32 = 7.0; const FONT_STACK: &str = "'Cascadia Code','JetBrains Mono','Fira Code',Menlo,Consolas,'DejaVu Sans Mono',monospace"; -struct Theme { - palette: [(u8, u8, u8); 16], - default_fg: (u8, u8, u8), - default_bg: (u8, u8, u8), +fn hex(c: Rgb) -> String { + c.to_hex() } -impl Default for Theme { - fn default() -> Self { - Theme { - palette: [ - (40, 45, 53), - (232, 131, 136), - (168, 204, 140), - (219, 171, 121), - (113, 190, 242), - (210, 144, 228), - (102, 194, 205), - (185, 191, 202), - (111, 119, 131), - (232, 131, 136), - (168, 204, 140), - (219, 171, 121), - (115, 190, 243), - (210, 144, 227), - (102, 194, 205), - (255, 255, 255), - ], - default_fg: (185, 191, 202), - default_bg: (40, 45, 53), - } - } -} - -impl Theme { - fn resolve(&self, color: Option, is_fg: bool) -> (u8, u8, u8) { - match color { - None => { - if is_fg { - self.default_fg - } else { - self.default_bg - } - } - Some(Color::Named(n)) => self.palette[n.index() as usize], - Some(Color::Idx(i)) => crate::assert::color::ansi256_to_rgb(i), - Some(Color::Rgb(r, g, b)) => (r, g, b), - } - } -} - -fn hex((r, g, b): (u8, u8, u8)) -> String { - format!("#{r:02x}{g:02x}{b:02x}") -} - -fn dim((r, g, b): (u8, u8, u8)) -> (u8, u8, u8) { +fn dim(c: Rgb) -> Rgb { let s = |v: u8| (v as f32 * 0.6) as u8; - (s(r), s(g), s(b)) + Rgb::new(s(c.r), s(c.g), s(c.b)) } static BLANK: EmuCell = EmuCell::blank(); @@ -90,9 +41,9 @@ fn cell_at(row: &[EmuCell], x: usize) -> &EmuCell { } /// Resolved background color for a cell (honoring inverse). -fn bg_of(cell: &EmuCell, theme: &Theme) -> (u8, u8, u8) { - let bg = theme.resolve(cell.bg, false); - let fg = theme.resolve(cell.fg, true); +fn bg_of(cell: &EmuCell, colors: &Colors) -> Rgb { + let bg = colors.resolve(cell.bg, false); + let fg = colors.resolve(cell.fg, true); if cell.has(Attrs::INVERSE) { fg } else { @@ -102,7 +53,7 @@ fn bg_of(cell: &EmuCell, theme: &Theme) -> (u8, u8, u8) { #[derive(PartialEq)] struct Style { - fg: (u8, u8, u8), + fg: Rgb, bold: bool, italic: bool, underline: bool, @@ -110,9 +61,9 @@ struct Style { invisible: bool, } -fn style_of(cell: &EmuCell, theme: &Theme) -> Style { - let mut fg = theme.resolve(cell.fg, true); - let bg = theme.resolve(cell.bg, false); +fn style_of(cell: &EmuCell, colors: &Colors) -> Style { + let mut fg = colors.resolve(cell.fg, true); + let bg = colors.resolve(cell.bg, false); if cell.has(Attrs::INVERSE) { fg = bg; } @@ -151,8 +102,7 @@ fn run_text(row: &[EmuCell], start: usize, end: usize) -> String { } /// Render a grid to a standalone SVG document. -pub fn render_svg(rows: &[Vec], cols: u16) -> String { - let theme = Theme::default(); +pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { let nerd_font = NerdFont::new(rows, FONT_SIZE); let cols = cols as usize; let x0 = MARGIN_X; @@ -169,7 +119,7 @@ pub fn render_svg(rows: &[Vec], cols: u16) -> String { let _ = write!( out, r#""#, - hex(theme.default_bg) + hex(colors.background) ); for (i, dot) in ["#ff5f56", "#ffbd2e", "#27c93f"].iter().enumerate() { let cx = MARGIN_X + 5.0 + i as f32 * 20.0; @@ -183,12 +133,12 @@ pub fn render_svg(rows: &[Vec], cols: u16) -> String { for (y, row) in rows.iter().enumerate() { let mut x = 0; while x < cols { - let bg = bg_of(cell_at(row, x), &theme); + let bg = bg_of(cell_at(row, x), colors); let mut run = 1; - while x + run < cols && bg_of(cell_at(row, x + run), &theme) == bg { + while x + run < cols && bg_of(cell_at(row, x + run), colors) == bg { run += 1; } - if bg != theme.default_bg { + if bg != colors.background { let rx = x0 + x as f32 * CELL_W; let ry = y0 + y as f32 * CELL_H; let rw = run as f32 * CELL_W; @@ -206,9 +156,9 @@ pub fn render_svg(rows: &[Vec], cols: u16) -> String { let baseline = y0 + y as f32 * CELL_H + FONT_BASELINE; let mut x = 0; while x < cols { - let style = style_of(cell_at(row, x), &theme); + let style = style_of(cell_at(row, x), colors); let mut run = 1; - while x + run < cols && style_of(cell_at(row, x + run), &theme) == style { + while x + run < cols && style_of(cell_at(row, x + run), colors) == style { run += 1; } if !style.invisible { @@ -265,6 +215,7 @@ pub fn render_svg(rows: &[Vec], cols: u16) -> String { #[cfg(test)] mod tests { use super::*; + use crate::terminal::cell::Color; fn cell(ch: &str, fg: Option, bg: Option) -> EmuCell { EmuCell { @@ -281,11 +232,14 @@ mod tests { cell("h", Some(Color::from_index(1)), None), cell("i", Some(Color::from_index(1)), None), ]]; - let svg = render_svg(&rows, 2); + let svg = render_svg(&rows, 2, &Colors::default()); assert!(svg.starts_with("")); assert!(svg.contains("textLength")); - assert!(svg.contains(&hex((232, 131, 136)))); + assert!( + svg.contains(&hex(Colors::default().rgb(1))), + "slot 1 is painted with the profile color" + ); assert!(svg.contains(">hi")); assert!(!svg.contains("")); assert!(!svg.contains("<")); } @@ -318,8 +272,11 @@ mod tests { #[test] fn background_run_emitted_for_non_default_bg() { let rows = vec![vec![cell(" ", None, Some(Color::from_index(4)))]]; - let svg = render_svg(&rows, 1); - assert!(svg.contains(&hex((113, 190, 242)))); + let svg = render_svg(&rows, 1, &Colors::default()); + assert!( + svg.contains(&hex(Colors::default().rgb(4))), + "slot 4 is painted with the profile color" + ); } #[test] @@ -330,7 +287,7 @@ mod tests { cell(glyph, None, None), cell("b", None, None), ]]; - let svg = render_svg(&rows, 3); + let svg = render_svg(&rows, 3, &Colors::default()); assert!(svg.contains(r#"")); diff --git a/crates/shell-use/src/session.rs b/crates/shell-use/src/session.rs index 41cbd35..ea5c05a 100644 --- a/crates/shell-use/src/session.rs +++ b/crates/shell-use/src/session.rs @@ -7,6 +7,7 @@ use std::thread::JoinHandle; use std::time::Instant; use crate::logger::Logger; +use crate::profile::Profile; use crate::shell::{self, Shell}; use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::emu::Emulator; @@ -26,6 +27,8 @@ pub struct TermState { pub struct Session { pub shell: Option, + /// Settings this session was opened with, fixed for its lifetime. + pub profile: Profile, pub cols: u16, pub rows: u16, /// Per-class timeout defaults for the lifetime of this session. @@ -49,6 +52,7 @@ impl Session { pub fn open( shell: Option, program: Option>, + profile: Profile, cols: u16, rows: u16, cwd: Option, @@ -76,7 +80,7 @@ impl Session { }; let state = Arc::new(Mutex::new(TermState { - emu: Box::new(AlacrittyEmu::new(cols, rows, 5_000)), + emu: Box::new(AlacrittyEmu::new(cols, rows, profile.scrollback)), tracker: CommandTracker::new(), last_change: Instant::now(), awaiting_start: None, @@ -139,6 +143,7 @@ impl Session { Ok(Session { shell, + profile, cols, rows, timeouts, diff --git a/crates/shell-use/tests/runtime.rs b/crates/shell-use/tests/runtime.rs index 0044715..df3420a 100644 --- a/crates/shell-use/tests/runtime.rs +++ b/crates/shell-use/tests/runtime.rs @@ -13,6 +13,7 @@ fn named_runtimes_share_a_process_local_terminal() { Request::Open { shell: None, program: None, + profile: Default::default(), cols: DEFAULT_COLS, rows: DEFAULT_ROWS, cwd: None, @@ -75,6 +76,7 @@ fn named_runtimes_share_a_process_local_terminal() { Request::Open { shell: None, program: None, + profile: Default::default(), cols: DEFAULT_COLS, rows: DEFAULT_ROWS, cwd: None, @@ -99,6 +101,7 @@ fn unrelated_session_state_does_not_wait_behind_another_session() { Request::Open { shell: None, program: None, + profile: Default::default(), cols: DEFAULT_COLS, rows: DEFAULT_ROWS, cwd: None, From 9900781db19e4ea97abec971cdaa827850e9917d Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 15:06:29 -0400 Subject: [PATCH 2/8] feat(terminal): answer OSC color queries and track dynamic colors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Programs ask the terminal what color it is before deciding whether to draw for a light or a dark background. Nothing answered, so every one of them blocked until it timed out and guessed. The emulator answers now, through `take_pending_writes`, which already exists for exactly this: the replies a terminal owes to device queries. alacritty parses the sequence, tracks what a program set, and hands back a formatter with the query's own prefix and terminator already captured, so the only missing piece was the color itself. That comes from the session profile, which the emulator is now constructed with. The alternative was parsing the sequences off the PTY stream, the way shell integration is tracked. That would have meant reimplementing color parsing, the runtime table, reply formatting, and terminator tracking, all of which the emulator already does — and getting the terminator wrong, since it is only visible to whoever parsed the sequence. Reading what the emulator already knows is both less code and more faithful. Colors resolve in three layers: what a program set, else the session profile, else the table the specification defines. A reset clears only the first, so the profile is unreachable from the byte stream and there is always something to restore. That is what the specification asks for, describing a reset as restoring "the color specified by the corresponding X resource". `Emulator` gains `color(slot)`, which every backend answers from its own state, plus a `palette()` snapshot of all 259 slots. The screenshot renderer and `expect --fg/--bg` take that snapshot rather than the emulator, so neither holds the session lock while it renders, and neither knows which backend produced the colors. Five conformance cases cover queries, terminator echo, set-then-reset, unconfigured indices, and that a cell follows whatever its slot now holds. They run against every backend, so a future one cannot answer differently. An end-to-end test drives a real program through the whole path: it reads the configured background, sets its own, resets, and gets the configured one back. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- SKILL.md | 6 + crates/shell-use-cli/src/monitor.rs | 2 +- .../shell-use-cli/tests/session_lifecycle.rs | 74 +++++++++ crates/shell-use/src/assert/color.rs | 35 +++- crates/shell-use/src/engine.rs | 12 +- crates/shell-use/src/profile.rs | 46 ++++++ crates/shell-use/src/render/svg.rs | 36 ++-- crates/shell-use/src/session.rs | 2 +- crates/shell-use/src/terminal/alacritty.rs | 95 ++++++++++- crates/shell-use/src/terminal/conformance.rs | 156 +++++++++++++++++- crates/shell-use/src/terminal/emu.rs | 37 +++++ 11 files changed, 459 insertions(+), 42 deletions(-) diff --git a/SKILL.md b/SKILL.md index fc185db..c82e0cc 100644 --- a/SKILL.md +++ b/SKILL.md @@ -321,6 +321,12 @@ stable across profiles. The palette is what a screenshot paints **and** what `expect --fg/--bg` matches a `#rrggbb` against, so the two always agree. +Programs can also set and query colours at runtime with `OSC 4/10/11/12` and +reset them with `OSC 104/110/111/112`. A query is answered with the colour +currently showing; a reset restores the profile's colour, which no escape +sequence can change. Note that a program setting a colour also changes what a +screenshot of that session looks like. + ## Supported shells & integration `open --shell S` accepts: `bash`, `zsh`, `fish`, `powershell`, `pwsh`, `cmd`, diff --git a/crates/shell-use-cli/src/monitor.rs b/crates/shell-use-cli/src/monitor.rs index 4fb14c8..e08b985 100644 --- a/crates/shell-use-cli/src/monitor.rs +++ b/crates/shell-use-cli/src/monitor.rs @@ -395,7 +395,7 @@ mod tests { ]; for want in styles { - let mut emu = AlacrittyEmu::new(10, 2, 0); + let mut emu = AlacrittyEmu::new(10, 2, &shell_use::profile::Profile::default()); emu.process(want.sgr().as_bytes()); emu.process(b"x"); let got = Style::from(&emu.viewable_rows()[0][0]); diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index 67e95e7..8ff72fb 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -343,6 +343,80 @@ fn an_unknown_profile_is_rejected() { ); } +/// A program that asks the terminal what color it is gets an answer. +/// +/// This is how tools decide whether they are on a light or a dark background. +/// A terminal that stays silent leaves them blocked until they time out and +/// guess, so this drives the whole path: daemon, emulator, and the reply on +/// its way back up the PTY. +#[test] +fn a_color_query_is_answered_over_the_pty() { + let sandbox = Sandbox::new("osc-query"); + let probe = sandbox.home.join("probe.py"); + std::fs::write( + &probe, + r#" +import os, sys, termios, tty, select + +# Unbuffered reads: a buffered reader would take bytes off the fd that +# select() then cannot see, and the reply would look truncated. +def ask(fd, query): + os.write(1, query) + buf = b"" + while select.select([fd], [], [], 2.0)[0]: + buf += os.read(fd, 64) + if buf.endswith(b"\x07"): + break + return buf.decode("utf8", "replace") + +fd = sys.stdin.fileno() +old = termios.tcgetattr(fd) +try: + tty.setraw(fd) + configured = ask(fd, b"\x1b]11;?\x07") + os.write(1, b"\x1b]11;#654321\x07") + overridden = ask(fd, b"\x1b]11;?\x07") + os.write(1, b"\x1b]111\x07") + restored = ask(fd, b"\x1b]11;?\x07") +finally: + termios.tcsetattr(fd, termios.TCSADRAIN, old) + +strip = lambda s: s.replace("\x1b", "").replace("\x07", "") +print("\r\nRESULT %s %s %s\r" % (strip(configured), strip(overridden), strip(restored))) +"#, + ) + .expect("write probe"); + + sandbox.ok(&["run", "--cols", "80", "--", "bash", "--norc"]); + sandbox.ok(&[ + "submit", + &format!("python3 {}", probe.to_str().expect("utf-8 path")), + ]); + sandbox.ok(&["wait", "command"]); + let text = sandbox.ok(&["text", "--full"]); + + let line = text + .lines() + .find(|l| l.contains("RESULT")) + .unwrap_or_else(|| panic!("the probe never reported: {text}")); + + // The default profile's background is black, so the terminal reports it, + // then the color the program set, then the configured one again. + assert!( + line.contains("]11;rgb:0000/0000/0000"), + "the configured background should be reported: {line}" + ); + assert!( + line.contains("]11;rgb:6565/4343/2121"), + "a set color should be reported back: {line}" + ); + assert_eq!( + line.matches("]11;rgb:0000/0000/0000").count(), + 2, + "a reset should restore the configured background: {line}" + ); +} + #[test] fn state_reports_effective_timeouts() { let sandbox = Sandbox::new("state-timeouts"); diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 56105ef..040788c 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -1,7 +1,7 @@ //! Color parsing and comparison for `expect --fg/--bg`. use super::super::terminal::cell::Color; -use crate::profile::Colors; +use crate::profile::Palette; /// The spelling of [`Expected::Default`], on the command line and in messages. pub const DEFAULT: &str = "default"; @@ -74,7 +74,7 @@ fn parse_hex(hex: &str) -> anyhow::Result<(u8, u8, u8)> { /// the screenshot renderer draws with. These used to be two separate hardcoded /// tables that disagreed on every ANSI slot, so `expect --fg "#800000"` passed /// on a cell a screenshot painted `#e88388`. -pub fn matches(cell: Option, expected: &Expected, colors: &Colors) -> bool { +pub fn matches(cell: Option, expected: &Expected, colors: &Palette) -> bool { let Some(cell) = cell else { return matches!(expected, Expected::Default); }; @@ -89,7 +89,7 @@ pub fn matches(cell: Option, expected: &Expected, colors: &Colors) -> boo } /// Render a cell's color in the same space as the expected value, for messages. -pub fn describe_cell(cell: Option, expected: &Expected, colors: &Colors) -> String { +pub fn describe_cell(cell: Option, expected: &Expected, colors: &Palette) -> String { let Some(cell) = cell else { return DEFAULT.to_string(); }; @@ -124,7 +124,24 @@ pub fn rgb_to_ansi256(r: u8, g: u8, b: u8) -> u8 { #[cfg(test)] mod tests { use super::*; + use crate::profile::{Colors, Profile}; + use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::cell::Color; + use crate::terminal::emu::Emulator; + + /// Snapshotted from a real emulator, so these exercise the same path a + /// session uses rather than a stand-in that could drift from it. + fn emu_with(colors: Colors) -> Palette { + AlacrittyEmu::new( + 10, + 2, + &Profile { + colors, + ..Default::default() + }, + ) + .palette() + } #[test] fn parse_forms() { @@ -144,7 +161,7 @@ mod tests { #[test] fn matches_palette_and_default() { - let c = Colors::default(); + let c = emu_with(Colors::default()); let idx = |i| Some(Color::from_index(i)); assert!(matches(idx(9), &Expected::Ansi256(9), &c)); assert!(!matches(idx(2), &Expected::Ansi256(9), &c)); @@ -161,7 +178,7 @@ mod tests { /// `default` keyword, which is the way to assert on it. #[test] fn default_color_matches_only_default() { - let c = Colors::default(); + let c = emu_with(Colors::default()); assert!(!matches(None, &Expected::Ansi256(0), &c)); assert!(!matches(None, &Expected::Hex(0, 0, 0), &c)); assert!(matches(None, &Expected::Default, &c)); @@ -170,7 +187,7 @@ mod tests { #[test] fn a_colored_cell_is_not_default() { - let c = Colors::default(); + let c = emu_with(Colors::default()); let red = Some(Color::from_index(1)); assert!(!matches(red, &Expected::Default, &c)); assert!(matches(red, &Expected::Ansi256(1), &c)); @@ -190,7 +207,7 @@ mod tests { /// value for every slot, because both come from the profile. #[test] fn an_assertion_matches_the_color_a_screenshot_paints() { - let colors = Colors::default(); + let colors = emu_with(Colors::default()); for index in 0u8..=255 { let cell = Some(Color::from_index(index)); let painted = colors.resolve(cell, true); @@ -210,10 +227,10 @@ mod tests { /// profiles genuinely disagree rather than sharing one hardcoded table. #[test] fn a_recolored_profile_moves_what_an_assertion_matches() { - let colors = Colors { + let colors = emu_with(Colors { red: crate::profile::Rgb::new(1, 2, 3), ..Default::default() - }; + }); let red = Some(Color::from_index(1)); assert!(matches(red, &Expected::Hex(1, 2, 3), &colors)); assert!(!matches(red, &Expected::Hex(128, 0, 0), &colors)); diff --git a/crates/shell-use/src/engine.rs b/crates/shell-use/src/engine.rs index 34d55ff..403da33 100644 --- a/crates/shell-use/src/engine.rs +++ b/crates/shell-use/src/engine.rs @@ -284,6 +284,12 @@ fn await_ready(s: &Session, timeout_ms: u64) -> bool { } } +/// Snapshot the colors the session is currently showing. Taken before +/// rendering or asserting so neither holds the session lock while it works. +fn palette(s: &Session) -> crate::profile::Palette { + s.state.lock().unwrap().emu.palette() +} + fn viewable(s: &Session) -> Vec> { s.state.lock().unwrap().emu.viewable_rows() } @@ -727,7 +733,7 @@ fn expect_text( let ok = poll_until( || match locator::find(&grid(s, full), &pattern, strict) { Ok(Some(cells)) if !cells.is_empty() => { - if let Some(err) = check_colors(&cells, &fg, &bg, not, &s.profile.colors) { + if let Some(err) = check_colors(&cells, &fg, &bg, not, &palette(s)) { last_err = Some(err); false } else { @@ -757,7 +763,7 @@ fn check_colors( fg: &Option, bg: &Option, not: bool, - colors: &crate::profile::Colors, + colors: &crate::profile::Palette, ) -> Option { let want = !not; if let Some(spec) = fg { @@ -870,7 +876,7 @@ fn screenshot(s: &Session, full: bool, path: Option) -> Response { let rows = grid(s, full); match path { Some(path) => { - let svg = crate::render::svg::render_svg(&rows, s.cols, &s.profile.colors); + let svg = crate::render::svg::render_svg(&rows, s.cols, &palette(s)); match std::fs::write(&path, svg) { Ok(()) => Response::with(json!({ "path": path })), Err(e) => Response::internal(e.to_string()), diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs index ec98799..a2c9e00 100644 --- a/crates/shell-use/src/profile.rs +++ b/crates/shell-use/src/profile.rs @@ -236,6 +236,52 @@ impl Colors { } } +/// The colors a session is showing right now. +/// +/// A snapshot of every slot, taken from the emulator, so the screenshot +/// renderer and `expect --fg/--bg` can resolve a cell without holding the +/// session lock or knowing which backend produced it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Palette { + slots: [Rgb; 259], +} + +impl Palette { + pub fn new(slots: [Rgb; 259]) -> Self { + Palette { slots } + } + + pub fn color(&self, slot: usize) -> Rgb { + self.slots.get(slot).copied().unwrap_or(self.slots[256]) + } + + /// Resolve a cell's color, where `None` is the terminal default. The grid + /// records the slot a cell chose, never a color, so this is where a cell + /// becomes something to paint or compare. + pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { + match color { + None => self.color(if is_fg { 256 } else { 257 }), + Some(Color::Named(n)) => self.color(n.index() as usize), + Some(Color::Idx(i)) => self.color(i as usize), + Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), + } + } +} + +impl Default for Palette { + fn default() -> Self { + let config = Colors::default(); + let mut slots = [config.foreground; 259]; + for (i, slot) in slots.iter_mut().enumerate().take(256) { + *slot = config.rgb(i as u8); + } + slots[256] = config.foreground; + slots[257] = config.background; + slots[258] = config.cursor; + Palette { slots } + } +} + /// The settings a session runs with. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(default, deny_unknown_fields)] diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index ddce6a1..e88710c 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -11,7 +11,7 @@ use std::fmt::Write; use super::nerd_font::NerdFont; -use crate::profile::{Colors, Rgb}; +use crate::profile::{Palette, Rgb}; use crate::terminal::cell::{Attrs, EmuCell}; const CELL_W: f32 = 10.0; @@ -41,7 +41,7 @@ fn cell_at(row: &[EmuCell], x: usize) -> &EmuCell { } /// Resolved background color for a cell (honoring inverse). -fn bg_of(cell: &EmuCell, colors: &Colors) -> Rgb { +fn bg_of(cell: &EmuCell, colors: &Palette) -> Rgb { let bg = colors.resolve(cell.bg, false); let fg = colors.resolve(cell.fg, true); if cell.has(Attrs::INVERSE) { @@ -61,7 +61,7 @@ struct Style { invisible: bool, } -fn style_of(cell: &EmuCell, colors: &Colors) -> Style { +fn style_of(cell: &EmuCell, colors: &Palette) -> Style { let mut fg = colors.resolve(cell.fg, true); let bg = colors.resolve(cell.bg, false); if cell.has(Attrs::INVERSE) { @@ -102,7 +102,7 @@ fn run_text(row: &[EmuCell], start: usize, end: usize) -> String { } /// Render a grid to a standalone SVG document. -pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { +pub fn render_svg(rows: &[Vec], cols: u16, colors: &Palette) -> String { let nerd_font = NerdFont::new(rows, FONT_SIZE); let cols = cols as usize; let x0 = MARGIN_X; @@ -119,7 +119,7 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { let _ = write!( out, r#""#, - hex(colors.background) + hex(colors.resolve(None, false)) ); for (i, dot) in ["#ff5f56", "#ffbd2e", "#27c93f"].iter().enumerate() { let cx = MARGIN_X + 5.0 + i as f32 * 20.0; @@ -138,7 +138,7 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { while x + run < cols && bg_of(cell_at(row, x + run), colors) == bg { run += 1; } - if bg != colors.background { + if bg != colors.resolve(None, false) { let rx = x0 + x as f32 * CELL_W; let ry = y0 + y as f32 * CELL_H; let rw = run as f32 * CELL_W; @@ -217,6 +217,10 @@ mod tests { use super::*; use crate::terminal::cell::Color; + fn colors() -> Palette { + Palette::default() + } + fn cell(ch: &str, fg: Option, bg: Option) -> EmuCell { EmuCell { ch: ch.into(), @@ -232,12 +236,12 @@ mod tests { cell("h", Some(Color::from_index(1)), None), cell("i", Some(Color::from_index(1)), None), ]]; - let svg = render_svg(&rows, 2, &Colors::default()); + let svg = render_svg(&rows, 2, &colors()); assert!(svg.starts_with("")); assert!(svg.contains("textLength")); assert!( - svg.contains(&hex(Colors::default().rgb(1))), + svg.contains(&hex(colors().color(1))), "slot 1 is painted with the profile color" ); assert!(svg.contains(">hi")); @@ -247,7 +251,7 @@ mod tests { #[test] fn emits_window_chrome() { - let svg = render_svg(&[vec![cell(" ", None, None)]], 1, &Colors::default()); + let svg = render_svg(&[vec![cell(" ", None, None)]], 1, &colors()); assert!(svg.contains("<")); } @@ -272,9 +276,9 @@ mod tests { #[test] fn background_run_emitted_for_non_default_bg() { let rows = vec![vec![cell(" ", None, Some(Color::from_index(4)))]]; - let svg = render_svg(&rows, 1, &Colors::default()); + let svg = render_svg(&rows, 1, &colors()); assert!( - svg.contains(&hex(Colors::default().rgb(4))), + svg.contains(&hex(colors().color(4))), "slot 4 is painted with the profile color" ); } @@ -287,7 +291,7 @@ mod tests { cell(glyph, None, None), cell("b", None, None), ]]; - let svg = render_svg(&rows, 3, &Colors::default()); + let svg = render_svg(&rows, 3, &colors()); assert!(svg.contains(r#"")); diff --git a/crates/shell-use/src/session.rs b/crates/shell-use/src/session.rs index ea5c05a..c3379a1 100644 --- a/crates/shell-use/src/session.rs +++ b/crates/shell-use/src/session.rs @@ -80,7 +80,7 @@ impl Session { }; let state = Arc::new(Mutex::new(TermState { - emu: Box::new(AlacrittyEmu::new(cols, rows, profile.scrollback)), + emu: Box::new(AlacrittyEmu::new(cols, rows, &profile)), tracker: CommandTracker::new(), last_change: Instant::now(), awaiting_start: None, diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 47c41a6..6ac8d05 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -12,11 +12,13 @@ use alacritty_terminal::term::cell::Flags as AlacFlags; use alacritty_terminal::term::test::TermSize; use alacritty_terminal::term::{Config as AlacConfig, Term}; use alacritty_terminal::vte::ansi; +use alacritty_terminal::vte::ansi::Rgb as AlacRgb; use compact_str::{CompactString, ToCompactString}; +use crate::profile::{Colors, Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; -use crate::terminal::emu::Emulator; +use crate::terminal::emu::{self, Emulator}; /// Alacritty's palette colors arrive either as a `Named` variant or an index; /// both funnel through [`Color::from_index`] so a given slot always yields the @@ -112,17 +114,37 @@ fn cell_from_alac(c: &alacritty_terminal::term::cell::Cell) -> EmuCell { } } +/// Formats a color query's reply once the color is known. alacritty builds +/// this closure with the query's own prefix and terminator already captured, +/// so the answer echoes the form the program asked in. +type ReplyFormat = Arc String + Send + Sync>; + +/// Queues what the terminal wants to say back to the PTY. +/// +/// Color queries cannot be answered here: the color lives in the terminal's +/// own palette, and this listener is constructed before the terminal it +/// listens to. They are parked instead, and [`AlacrittyEmu::answer_queries`] +/// resolves them once `process` returns, where the terminal is in scope. #[derive(Default, Clone)] struct CaptureProxy { pending: Arc>>, + queries: Arc>>, } impl EventListener for CaptureProxy { fn send_event(&self, ev: Event) { - if let Event::PtyWrite(bytes) = ev { - if let Ok(mut buf) = self.pending.lock() { - buf.extend_from_slice(bytes.as_bytes()); + match ev { + Event::PtyWrite(bytes) => { + if let Ok(mut buf) = self.pending.lock() { + buf.extend_from_slice(bytes.as_bytes()); + } + } + Event::ColorRequest(slot, format) => { + if let Ok(mut queries) = self.queries.lock() { + queries.push((slot, format)); + } } + _ => {} } } } @@ -133,25 +155,62 @@ pub struct AlacrittyEmu { cols: u16, rows: u16, pending: Arc>>, + queries: Arc>>, + /// The colors this session was configured with. A program can shadow them + /// at runtime but never reach them, so a reset always has a value to + /// restore. + config: Colors, } impl AlacrittyEmu { - pub fn new(cols: u16, rows: u16, scrollback: usize) -> Self { + pub fn new(cols: u16, rows: u16, profile: &Profile) -> Self { let size = TermSize::new(cols as usize, rows as usize); - let config = AlacConfig { - scrolling_history: scrollback, + let alac_config = AlacConfig { + scrolling_history: profile.scrollback, ..Default::default() }; let pending: Arc>> = Arc::default(); + let queries: Arc>> = Arc::default(); let proxy = CaptureProxy { pending: pending.clone(), + queries: queries.clone(), }; AlacrittyEmu { - term: Term::new(config, &size, proxy), + term: Term::new(alac_config, &size, proxy), processor: ansi::Processor::new(), cols, rows, pending, + queries, + config: profile.colors, + } + } + + /// Answer any color queries parked while the last chunk was parsed. + /// + /// alacritty stores a color a program set, and leaves the slot empty + /// otherwise, so an empty slot is answered from the session profile. + fn answer_queries(&mut self) { + let parked: Vec<(usize, ReplyFormat)> = match self.queries.lock() { + Ok(mut queries) => queries.drain(..).collect(), + Err(_) => return, + }; + if parked.is_empty() { + return; + } + let replies: String = parked + .into_iter() + .map(|(slot, format)| { + let c = self.color(slot); + format(AlacRgb { + r: c.r, + g: c.g, + b: c.b, + }) + }) + .collect(); + if let Ok(mut buf) = self.pending.lock() { + buf.extend_from_slice(replies.as_bytes()); } } @@ -173,6 +232,9 @@ impl AlacrittyEmu { impl Emulator for AlacrittyEmu { fn process(&mut self, bytes: &[u8]) { self.processor.advance(&mut self.term, bytes); + // Queries are answered here rather than in the listener because the + // terminal holding the palette is only in scope once parsing is done. + self.answer_queries(); } fn take_pending_writes(&mut self) -> Vec { @@ -204,6 +266,21 @@ impl Emulator for AlacrittyEmu { self.rows_in_range(0, self.rows as i32) } + fn color(&self, slot: usize) -> Rgb { + // `Colors` stores only what a program set; an empty slot means the + // session's configured color still shows through. + if let Some(set) = self.term.colors()[slot] { + return Rgb::new(set.r, set.g, set.b); + } + match slot { + emu::FOREGROUND => self.config.foreground, + emu::BACKGROUND => self.config.background, + emu::CURSOR => self.config.cursor, + i if i < emu::FOREGROUND => self.config.rgb(i as u8), + _ => self.config.foreground, + } + } + fn full_rows(&self) -> Vec> { let grid = self.term.grid(); let total = grid.total_lines() as i32; @@ -217,5 +294,5 @@ impl Emulator for AlacrittyEmu { mod tests { use super::*; - crate::emulator_conformance_tests!(|c, r, s| Box::new(AlacrittyEmu::new(c, r, s))); + crate::emulator_conformance_tests!(|c, r, p| Box::new(AlacrittyEmu::new(c, r, p))); } diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index bee931a..6b5fc85 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -16,7 +16,7 @@ //! the test pins only the part that is universal and says why it stops short. /// Generates the conformance tests for one backend. `$make` builds a boxed -/// emulator from `(cols, rows, scrollback)`. +/// emulator from `(cols, rows, &Profile)`. /// /// The body is fully path-qualified because it expands into the caller's /// module; it must not collide with whatever that module already imports. @@ -28,8 +28,27 @@ macro_rules! emulator_conformance_tests { rows: u16, scrollback: usize, ) -> Box { - let make: fn(u16, u16, usize) -> Box = $make; - make(cols, rows, scrollback) + conformance_emu_with( + cols, + rows, + $crate::profile::Profile { + scrollback, + ..Default::default() + }, + ) + } + + fn conformance_emu_with( + cols: u16, + rows: u16, + profile: $crate::profile::Profile, + ) -> Box { + let make: fn( + u16, + u16, + &$crate::profile::Profile, + ) -> Box = $make; + make(cols, rows, &profile) } /// Row text with trailing blanks removed, for readable assertions. @@ -542,6 +561,137 @@ macro_rules! emulator_conformance_tests { ); } + /// A color query is answered with the session's configured color. + /// + /// Programs query the background to decide whether they are on a light + /// or a dark terminal. A backend that stays silent leaves them blocked + /// until they time out and guess. + #[test] + fn conformance_color_queries_are_answered() { + use $crate::profile::{Colors, Profile, Rgb}; + let profile = Profile { + colors: Colors { + background: Rgb::new(0x12, 0x34, 0x56), + red: Rgb::new(0xab, 0xcd, 0xef), + ..Default::default() + }, + ..Default::default() + }; + let mut e = conformance_emu_with(10, 4, profile); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]11;?\x07"); + assert_eq!( + String::from_utf8_lossy(&e.take_pending_writes()), + "\x1b]11;rgb:1212/3434/5656\x07", + "OSC 11 must report the configured background" + ); + + e.process(b"\x1b]4;1;?\x07"); + assert_eq!( + String::from_utf8_lossy(&e.take_pending_writes()), + "\x1b]4;1;rgb:abab/cdcd/efef\x07", + "OSC 4 must report the configured palette entry" + ); + } + + /// A reply uses the terminator the query used. A program that reads + /// until the terminator it sent would otherwise wait for one that + /// never comes. + #[test] + fn conformance_a_color_reply_echoes_the_terminator() { + let mut e = conformance_emu(10, 4, 100); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]11;?\x07"); + let bel = e.take_pending_writes(); + assert!( + bel.ends_with(b"\x07"), + "a BEL query is answered with BEL: {:?}", + String::from_utf8_lossy(&bel) + ); + + e.process(b"\x1b]11;?\x1b\\"); + let st = e.take_pending_writes(); + assert!( + st.ends_with(b"\x1b\\"), + "an ST query is answered with ST: {:?}", + String::from_utf8_lossy(&st) + ); + } + + /// A program can shadow a color, and a reset puts the configured one + /// back. The configured color is never reachable, so a reset always + /// has something to restore. + #[test] + fn conformance_a_color_set_is_undone_by_a_reset() { + use $crate::profile::{Colors, Profile, Rgb}; + let configured = Rgb::new(0x11, 0x22, 0x33); + let profile = Profile { + colors: Colors { + background: configured, + ..Default::default() + }, + ..Default::default() + }; + let mut e = conformance_emu_with(10, 4, profile); + let background = $crate::terminal::emu::BACKGROUND; + assert_eq!(e.color(background), configured); + + e.process(b"\x1b]11;#654321\x07"); + assert_eq!( + e.color(background), + Rgb::new(0x65, 0x43, 0x21), + "a set shadows the configured color" + ); + + e.process(b"\x1b]111\x07"); + assert_eq!( + e.color(background), + configured, + "OSC 111 restores the configured color" + ); + + // The same for a palette entry, which resets with OSC 104. + e.process(b"\x1b]4;2;#010203\x07"); + assert_eq!(e.color(2), Rgb::new(1, 2, 3)); + e.process(b"\x1b]104;2\x07"); + assert_eq!(e.color(2), Colors::default().green); + } + + /// An unconfigured palette entry still answers, from the table the + /// specification defines for it. + #[test] + fn conformance_an_unconfigured_index_resolves_from_the_spec_table() { + use $crate::profile::Rgb; + let e = conformance_emu(10, 4, 100); + assert_eq!( + e.color(196), + Rgb::new(255, 0, 0), + "index 196 is pure red in the xterm color cube" + ); + assert_eq!(e.color(232), Rgb::new(8, 8, 8), "the gray ramp starts at 8"); + } + + /// A cell records which slot it chose, never a color, so what it + /// paints follows whatever that slot currently holds. + #[test] + fn conformance_a_cell_follows_its_slot() { + use $crate::profile::Rgb; + let mut e = conformance_emu(10, 4, 100); + e.process(b"\x1b[31mR"); + let cell = e.viewable_rows()[0][0].clone(); + + let before = e.palette().resolve(cell.fg, true); + e.process(b"\x1b]4;1;#0a0b0c\x07"); + assert_eq!( + e.palette().resolve(cell.fg, true), + Rgb::new(0x0a, 0x0b, 0x0c), + "recoloring the slot recolors the cell that chose it" + ); + assert_ne!(before, e.palette().resolve(cell.fg, true)); + } + /// The alternate screen hides primary content and restores it on exit. #[test] fn conformance_alt_screen_round_trip() { diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index 83c0ba7..9da0c00 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -10,8 +10,18 @@ //! identical shell-integration behavior by construction rather than by //! reimplementation. +use crate::profile::{Palette, Rgb}; use crate::terminal::cell::EmuCell; +/// Runtime color slots: the 256-color palette, then the three dynamic colors. +/// +/// The numbering is not ours — both emulators already address their special +/// colors this way, so a backend can hand its own table straight through. +pub const FOREGROUND: usize = 256; +pub const BACKGROUND: usize = 257; +pub const CURSOR: usize = 258; +pub const COLOR_SLOTS: usize = 259; + /// A headless terminal emulator: bytes in, cell grid out. /// /// Implementations must be `Send`; the daemon shares the emulator across its @@ -40,4 +50,31 @@ pub trait Emulator: Send { /// Scrollback history followed by the visible screen. fn full_rows(&self) -> Vec>; + + /// The color a slot currently shows. + /// + /// Programs move these with `OSC 4` (palette) and `OSC 10/11/12` (default + /// foreground, background, cursor), and put them back with `OSC 104` and + /// `OSC 110/111/112`. A reset restores the color the session was configured + /// with; nothing a program sends can change that configured value, so + /// there is always something to fall back to. + /// + /// Backends answer color *queries* themselves, through + /// [`Emulator::take_pending_writes`], because each one already parses the + /// sequence and knows which terminator the query used. This method is how + /// the screenshot renderer and `expect --fg/--bg` see the same answer. + /// + /// `slot` is a palette index, or one of [`FOREGROUND`], [`BACKGROUND`], + /// [`CURSOR`]. + fn color(&self, slot: usize) -> Rgb; + + /// Every slot at once, so a consumer can resolve colors without holding + /// the session lock or knowing which backend produced them. + fn palette(&self) -> Palette { + let mut slots = [Rgb::new(0, 0, 0); COLOR_SLOTS]; + for (slot, out) in slots.iter_mut().enumerate() { + *out = self.color(slot); + } + Palette::new(slots) + } } From 915bc8b59ac1040d1d6fe9866d6eaf71b8cfade3 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 15:22:49 -0400 Subject: [PATCH 3/8] refactor(terminal): resolve colors in the emulator against its profile Follow-up to the previous commit, which routed color resolution through a `Palette` snapshot the session passed around. That put the fallback in the wrong place: the emulator reported only what a program had set, and every consumer had to know how to fill in the rest. The emulator now takes the session profile at construction and answers `color(slot)` for any slot, mapping its own table onto the profile when nothing has overridden it. Consumers ask the emulator and get a color, with no second layer to consult. `Palette` is gone. The 256-color table above the sixteen configurable slots is a static built at compile time rather than arithmetic run per lookup. It is the same in every terminal, so computing it repeatedly only invited the two implementations of it to drift. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/assert/color.rs | 13 +- crates/shell-use/src/engine.rs | 15 +- crates/shell-use/src/profile.rs | 211 ++++++++++++------- crates/shell-use/src/render/svg.rs | 17 +- crates/shell-use/src/terminal/alacritty.rs | 40 ++-- crates/shell-use/src/terminal/conformance.rs | 8 +- crates/shell-use/src/terminal/emu.rs | 52 ++--- 7 files changed, 200 insertions(+), 156 deletions(-) diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 040788c..5cf89ea 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -1,7 +1,7 @@ //! Color parsing and comparison for `expect --fg/--bg`. use super::super::terminal::cell::Color; -use crate::profile::Palette; +use crate::terminal::emu::Emulator; /// The spelling of [`Expected::Default`], on the command line and in messages. pub const DEFAULT: &str = "default"; @@ -74,7 +74,7 @@ fn parse_hex(hex: &str) -> anyhow::Result<(u8, u8, u8)> { /// the screenshot renderer draws with. These used to be two separate hardcoded /// tables that disagreed on every ANSI slot, so `expect --fg "#800000"` passed /// on a cell a screenshot painted `#e88388`. -pub fn matches(cell: Option, expected: &Expected, colors: &Palette) -> bool { +pub fn matches(cell: Option, expected: &Expected, colors: &dyn Emulator) -> bool { let Some(cell) = cell else { return matches!(expected, Expected::Default); }; @@ -89,7 +89,7 @@ pub fn matches(cell: Option, expected: &Expected, colors: &Palette) -> bo } /// Render a cell's color in the same space as the expected value, for messages. -pub fn describe_cell(cell: Option, expected: &Expected, colors: &Palette) -> String { +pub fn describe_cell(cell: Option, expected: &Expected, colors: &dyn Emulator) -> String { let Some(cell) = cell else { return DEFAULT.to_string(); }; @@ -129,9 +129,9 @@ mod tests { use crate::terminal::cell::Color; use crate::terminal::emu::Emulator; - /// Snapshotted from a real emulator, so these exercise the same path a - /// session uses rather than a stand-in that could drift from it. - fn emu_with(colors: Colors) -> Palette { + /// A real emulator, so these exercise the same resolution path a session + /// uses rather than a stand-in that could drift from it. + fn emu_with(colors: Colors) -> AlacrittyEmu { AlacrittyEmu::new( 10, 2, @@ -140,7 +140,6 @@ mod tests { ..Default::default() }, ) - .palette() } #[test] diff --git a/crates/shell-use/src/engine.rs b/crates/shell-use/src/engine.rs index 403da33..584b780 100644 --- a/crates/shell-use/src/engine.rs +++ b/crates/shell-use/src/engine.rs @@ -284,12 +284,6 @@ fn await_ready(s: &Session, timeout_ms: u64) -> bool { } } -/// Snapshot the colors the session is currently showing. Taken before -/// rendering or asserting so neither holds the session lock while it works. -fn palette(s: &Session) -> crate::profile::Palette { - s.state.lock().unwrap().emu.palette() -} - fn viewable(s: &Session) -> Vec> { s.state.lock().unwrap().emu.viewable_rows() } @@ -733,7 +727,9 @@ fn expect_text( let ok = poll_until( || match locator::find(&grid(s, full), &pattern, strict) { Ok(Some(cells)) if !cells.is_empty() => { - if let Some(err) = check_colors(&cells, &fg, &bg, not, &palette(s)) { + if let Some(err) = + check_colors(&cells, &fg, &bg, not, s.state.lock().unwrap().emu.as_ref()) + { last_err = Some(err); false } else { @@ -763,7 +759,7 @@ fn check_colors( fg: &Option, bg: &Option, not: bool, - colors: &crate::profile::Palette, + colors: &dyn crate::terminal::emu::Emulator, ) -> Option { let want = !not; if let Some(spec) = fg { @@ -876,7 +872,8 @@ fn screenshot(s: &Session, full: bool, path: Option) -> Response { let rows = grid(s, full); match path { Some(path) => { - let svg = crate::render::svg::render_svg(&rows, s.cols, &palette(s)); + let svg = + crate::render::svg::render_svg(&rows, s.cols, s.state.lock().unwrap().emu.as_ref()); match std::fs::write(&path, svg) { Ok(()) => Response::with(json!({ "path": path })), Err(e) => Response::internal(e.to_string()), diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs index a2c9e00..64b630e 100644 --- a/crates/shell-use/src/profile.rs +++ b/crates/shell-use/src/profile.rs @@ -22,7 +22,7 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use crate::terminal::cell::{Color, NamedColor}; +use crate::terminal::cell::NamedColor; /// Rows of scrollback a profile retains when it does not say otherwise. /// @@ -193,93 +193,99 @@ impl Colors { /// Resolve any 256-color index. /// - /// Slots 0-15 come from the profile. The color cube (16-231) and gray ramp - /// (232-255) are fixed by the xterm spec and identical under every profile. + /// Slots 0-15 come from the profile; everything above comes from the + /// xterm table, which no profile can move. pub fn rgb(&self, index: u8) -> Rgb { match index { 0..=15 => self.ansi()[index as usize], - 16..=231 => { - let i = index as u16 - 16; - let level = |c: u16| -> u8 { - if c == 0 { - 0 - } else { - (c * 40 + 55) as u8 - } - }; - Rgb::new(level((i / 36) % 6), level((i / 6) % 6), level(i % 6)) - } - 232..=255 => { - let v = ((index as u16 - 232) * 10 + 8) as u8; - Rgb::new(v, v, v) - } + _ => xterm_color(index), } } - /// Resolve a cell's color, where `None` is the terminal default. + /// The color a slot shows when no program has overridden it. /// - /// This is the one function both the screenshot renderer and `expect - /// --fg/--bg` call, which is what keeps them agreeing. - pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { - match color { - None => { - if is_fg { - self.foreground - } else { - self.background - } - } - Some(Color::Named(n)) => self.rgb(n.index()), - Some(Color::Idx(i)) => self.rgb(i), - Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), + /// This is what a backend falls back to: `slot` is a palette index, or one + /// of [`FOREGROUND`], [`BACKGROUND`], [`CURSOR`], matching how `OSC 4` and + /// `OSC 10/11/12` address them. + pub fn color(&self, slot: usize) -> Rgb { + match slot { + FOREGROUND => self.foreground, + BACKGROUND => self.background, + CURSOR => self.cursor, + index if index < FOREGROUND => self.rgb(index as u8), + // Above the addressable range there is nothing sensible to report; + // the foreground is the least surprising answer. + _ => self.foreground, } } } -/// The colors a session is showing right now. +/// Where the three dynamic colors sit when a color slot is addressed by +/// number. /// -/// A snapshot of every slot, taken from the emulator, so the screenshot -/// renderer and `expect --fg/--bg` can resolve a cell without holding the -/// session lock or knowing which backend produced it. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct Palette { - slots: [Rgb; 259], -} - -impl Palette { - pub fn new(slots: [Rgb; 259]) -> Self { - Palette { slots } - } - - pub fn color(&self, slot: usize) -> Rgb { - self.slots.get(slot).copied().unwrap_or(self.slots[256]) - } - - /// Resolve a cell's color, where `None` is the terminal default. The grid - /// records the slot a cell chose, never a color, so this is where a cell - /// becomes something to paint or compare. - pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { - match color { - None => self.color(if is_fg { 256 } else { 257 }), - Some(Color::Named(n)) => self.color(n.index() as usize), - Some(Color::Idx(i)) => self.color(i as usize), - Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), - } - } +/// The numbering is not ours: `OSC 4` addresses the 256-color palette, and +/// `OSC 10/11/12` address the three colors after it. Both emulators already +/// index their tables this way, so a backend reads its own state directly. +pub const FOREGROUND: usize = 256; +pub const BACKGROUND: usize = 257; +pub const CURSOR: usize = 258; + +/// The xterm 256-color table, which is the same in every terminal. +/// +/// Slots 0-15 here are the classic VGA colors, and a profile overrides them. +/// The rest is the 6x6x6 color cube and the 24-step gray ramp, which the +/// specification fixes and no profile can move: `--fg 196` has to mean the +/// same thing in every session. +static XTERM_256: [Rgb; 256] = build_xterm_256(); + +const fn build_xterm_256() -> [Rgb; 256] { + let mut table = [Rgb::new(0, 0, 0); 256]; + + // 0-15: VGA. + let vga = [ + (0, 0, 0), + (128, 0, 0), + (0, 128, 0), + (128, 128, 0), + (0, 0, 128), + (128, 0, 128), + (0, 128, 128), + (192, 192, 192), + (128, 128, 128), + (255, 0, 0), + (0, 255, 0), + (255, 255, 0), + (0, 0, 255), + (255, 0, 255), + (0, 255, 255), + (255, 255, 255), + ]; + let mut i = 0; + while i < 16 { + table[i] = Rgb::new(vga[i].0, vga[i].1, vga[i].2); + i += 1; + } + + // 16-231: a 6x6x6 cube whose levels step 0, 95, 135, 175, 215, 255. + let levels = [0u8, 95, 135, 175, 215, 255]; + while i < 232 { + let n = i - 16; + table[i] = Rgb::new(levels[(n / 36) % 6], levels[(n / 6) % 6], levels[n % 6]); + i += 1; + } + + // 232-255: a gray ramp from 8 to 238 in steps of 10. + while i < 256 { + let v = (i - 232) as u8 * 10 + 8; + table[i] = Rgb::new(v, v, v); + i += 1; + } + table } -impl Default for Palette { - fn default() -> Self { - let config = Colors::default(); - let mut slots = [config.foreground; 259]; - for (i, slot) in slots.iter_mut().enumerate().take(256) { - *slot = config.rgb(i as u8); - } - slots[256] = config.foreground; - slots[257] = config.background; - slots[258] = config.cursor; - Palette { slots } - } +/// The color a slot has when nothing has overridden it. +pub fn xterm_color(index: u8) -> Rgb { + XTERM_256[index as usize] } /// The settings a session runs with. @@ -494,15 +500,60 @@ mod tests { assert_eq!(Colors::slot_name(16), None, "only 0-15 are configurable"); } + /// Every slot a program can address resolves, so a backend always has a + /// color to fall back to and a query always has an answer. #[test] - fn a_cell_that_set_no_color_takes_the_profile_default() { + fn every_addressable_slot_resolves() { let c = Colors::default(); - assert_eq!(c.resolve(None, true), c.foreground); - assert_eq!(c.resolve(None, false), c.background); + for index in 0u8..=255 { + assert_eq!(c.color(index as usize), c.rgb(index), "slot {index}"); + } + assert_eq!(c.color(FOREGROUND), c.foreground); + assert_eq!(c.color(BACKGROUND), c.background); + assert_eq!(c.color(CURSOR), c.cursor); + } + + /// The 16 configurable slots come from the profile; the rest come from the + /// xterm table, which is the same in every terminal. + #[test] + fn only_the_ansi_slots_follow_the_profile() { + let recolored = Colors { + red: Rgb::new(1, 2, 3), + ..Default::default() + }; + assert_eq!(recolored.rgb(1), Rgb::new(1, 2, 3), "slot 1 follows it"); + for index in 16u8..=255 { + assert_eq!( + recolored.rgb(index), + xterm_color(index), + "slot {index} is fixed by the specification" + ); + } + } + + /// Spot-check the static table against the values the specification + /// defines, so a typo in 256 entries cannot pass unnoticed. + #[test] + fn the_xterm_table_matches_the_specification() { + assert_eq!(xterm_color(0), Rgb::new(0, 0, 0), "VGA black"); + assert_eq!(xterm_color(1), Rgb::new(128, 0, 0), "VGA red"); + assert_eq!(xterm_color(15), Rgb::new(255, 255, 255), "VGA bright white"); + assert_eq!( + xterm_color(16), + Rgb::new(0, 0, 0), + "the cube starts at black" + ); + assert_eq!(xterm_color(196), Rgb::new(255, 0, 0), "cube red"); + assert_eq!( + xterm_color(231), + Rgb::new(255, 255, 255), + "the cube ends white" + ); + assert_eq!(xterm_color(232), Rgb::new(8, 8, 8), "the ramp starts at 8"); assert_eq!( - c.resolve(Some(Color::Rgb(1, 2, 3)), true), - Rgb::new(1, 2, 3), - "a true-color cell is itself whatever the profile says" + xterm_color(255), + Rgb::new(238, 238, 238), + "the ramp ends at 238" ); } diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index e88710c..3a3f0b1 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -11,8 +11,9 @@ use std::fmt::Write; use super::nerd_font::NerdFont; -use crate::profile::{Palette, Rgb}; +use crate::profile::Rgb; use crate::terminal::cell::{Attrs, EmuCell}; +use crate::terminal::emu::Emulator; const CELL_W: f32 = 10.0; const CELL_H: f32 = 21.0; @@ -41,7 +42,7 @@ fn cell_at(row: &[EmuCell], x: usize) -> &EmuCell { } /// Resolved background color for a cell (honoring inverse). -fn bg_of(cell: &EmuCell, colors: &Palette) -> Rgb { +fn bg_of(cell: &EmuCell, colors: &dyn Emulator) -> Rgb { let bg = colors.resolve(cell.bg, false); let fg = colors.resolve(cell.fg, true); if cell.has(Attrs::INVERSE) { @@ -61,7 +62,7 @@ struct Style { invisible: bool, } -fn style_of(cell: &EmuCell, colors: &Palette) -> Style { +fn style_of(cell: &EmuCell, colors: &dyn Emulator) -> Style { let mut fg = colors.resolve(cell.fg, true); let bg = colors.resolve(cell.bg, false); if cell.has(Attrs::INVERSE) { @@ -102,7 +103,7 @@ fn run_text(row: &[EmuCell], start: usize, end: usize) -> String { } /// Render a grid to a standalone SVG document. -pub fn render_svg(rows: &[Vec], cols: u16, colors: &Palette) -> String { +pub fn render_svg(rows: &[Vec], cols: u16, colors: &dyn Emulator) -> String { let nerd_font = NerdFont::new(rows, FONT_SIZE); let cols = cols as usize; let x0 = MARGIN_X; @@ -215,10 +216,14 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &Palette) -> String #[cfg(test)] mod tests { use super::*; + use crate::profile::Profile; + use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::cell::Color; - fn colors() -> Palette { - Palette::default() + /// A real emulator: the renderer resolves through the same path a session + /// uses, so a stand-in could not drift from it. + fn colors() -> AlacrittyEmu { + AlacrittyEmu::new(10, 2, &Profile::default()) } fn cell(ch: &str, fg: Option, bg: Option) -> EmuCell { diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 6ac8d05..24bc20e 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -16,9 +16,9 @@ use alacritty_terminal::vte::ansi::Rgb as AlacRgb; use compact_str::{CompactString, ToCompactString}; -use crate::profile::{Colors, Profile, Rgb}; +use crate::profile::{Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; -use crate::terminal::emu::{self, Emulator}; +use crate::terminal::emu::Emulator; /// Alacritty's palette colors arrive either as a `Named` variant or an index; /// both funnel through [`Color::from_index`] so a given slot always yields the @@ -156,10 +156,10 @@ pub struct AlacrittyEmu { rows: u16, pending: Arc>>, queries: Arc>>, - /// The colors this session was configured with. A program can shadow them - /// at runtime but never reach them, so a reset always has a value to - /// restore. - config: Colors, + /// The settings this session was opened with. A program can shadow the + /// colors at runtime but never reach them, so a reset always has a value + /// to restore. + profile: Profile, } impl AlacrittyEmu { @@ -182,14 +182,12 @@ impl AlacrittyEmu { rows, pending, queries, - config: profile.colors, + profile: *profile, } } /// Answer any color queries parked while the last chunk was parsed. /// - /// alacritty stores a color a program set, and leaves the slot empty - /// otherwise, so an empty slot is answered from the session profile. fn answer_queries(&mut self) { let parked: Vec<(usize, ReplyFormat)> = match self.queries.lock() { Ok(mut queries) => queries.drain(..).collect(), @@ -255,6 +253,15 @@ impl Emulator for AlacrittyEmu { (self.cols, self.rows) } + /// alacritty stores only what a program set, leaving every other slot + /// empty, so an empty slot means the profile's color still shows through. + fn color(&self, slot: usize) -> Rgb { + match self.term.colors()[slot] { + Some(set) => Rgb::new(set.r, set.g, set.b), + None => self.profile.colors.color(slot), + } + } + fn cursor(&self) -> (u16, u16) { let p = self.term.grid().cursor.point; let y = p.line.0.max(0).min(self.rows as i32 - 1) as u16; @@ -266,21 +273,6 @@ impl Emulator for AlacrittyEmu { self.rows_in_range(0, self.rows as i32) } - fn color(&self, slot: usize) -> Rgb { - // `Colors` stores only what a program set; an empty slot means the - // session's configured color still shows through. - if let Some(set) = self.term.colors()[slot] { - return Rgb::new(set.r, set.g, set.b); - } - match slot { - emu::FOREGROUND => self.config.foreground, - emu::BACKGROUND => self.config.background, - emu::CURSOR => self.config.cursor, - i if i < emu::FOREGROUND => self.config.rgb(i as u8), - _ => self.config.foreground, - } - } - fn full_rows(&self) -> Vec> { let grid = self.term.grid(); let total = grid.total_lines() as i32; diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index 6b5fc85..2672a59 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -635,7 +635,7 @@ macro_rules! emulator_conformance_tests { ..Default::default() }; let mut e = conformance_emu_with(10, 4, profile); - let background = $crate::terminal::emu::BACKGROUND; + let background = $crate::profile::BACKGROUND; assert_eq!(e.color(background), configured); e.process(b"\x1b]11;#654321\x07"); @@ -682,14 +682,14 @@ macro_rules! emulator_conformance_tests { e.process(b"\x1b[31mR"); let cell = e.viewable_rows()[0][0].clone(); - let before = e.palette().resolve(cell.fg, true); + let before = e.resolve(cell.fg, true); e.process(b"\x1b]4;1;#0a0b0c\x07"); assert_eq!( - e.palette().resolve(cell.fg, true), + e.resolve(cell.fg, true), Rgb::new(0x0a, 0x0b, 0x0c), "recoloring the slot recolors the cell that chose it" ); - assert_ne!(before, e.palette().resolve(cell.fg, true)); + assert_ne!(before, e.resolve(cell.fg, true)); } /// The alternate screen hides primary content and restores it on exit. diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index 9da0c00..de43d4d 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -10,17 +10,8 @@ //! identical shell-integration behavior by construction rather than by //! reimplementation. -use crate::profile::{Palette, Rgb}; -use crate::terminal::cell::EmuCell; - -/// Runtime color slots: the 256-color palette, then the three dynamic colors. -/// -/// The numbering is not ours — both emulators already address their special -/// colors this way, so a backend can hand its own table straight through. -pub const FOREGROUND: usize = 256; -pub const BACKGROUND: usize = 257; -pub const CURSOR: usize = 258; -pub const COLOR_SLOTS: usize = 259; +use crate::profile::Rgb; +use crate::terminal::cell::{Color, EmuCell}; /// A headless terminal emulator: bytes in, cell grid out. /// @@ -51,30 +42,39 @@ pub trait Emulator: Send { /// Scrollback history followed by the visible screen. fn full_rows(&self) -> Vec>; - /// The color a slot currently shows. + /// The color a slot is currently showing. /// /// Programs move these with `OSC 4` (palette) and `OSC 10/11/12` (default /// foreground, background, cursor), and put them back with `OSC 104` and - /// `OSC 110/111/112`. A reset restores the color the session was configured - /// with; nothing a program sends can change that configured value, so - /// there is always something to fall back to. + /// `OSC 110/111/112`. A slot nothing has overridden shows the color the + /// session's profile gives it, so a reset always has something to restore + /// and this always has an answer. /// /// Backends answer color *queries* themselves, through /// [`Emulator::take_pending_writes`], because each one already parses the - /// sequence and knows which terminator the query used. This method is how - /// the screenshot renderer and `expect --fg/--bg` see the same answer. + /// sequence and knows which terminator the query used. This reports the + /// same colors, so a screenshot and `expect --fg/--bg` agree with what a + /// program was told. /// - /// `slot` is a palette index, or one of [`FOREGROUND`], [`BACKGROUND`], - /// [`CURSOR`]. + /// `slot` is a palette index, or one of [`crate::profile::FOREGROUND`], + /// [`crate::profile::BACKGROUND`], [`crate::profile::CURSOR`]. fn color(&self, slot: usize) -> Rgb; - /// Every slot at once, so a consumer can resolve colors without holding - /// the session lock or knowing which backend produced them. - fn palette(&self) -> Palette { - let mut slots = [Rgb::new(0, 0, 0); COLOR_SLOTS]; - for (slot, out) in slots.iter_mut().enumerate() { - *out = self.color(slot); + /// Resolve a cell's color, where `None` is the terminal default. + /// + /// The grid records which slot a cell chose, never a color, so this is + /// where a cell becomes something to paint or compare. Provided rather + /// than required so every backend resolves a cell identically. + fn resolve(&self, color: Option, is_fg: bool) -> Rgb { + match color { + None => self.color(if is_fg { + crate::profile::FOREGROUND + } else { + crate::profile::BACKGROUND + }), + Some(Color::Named(n)) => self.color(n.index() as usize), + Some(Color::Idx(i)) => self.color(i as usize), + Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), } - Palette::new(slots) } } From 9de66758deb857c8d6f11b96d57165985b3aba58 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 15:28:57 -0400 Subject: [PATCH 4/8] refactor(profile): name color slots instead of numbering them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The profile addressed colors by index, using 256, 257, 258 for the foreground, background, and cursor. That is alacritty's layout — it stores the dynamic colors after the palette — and it had spread into the profile, the emulator trait, and the conformance suite, none of which have any reason to know it. A backend that numbered its own table differently would have had to pretend otherwise. Slots are a `ColorSlot` enum now: `Indexed(u8)`, `Foreground`, `Background`, `Cursor`. The alacritty backend translates that to its own indices inside the one match that reads its table, and resolves an unset slot to the profile, falling back to the xterm table for an index the profile does not name. The tests for that resolution moved to the backend that performs it, where they exercise the real path rather than a helper that mirrored it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/profile.rs | 49 +++-------- crates/shell-use/src/render/svg.rs | 6 +- crates/shell-use/src/terminal/alacritty.rs | 88 ++++++++++++++++++-- crates/shell-use/src/terminal/conformance.rs | 20 +++-- crates/shell-use/src/terminal/emu.rs | 14 ++-- 5 files changed, 117 insertions(+), 60 deletions(-) diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs index 64b630e..698afd7 100644 --- a/crates/shell-use/src/profile.rs +++ b/crates/shell-use/src/profile.rs @@ -201,34 +201,20 @@ impl Colors { _ => xterm_color(index), } } - - /// The color a slot shows when no program has overridden it. - /// - /// This is what a backend falls back to: `slot` is a palette index, or one - /// of [`FOREGROUND`], [`BACKGROUND`], [`CURSOR`], matching how `OSC 4` and - /// `OSC 10/11/12` address them. - pub fn color(&self, slot: usize) -> Rgb { - match slot { - FOREGROUND => self.foreground, - BACKGROUND => self.background, - CURSOR => self.cursor, - index if index < FOREGROUND => self.rgb(index as u8), - // Above the addressable range there is nothing sensible to report; - // the foreground is the least surprising answer. - _ => self.foreground, - } - } } -/// Where the three dynamic colors sit when a color slot is addressed by -/// number. +/// A color a program can address. /// -/// The numbering is not ours: `OSC 4` addresses the 256-color palette, and -/// `OSC 10/11/12` address the three colors after it. Both emulators already -/// index their tables this way, so a backend reads its own state directly. -pub const FOREGROUND: usize = 256; -pub const BACKGROUND: usize = 257; -pub const CURSOR: usize = 258; +/// `OSC 4` names a palette entry and `OSC 10/11/12` name the three defaults. +/// Emulators number these however they like internally, so each backend +/// translates its own layout and that numbering never reaches here. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ColorSlot { + Indexed(u8), + Foreground, + Background, + Cursor, +} /// The xterm 256-color table, which is the same in every terminal. /// @@ -500,19 +486,6 @@ mod tests { assert_eq!(Colors::slot_name(16), None, "only 0-15 are configurable"); } - /// Every slot a program can address resolves, so a backend always has a - /// color to fall back to and a query always has an answer. - #[test] - fn every_addressable_slot_resolves() { - let c = Colors::default(); - for index in 0u8..=255 { - assert_eq!(c.color(index as usize), c.rgb(index), "slot {index}"); - } - assert_eq!(c.color(FOREGROUND), c.foreground); - assert_eq!(c.color(BACKGROUND), c.background); - assert_eq!(c.color(CURSOR), c.cursor); - } - /// The 16 configurable slots come from the profile; the rest come from the /// xterm table, which is the same in every terminal. #[test] diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index 3a3f0b1..c2f1666 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -216,7 +216,7 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &dyn Emulator) -> St #[cfg(test)] mod tests { use super::*; - use crate::profile::Profile; + use crate::profile::{ColorSlot, Profile}; use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::cell::Color; @@ -246,7 +246,7 @@ mod tests { assert!(svg.ends_with("")); assert!(svg.contains("textLength")); assert!( - svg.contains(&hex(colors().color(1))), + svg.contains(&hex(colors().color(ColorSlot::Indexed(1)))), "slot 1 is painted with the profile color" ); assert!(svg.contains(">hi")); @@ -283,7 +283,7 @@ mod tests { let rows = vec![vec![cell(" ", None, Some(Color::from_index(4)))]]; let svg = render_svg(&rows, 1, &colors()); assert!( - svg.contains(&hex(colors().color(4))), + svg.contains(&hex(colors().color(ColorSlot::Indexed(4)))), "slot 4 is painted with the profile color" ); } diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 24bc20e..5fdb9c3 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -12,11 +12,12 @@ use alacritty_terminal::term::cell::Flags as AlacFlags; use alacritty_terminal::term::test::TermSize; use alacritty_terminal::term::{Config as AlacConfig, Term}; use alacritty_terminal::vte::ansi; +use alacritty_terminal::vte::ansi::NamedColor; use alacritty_terminal::vte::ansi::Rgb as AlacRgb; use compact_str::{CompactString, ToCompactString}; -use crate::profile::{Profile, Rgb}; +use crate::profile::{xterm_color, ColorSlot, Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; use crate::terminal::emu::Emulator; @@ -198,7 +199,13 @@ impl AlacrittyEmu { } let replies: String = parked .into_iter() - .map(|(slot, format)| { + .map(|(index, format)| { + let slot = match index { + i if i == NamedColor::Foreground as usize => ColorSlot::Foreground, + i if i == NamedColor::Background as usize => ColorSlot::Background, + i if i == NamedColor::Cursor as usize => ColorSlot::Cursor, + i => ColorSlot::Indexed(i as u8), + }; let c = self.color(slot); format(AlacRgb { r: c.r, @@ -254,11 +261,30 @@ impl Emulator for AlacrittyEmu { } /// alacritty stores only what a program set, leaving every other slot - /// empty, so an empty slot means the profile's color still shows through. - fn color(&self, slot: usize) -> Rgb { - match self.term.colors()[slot] { + /// empty, so an empty slot falls through to the profile, and then to the + /// xterm table for an index the profile does not name. + /// + /// The indices are alacritty's own: it lays its table out as the + /// 256-color palette followed by the dynamic colors, which is why the + /// three are 256, 257, 258. That layout stops here. + fn color(&self, slot: ColorSlot) -> Rgb { + let colors = &self.profile.colors; + let (index, configured) = match slot { + ColorSlot::Indexed(index) => ( + index as usize, + colors + .ansi() + .get(index as usize) + .copied() + .unwrap_or_else(|| xterm_color(index)), + ), + ColorSlot::Foreground => (NamedColor::Foreground as usize, colors.foreground), + ColorSlot::Background => (NamedColor::Background as usize, colors.background), + ColorSlot::Cursor => (NamedColor::Cursor as usize, colors.cursor), + }; + match self.term.colors()[index] { Some(set) => Rgb::new(set.r, set.g, set.b), - None => self.profile.colors.color(slot), + None => configured, } } @@ -287,4 +313,54 @@ mod tests { use super::*; crate::emulator_conformance_tests!(|c, r, p| Box::new(AlacrittyEmu::new(c, r, p))); + + /// Every slot a program can address resolves, so a query always has an + /// answer and a reset always has something to restore. The profile names + /// sixteen; everything above falls through to the xterm table. + #[test] + fn every_slot_resolves_through_the_profile_then_xterm() { + use crate::profile::{Colors, Rgb}; + let profile = Profile { + colors: Colors { + red: Rgb::new(1, 2, 3), + background: Rgb::new(4, 5, 6), + ..Default::default() + }, + ..Default::default() + }; + let emu = AlacrittyEmu::new(10, 2, &profile); + + assert_eq!( + emu.color(ColorSlot::Indexed(1)), + Rgb::new(1, 2, 3), + "the profile names slot 1" + ); + assert_eq!(emu.color(ColorSlot::Background), Rgb::new(4, 5, 6)); + assert_eq!( + emu.color(ColorSlot::Foreground), + Colors::default().foreground, + "an unset profile color keeps its default" + ); + for index in 16u8..=255 { + assert_eq!( + emu.color(ColorSlot::Indexed(index)), + xterm_color(index), + "slot {index} is not the profile's to name" + ); + } + } + + /// A program's color outranks the profile until it is reset, at which + /// point the profile shows through again. + #[test] + fn a_program_color_outranks_the_profile_until_reset() { + let mut emu = AlacrittyEmu::new(10, 2, &Profile::default()); + let configured = emu.color(ColorSlot::Background); + + emu.process(b"\x1b]11;#123456\x07"); + assert_eq!(emu.color(ColorSlot::Background), Rgb::new(0x12, 0x34, 0x56)); + + emu.process(b"\x1b]111\x07"); + assert_eq!(emu.color(ColorSlot::Background), configured); + } } diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index 2672a59..df2c708 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -635,7 +635,7 @@ macro_rules! emulator_conformance_tests { ..Default::default() }; let mut e = conformance_emu_with(10, 4, profile); - let background = $crate::profile::BACKGROUND; + let background = $crate::profile::ColorSlot::Background; assert_eq!(e.color(background), configured); e.process(b"\x1b]11;#654321\x07"); @@ -654,9 +654,15 @@ macro_rules! emulator_conformance_tests { // The same for a palette entry, which resets with OSC 104. e.process(b"\x1b]4;2;#010203\x07"); - assert_eq!(e.color(2), Rgb::new(1, 2, 3)); + assert_eq!( + e.color($crate::profile::ColorSlot::Indexed(2)), + Rgb::new(1, 2, 3) + ); e.process(b"\x1b]104;2\x07"); - assert_eq!(e.color(2), Colors::default().green); + assert_eq!( + e.color($crate::profile::ColorSlot::Indexed(2)), + Colors::default().green + ); } /// An unconfigured palette entry still answers, from the table the @@ -666,11 +672,15 @@ macro_rules! emulator_conformance_tests { use $crate::profile::Rgb; let e = conformance_emu(10, 4, 100); assert_eq!( - e.color(196), + e.color($crate::profile::ColorSlot::Indexed(196)), Rgb::new(255, 0, 0), "index 196 is pure red in the xterm color cube" ); - assert_eq!(e.color(232), Rgb::new(8, 8, 8), "the gray ramp starts at 8"); + assert_eq!( + e.color($crate::profile::ColorSlot::Indexed(232)), + Rgb::new(8, 8, 8), + "the gray ramp starts at 8" + ); } /// A cell records which slot it chose, never a color, so what it diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index de43d4d..edbcb41 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -10,7 +10,7 @@ //! identical shell-integration behavior by construction rather than by //! reimplementation. -use crate::profile::Rgb; +use crate::profile::{ColorSlot, Rgb}; use crate::terminal::cell::{Color, EmuCell}; /// A headless terminal emulator: bytes in, cell grid out. @@ -56,9 +56,7 @@ pub trait Emulator: Send { /// same colors, so a screenshot and `expect --fg/--bg` agree with what a /// program was told. /// - /// `slot` is a palette index, or one of [`crate::profile::FOREGROUND`], - /// [`crate::profile::BACKGROUND`], [`crate::profile::CURSOR`]. - fn color(&self, slot: usize) -> Rgb; + fn color(&self, slot: ColorSlot) -> Rgb; /// Resolve a cell's color, where `None` is the terminal default. /// @@ -68,12 +66,12 @@ pub trait Emulator: Send { fn resolve(&self, color: Option, is_fg: bool) -> Rgb { match color { None => self.color(if is_fg { - crate::profile::FOREGROUND + ColorSlot::Foreground } else { - crate::profile::BACKGROUND + ColorSlot::Background }), - Some(Color::Named(n)) => self.color(n.index() as usize), - Some(Color::Idx(i)) => self.color(i as usize), + Some(Color::Named(n)) => self.color(ColorSlot::Indexed(n.index())), + Some(Color::Idx(i)) => self.color(ColorSlot::Indexed(i)), Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), } } From 68cb9f6777eb692858f99f5c79b147d291f922a2 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 16:23:16 -0400 Subject: [PATCH 5/8] test(snapshot): pin that a snapshot records slots, not colors A snapshot stores the palette slot a cell chose rather than the color that slot resolves to, which is what lets a saved baseline outlive a profile change: recoloring a terminal would otherwise invalidate every snapshot in a suite at once. That was already true and nothing checked it, so a change to how colors are serialized could have quietly made snapshots profile-dependent. The companion case pins the exception: a true-color cell names its own color, so that one is recorded literally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/assert/snapshot.rs | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/shell-use/src/assert/snapshot.rs b/crates/shell-use/src/assert/snapshot.rs index ca0f00a..186b686 100644 --- a/crates/shell-use/src/assert/snapshot.rs +++ b/crates/shell-use/src/assert/snapshot.rs @@ -149,6 +149,43 @@ mod tests { use super::*; use crate::terminal::cell::CONTINUATION; + /// A snapshot records the palette *slot* a cell chose, never the color + /// that slot resolves to. + /// + /// This is what lets a saved baseline outlive a profile change: the same + /// screen recorded under two profiles that disagree about what red looks + /// like still produces the same snapshot, so recoloring a terminal does + /// not invalidate every snapshot in a suite. + #[test] + fn a_snapshot_records_the_slot_rather_than_the_color() { + let colored = EmuCell { + ch: "x".into(), + fg: Some(Color::from_index(1)), + ..EmuCell::blank() + }; + let out = serialize(&[vec![colored]], 1, true); + assert!( + out.contains("\"fg\": 1"), + "the slot is recorded, not an rgb value: {out}" + ); + assert!( + !out.contains('#'), + "a palette color must not be resolved into the snapshot: {out}" + ); + } + + /// A true-color cell names its own color, so that one *is* recorded + /// literally: no profile can change what `38;2;r;g;b` means. + #[test] + fn a_true_color_cell_records_its_own_value() { + let rgb = EmuCell { + ch: "x".into(), + fg: Some(Color::Rgb(0x11, 0x22, 0x33)), + ..EmuCell::blank() + }; + assert!(serialize(&[vec![rgb]], 1, true).contains("#112233")); + } + fn cell(s: &str) -> EmuCell { EmuCell { ch: s.into(), From 1fe6420a64dbfa15e42abca033180dad74c94f3c Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 16:29:32 -0400 Subject: [PATCH 6/8] test(color): cover setting the foreground and cursor, and rendering after The color tests leaned almost entirely on `OSC 11`. That is the sequence programs actually reach for, but it meant the foreground and cursor were never set, never queried, and never reset, so wiring any of them to the wrong slot would have gone unnoticed. Three conformance cases now set all three to distinct colors and check that each reset frees only its own, that each answers its own query, and that a bare `OSC 104` resets the palette without touching them. Nothing covered the path from an escape sequence to a rendered pixel either. Both halves are pinned now: a screenshot paints the background a program set and recolors a cell whose slot it moved, and an assertion matches that same color while still comparing the index unchanged. Both return to the profile after a reset. They read the same state, so this is the earlier "a screenshot and an assertion agree" guarantee held at every point in a session rather than only at the start. The end-to-end test drives all three dynamic colors over a real PTY. It needs a wide terminal: its report is one line, and `text` returns the grid, so a narrower one wrapped the reply out of the assertion's reach. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- .../shell-use-cli/tests/session_lifecycle.rs | 23 ++++- crates/shell-use/src/assert/color.rs | 48 ++++++++++ crates/shell-use/src/render/svg.rs | 49 ++++++++++ crates/shell-use/src/terminal/conformance.rs | 92 +++++++++++++++++++ 4 files changed, 208 insertions(+), 4 deletions(-) diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index 8ff72fb..aabc5f3 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -374,20 +374,27 @@ old = termios.tcgetattr(fd) try: tty.setraw(fd) configured = ask(fd, b"\x1b]11;?\x07") - os.write(1, b"\x1b]11;#654321\x07") + # Every dynamic colour, not just the background: a program that sets the + # foreground and cursor has to be answered about those too. + os.write(1, b"\x1b]10;#abcdef\x07\x1b]11;#654321\x07\x1b]12;#fedcba\x07") + fg = ask(fd, b"\x1b]10;?\x07") overridden = ask(fd, b"\x1b]11;?\x07") + cursor = ask(fd, b"\x1b]12;?\x07") os.write(1, b"\x1b]111\x07") restored = ask(fd, b"\x1b]11;?\x07") finally: termios.tcsetattr(fd, termios.TCSADRAIN, old) strip = lambda s: s.replace("\x1b", "").replace("\x07", "") -print("\r\nRESULT %s %s %s\r" % (strip(configured), strip(overridden), strip(restored))) +print("\r\nRESULT %s %s %s %s %s\r" % ( + strip(configured), strip(fg), strip(overridden), strip(cursor), strip(restored))) "#, ) .expect("write probe"); - sandbox.ok(&["run", "--cols", "80", "--", "bash", "--norc"]); + // Wide enough that the report is one unwrapped line: `text` returns the + // grid, so a wrapped reply would be split across rows. + sandbox.ok(&["run", "--cols", "200", "--", "bash", "--norc"]); sandbox.ok(&[ "submit", &format!("python3 {}", probe.to_str().expect("utf-8 path")), @@ -408,7 +415,15 @@ print("\r\nRESULT %s %s %s\r" % (strip(configured), strip(overridden), strip(res ); assert!( line.contains("]11;rgb:6565/4343/2121"), - "a set color should be reported back: {line}" + "a set background should be reported back: {line}" + ); + assert!( + line.contains("]10;rgb:abab/cdcd/efef"), + "a set foreground should be reported back: {line}" + ); + assert!( + line.contains("]12;rgb:fefe/dcdc/baba"), + "a set cursor color should be reported back: {line}" ); assert_eq!( line.matches("]11;rgb:0000/0000/0000").count(), diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 5cf89ea..ed6b245 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -222,6 +222,54 @@ mod tests { } } + /// An assertion compares against what the terminal is *currently* + /// showing, so a program that recolors a slot changes what matches. + /// + /// This is the other half of the screenshot test: both read the same + /// state, so a colour a screenshot paints is a colour an assertion + /// matches, at every point in a session rather than only at the start. + #[test] + fn an_assertion_follows_a_color_a_program_set() { + use crate::terminal::emu::Emulator; + let mut emu = emu_with(Colors::default()); + let red = Some(Color::from_index(1)); + let configured = Colors::default().red; + + assert!(matches( + red, + &Expected::Hex(configured.r, configured.g, configured.b), + &emu + )); + + emu.process(b"\x1b]4;1;#22c55e\x07"); + assert!( + matches(red, &Expected::Hex(0x22, 0xc5, 0x5e), &emu), + "the assertion follows the colour the program set" + ); + assert!( + !matches( + red, + &Expected::Hex(configured.r, configured.g, configured.b), + &emu + ), + "the configured colour is no longer what slot 1 shows" + ); + assert!( + matches(red, &Expected::Ansi256(1), &emu), + "the index is unaffected: it names a slot, not a colour" + ); + + emu.process(b"\x1b]104;1\x07"); + assert!( + matches( + red, + &Expected::Hex(configured.r, configured.g, configured.b), + &emu + ), + "a reset restores the configured colour" + ); + } + /// A profile's palette is what an assertion compares against, so two /// profiles genuinely disagree rather than sharing one hardcoded table. #[test] diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index c2f1666..ff15d99 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -235,6 +235,55 @@ mod tests { } } + /// A program that repaints the terminal repaints the screenshot. + /// + /// The renderer draws what the terminal is currently showing, not what it + /// was configured with, so a background set with `OSC 11` is the one that + /// gets painted. Nothing else covers the path from an escape sequence to + /// a rendered pixel. + #[test] + fn a_screenshot_follows_colors_a_program_set() { + use crate::terminal::emu::Emulator; + let mut emu = colors(); + let rows = vec![vec![cell("x", Some(Color::from_index(1)), None)]]; + + let before = render_svg(&rows, 1, &emu); + assert!(before.contains(&hex(Profile::default().colors.red))); + assert!(before.contains(&hex(Profile::default().colors.background))); + + // The program picks its own background and recolors palette slot 1. + emu.process(b"\x1b]11;#3b0764\x07\x1b]4;1;#22c55e\x07"); + + let after = render_svg(&rows, 1, &emu); + assert!( + after.contains("#3b0764"), + "the window is painted with the background the program set" + ); + assert!( + after.contains("#22c55e"), + "a cell follows the slot the program recolored" + ); + assert!( + !after.contains(&hex(Profile::default().colors.red)), + "the configured red is no longer what slot 1 shows" + ); + } + + /// And a reset puts the configured colors back on screen. + #[test] + fn a_screenshot_returns_to_the_profile_after_a_reset() { + use crate::terminal::emu::Emulator; + let mut emu = colors(); + let rows = vec![vec![cell("x", Some(Color::from_index(1)), None)]]; + + emu.process(b"\x1b]11;#3b0764\x07\x1b]4;1;#22c55e\x07"); + emu.process(b"\x1b]111\x07\x1b]104;1\x07"); + + let after = render_svg(&rows, 1, &emu); + assert!(after.contains(&hex(Profile::default().colors.background))); + assert!(after.contains(&hex(Profile::default().colors.red))); + } + #[test] fn emits_valid_svg_with_text_and_color() { let rows = vec![vec![ diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index df2c708..713629c 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -665,6 +665,98 @@ macro_rules! emulator_conformance_tests { ); } + /// Each dynamic color is addressed by its own sequence, and each is + /// reset by its own. + /// + /// `OSC 11` is the one programs reach for, so it is easy to wire that + /// up and leave the foreground or the cursor answering the wrong slot. + #[test] + fn conformance_each_dynamic_color_is_separately_addressable() { + use $crate::profile::ColorSlot; + let mut e = conformance_emu(10, 4, 100); + let before = [ + e.color(ColorSlot::Foreground), + e.color(ColorSlot::Background), + e.color(ColorSlot::Cursor), + ]; + + // Set all three to distinct colors, then check none bled into + // another. + e.process(b"\x1b]10;#111111\x07\x1b]11;#222222\x07\x1b]12;#333333\x07"); + assert_eq!(e.color(ColorSlot::Foreground), Rgb::new(0x11, 0x11, 0x11)); + assert_eq!(e.color(ColorSlot::Background), Rgb::new(0x22, 0x22, 0x22)); + assert_eq!(e.color(ColorSlot::Cursor), Rgb::new(0x33, 0x33, 0x33)); + + // And each reset frees only its own slot. + e.process(b"\x1b]110\x07"); + assert_eq!(e.color(ColorSlot::Foreground), before[0], "110 resets fg"); + assert_eq!( + e.color(ColorSlot::Background), + Rgb::new(0x22, 0x22, 0x22), + "110 must leave the background alone" + ); + + e.process(b"\x1b]112\x07"); + assert_eq!( + e.color(ColorSlot::Cursor), + before[2], + "112 resets the cursor" + ); + assert_eq!( + e.color(ColorSlot::Background), + Rgb::new(0x22, 0x22, 0x22), + "112 must leave the background alone" + ); + + e.process(b"\x1b]111\x07"); + assert_eq!(e.color(ColorSlot::Background), before[1], "111 resets bg"); + } + + /// Every dynamic color answers a query, not just the background. + #[test] + fn conformance_every_dynamic_color_answers_a_query() { + let mut e = conformance_emu(10, 4, 100); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]10;#010203\x07\x1b]11;#040506\x07\x1b]12;#070809\x07"); + let _ = e.take_pending_writes(); + + for (query, expected) in [ + (&b"\x1b]10;?\x07"[..], "\x1b]10;rgb:0101/0202/0303\x07"), + (b"\x1b]11;?\x07", "\x1b]11;rgb:0404/0505/0606\x07"), + (b"\x1b]12;?\x07", "\x1b]12;rgb:0707/0808/0909\x07"), + ] { + e.process(query); + assert_eq!( + String::from_utf8_lossy(&e.take_pending_writes()), + expected, + "querying {:?}", + String::from_utf8_lossy(query) + ); + } + } + + /// `OSC 104` with no index resets the whole palette, and leaves the + /// three dynamic colors alone: they have their own resets. + #[test] + fn conformance_a_bare_palette_reset_spares_the_dynamic_colors() { + use $crate::profile::ColorSlot; + let mut e = conformance_emu(10, 4, 100); + let configured_red = e.color(ColorSlot::Indexed(1)); + + e.process(b"\x1b]4;1;#111111;200;#222222\x07\x1b]11;#333333\x07"); + assert_eq!(e.color(ColorSlot::Indexed(1)), Rgb::new(0x11, 0x11, 0x11)); + assert_eq!(e.color(ColorSlot::Indexed(200)), Rgb::new(0x22, 0x22, 0x22)); + + e.process(b"\x1b]104\x07"); + assert_eq!(e.color(ColorSlot::Indexed(1)), configured_red); + assert_eq!( + e.color(ColorSlot::Background), + Rgb::new(0x33, 0x33, 0x33), + "a palette reset is not a background reset" + ); + } + /// An unconfigured palette entry still answers, from the table the /// specification defines for it. #[test] From c26b8a205f53e6c5ee381293814cfc2171d2365b Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 16:54:16 -0400 Subject: [PATCH 7/8] feat(render): draw the cursor in screenshots A screenshot showed the grid but never where the terminal was about to write, so a reader could not tell an editor's caret position, whether a program had hidden the cursor, or which mode it was in. The emulator now reports visibility (`DECTCEM`) and shape (`DECSCUSR`), and the renderer draws the cursor after the text pass: a block fills the cell, an underline sits on its bottom edge, and a bar on its left. A block redraws the character beneath it in the cell's background color, so it stays readable rather than being swallowed. The redraw goes through the same path as the text pass, which keeps two cases right that a naive redraw gets wrong: a double-width character is covered across both of its cells instead of being clipped and squashed into one, and a nerd font glyph comes back as a vector glyph rather than as a character the text font has no glyph for. Blink is deliberately not represented; a screenshot is a single moment, and a blinking cursor is drawn in the half of the cycle where it shows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/engine.rs | 23 +- crates/shell-use/src/render/svg.rs | 251 +++++++++++++++++-- crates/shell-use/src/terminal/alacritty.rs | 22 +- crates/shell-use/src/terminal/conformance.rs | 49 ++++ crates/shell-use/src/terminal/emu.rs | 25 ++ 5 files changed, 352 insertions(+), 18 deletions(-) diff --git a/crates/shell-use/src/engine.rs b/crates/shell-use/src/engine.rs index 584b780..129cd20 100644 --- a/crates/shell-use/src/engine.rs +++ b/crates/shell-use/src/engine.rs @@ -868,12 +868,31 @@ fn do_snapshot( } } +/// Where to draw the cursor within `rows`, or `None` when the terminal is not +/// showing one. +/// +/// `Emulator::cursor` is relative to the visible screen, so a full screenshot +/// has to push it down past the scrollback that precedes it. +fn cursor_in( + rows: &[Vec], + emu: &dyn crate::terminal::emu::Emulator, +) -> Option<(u16, u16)> { + if !emu.cursor_visible() { + return None; + } + let (x, y) = emu.cursor(); + let (_, screen) = emu.size(); + let history = rows.len().saturating_sub(screen as usize) as u16; + Some((x, y.saturating_add(history))) +} + fn screenshot(s: &Session, full: bool, path: Option) -> Response { let rows = grid(s, full); match path { Some(path) => { - let svg = - crate::render::svg::render_svg(&rows, s.cols, s.state.lock().unwrap().emu.as_ref()); + let st = s.state.lock().unwrap(); + let emu = st.emu.as_ref(); + let svg = crate::render::svg::render_svg(&rows, s.cols, emu, cursor_in(&rows, emu)); match std::fs::write(&path, svg) { Ok(()) => Response::with(json!({ "path": path })), Err(e) => Response::internal(e.to_string()), diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index ff15d99..ffe5bb9 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -11,9 +11,9 @@ use std::fmt::Write; use super::nerd_font::NerdFont; -use crate::profile::Rgb; -use crate::terminal::cell::{Attrs, EmuCell}; -use crate::terminal::emu::Emulator; +use crate::profile::{ColorSlot, Rgb}; +use crate::terminal::cell::{Attrs, EmuCell, CONTINUATION}; +use crate::terminal::emu::{CursorShape, Emulator}; const CELL_W: f32 = 10.0; const CELL_H: f32 = 21.0; @@ -103,7 +103,80 @@ fn run_text(row: &[EmuCell], start: usize, end: usize) -> String { } /// Render a grid to a standalone SVG document. -pub fn render_svg(rows: &[Vec], cols: u16, colors: &dyn Emulator) -> String { +/// How much of a cell the thin cursor shapes cover. +const CURSOR_THICKNESS: f32 = 2.0; + +/// Draw the cursor over the cell it sits on. +/// +/// A block is filled and the character redrawn in the cell's background color, +/// which is how a terminal keeps the character under a block cursor readable. +/// It is drawn after the text pass so the block covers the first, normally +/// colored draw of that character. +fn write_cursor( + out: &mut String, + rows: &[Vec], + (cx, cy): (u16, u16), + colors: &dyn Emulator, + nerd_font: &NerdFont, +) { + let Some(row) = rows.get(cy as usize) else { + return; + }; + let cell = cell_at(row, cx as usize); + // A double-width character stores its second half as a continuation cell, + // so the cursor has to cover both or it clips the glyph down the middle. + let span = if row + .get(cx as usize + 1) + .is_some_and(|next| next.ch == CONTINUATION) + { + 2.0 + } else { + 1.0 + }; + let w = span * CELL_W; + let x = MARGIN_X + cx as f32 * CELL_W; + let y = HEADER_H + cy as f32 * CELL_H; + let fill = hex(colors.color(ColorSlot::Cursor)); + + let (rx, ry, rw, rh) = match colors.cursor_shape() { + CursorShape::Block => (x, y, w, CELL_H), + CursorShape::Underline => (x, y + CELL_H - CURSOR_THICKNESS, w, CURSOR_THICKNESS), + CursorShape::Bar => (x, y, CURSOR_THICKNESS, CELL_H), + }; + let _ = write!( + out, + r#""# + ); + + if colors.cursor_shape() != CursorShape::Block || cell.ch.trim().is_empty() { + return; + } + // Redraw exactly as the text pass would, so a vector glyph comes back as a + // glyph rather than as a character the text font may not even have. + let under = hex(bg_of(cell, colors)); + let (text, run_x_adjust) = nerd_font.prepare_run(&cell.ch, w, CELL_W); + if !text.trim().is_empty() { + let _ = write!( + out, + r#"{esc}"#, + baseline = y + FONT_BASELINE, + esc = escape(&text), + ); + } + for c in cell.ch.chars() { + nerd_font.write_use(out, c, (x, y), (CELL_W, CELL_H), run_x_adjust, &under); + } +} + +/// Render the grid. `cursor` is where to draw the cursor *within `rows`*, so a +/// caller passing scrollback has already offset it, and `None` means the +/// terminal is not showing one. +pub fn render_svg( + rows: &[Vec], + cols: u16, + colors: &dyn Emulator, + cursor: Option<(u16, u16)>, +) -> String { let nerd_font = NerdFont::new(rows, FONT_SIZE); let cols = cols as usize; let x0 = MARGIN_X; @@ -209,6 +282,10 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &dyn Emulator) -> St } } + if let Some(at) = cursor { + write_cursor(&mut out, rows, at, colors, &nerd_font); + } + out.push_str(""); out } @@ -235,6 +312,151 @@ mod tests { } } + /// Each shape draws something recognisably different. + /// + /// A block covers the cell, an underline sits on the bottom edge, and a + /// bar on the left, so all three are checked by the rectangle they emit + /// rather than by merely appearing. + #[test] + fn each_cursor_shape_draws_its_own_rectangle() { + use crate::terminal::emu::Emulator; + let rows = vec![vec![cell("x", None, None)]]; + let cursor_fill = hex(Profile::default().colors.cursor); + + let mut emu = colors(); + let block = render_svg(&rows, 1, &emu, Some((0, 0))); + assert!( + block.contains(&format!( + r#"width="10.00" height="21.00" fill="{cursor_fill}""# + )), + "a block covers the whole cell: {block}" + ); + + emu.process(b"\x1b[4 q"); + let underline = render_svg(&rows, 1, &emu, Some((0, 0))); + assert!( + underline.contains(&format!( + r#"width="10.00" height="2.00" fill="{cursor_fill}""# + )), + "an underline is a thin full-width bar: {underline}" + ); + + emu.process(b"\x1b[6 q"); + let bar = render_svg(&rows, 1, &emu, Some((0, 0))); + assert!( + bar.contains(&format!( + r#"width="2.00" height="21.00" fill="{cursor_fill}""# + )), + "a bar is a thin full-height stripe: {bar}" + ); + } + + /// The character under a block cursor is redrawn in the cell background, + /// which is how a terminal keeps it readable rather than hiding it behind + /// the block. + #[test] + fn a_block_cursor_keeps_its_character_readable() { + let rows = vec![vec![cell("Z", None, None)]]; + let svg = render_svg(&rows, 1, &colors(), Some((0, 0))); + let background = hex(Profile::default().colors.background); + assert!( + svg.contains(&format!(r#"fill="{background}""#)) && svg.matches(">Z<").count() == 2, + "the character is drawn again, in the background color: {svg}" + ); + } + + /// A double-width character keeps both of its halves. + /// + /// The second half lives in a continuation cell, so a cursor sized to one + /// cell would cover half the glyph and redraw it squashed into that half. + #[test] + fn a_block_cursor_covers_a_double_width_character() { + let rows = vec![vec![ + cell("日", None, None), + cell(CONTINUATION, None, None), + cell("a", None, None), + ]]; + let svg = render_svg(&rows, 3, &colors(), Some((0, 0))); + let cursor_fill = hex(Profile::default().colors.cursor); + assert!( + svg.contains(&format!( + r#"width="20.00" height="21.00" fill="{cursor_fill}""# + )), + "the block spans both halves: {svg}" + ); + assert!( + svg.contains( + r#"textLength="20.00" lengthAdjust="spacingAndGlyphs" xml:space="preserve">日<"# + ), + "the redraw is given both halves too, so it is not squashed: {svg}" + ); + } + + /// A vector glyph under a block cursor comes back as a glyph. + /// + /// Nerd font characters are drawn as `` references and masked out of + /// the text run, so redrawing one as text would emit a character the text + /// font has no glyph for and the block would simply swallow it. + #[test] + fn a_block_cursor_redraws_a_vector_glyph() { + let rows = vec![vec![cell("\u{f115}", None, None)]]; + let background = hex(Profile::default().colors.background); + let svg = render_svg(&rows, 1, &colors(), Some((0, 0))); + assert_eq!( + svg.matches("")); assert!(svg.contains("textLength")); @@ -305,7 +527,7 @@ mod tests { #[test] fn emits_window_chrome() { - let svg = render_svg(&[vec![cell(" ", None, None)]], 1, &colors()); + let svg = render_svg(&[vec![cell(" ", None, None)]], 1, &colors(), None); assert!(svg.contains("<")); } @@ -330,7 +552,7 @@ mod tests { #[test] fn background_run_emitted_for_non_default_bg() { let rows = vec![vec![cell(" ", None, Some(Color::from_index(4)))]]; - let svg = render_svg(&rows, 1, &colors()); + let svg = render_svg(&rows, 1, &colors(), None); assert!( svg.contains(&hex(colors().color(ColorSlot::Indexed(4)))), "slot 4 is painted with the profile color" @@ -345,7 +567,7 @@ mod tests { cell(glyph, None, None), cell("b", None, None), ]]; - let svg = render_svg(&rows, 3, &colors()); + let svg = render_svg(&rows, 3, &colors(), None); assert!(svg.contains(r#"")); diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 5fdb9c3..f858dbc 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -10,8 +10,9 @@ use alacritty_terminal::grid::Dimensions; use alacritty_terminal::index::{Column, Line}; use alacritty_terminal::term::cell::Flags as AlacFlags; use alacritty_terminal::term::test::TermSize; -use alacritty_terminal::term::{Config as AlacConfig, Term}; +use alacritty_terminal::term::{Config as AlacConfig, Term, TermMode}; use alacritty_terminal::vte::ansi; +use alacritty_terminal::vte::ansi::CursorShape as AlacCursorShape; use alacritty_terminal::vte::ansi::NamedColor; use alacritty_terminal::vte::ansi::Rgb as AlacRgb; @@ -19,7 +20,7 @@ use compact_str::{CompactString, ToCompactString}; use crate::profile::{xterm_color, ColorSlot, Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; -use crate::terminal::emu::Emulator; +use crate::terminal::emu::{CursorShape, Emulator}; /// Alacritty's palette colors arrive either as a `Named` variant or an index; /// both funnel through [`Color::from_index`] so a given slot always yields the @@ -288,6 +289,23 @@ impl Emulator for AlacrittyEmu { } } + fn cursor_visible(&self) -> bool { + // `Hidden` is a shape alacritty uses for a cursor it will not draw, so + // it means the same thing as the mode being off. + self.term.mode().contains(TermMode::SHOW_CURSOR) + && self.term.cursor_style().shape != AlacCursorShape::Hidden + } + + fn cursor_shape(&self) -> CursorShape { + match self.term.cursor_style().shape { + AlacCursorShape::Underline => CursorShape::Underline, + AlacCursorShape::Beam => CursorShape::Bar, + // `HollowBlock` is what alacritty draws for an unfocused window, + // which a headless terminal has no notion of. + _ => CursorShape::Block, + } + } + fn cursor(&self) -> (u16, u16) { let p = self.term.grid().cursor.point; let y = p.line.0.max(0).min(self.rows as i32 - 1) as u16; diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index 713629c..7b2820d 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -561,6 +561,55 @@ macro_rules! emulator_conformance_tests { ); } + /// A program can hide the cursor and show it again. + /// + /// Full-screen programs hide it while they repaint, so a backend that + /// ignored `DECTCEM` would leave a cursor in every screenshot taken + /// mid-draw, parked wherever the last write happened to end. + #[test] + fn conformance_the_cursor_can_be_hidden_and_shown() { + let mut e = conformance_emu(10, 3, 100); + assert!( + e.cursor_visible(), + "a terminal starts by showing its cursor" + ); + + e.process(b"\x1b[?25l"); + assert!(!e.cursor_visible(), "DECTCEM off hides it"); + + e.process(b"\x1b[?25h"); + assert!(e.cursor_visible(), "DECTCEM on shows it again"); + } + + /// The cursor takes the shape a program asks for with `DECSCUSR`. + /// + /// Each shape has a blinking and a steady form, and both report the + /// same shape: a screenshot is one moment, so the blink is not part of + /// what a backend has to agree on. + #[test] + fn conformance_the_cursor_takes_the_shape_it_is_given() { + use $crate::terminal::emu::CursorShape; + let mut e = conformance_emu(10, 3, 100); + assert_eq!(e.cursor_shape(), CursorShape::Block, "block is the default"); + + for (sequence, expected) in [ + (&b"\x1b[3 q"[..], CursorShape::Underline), + (b"\x1b[4 q", CursorShape::Underline), + (b"\x1b[5 q", CursorShape::Bar), + (b"\x1b[6 q", CursorShape::Bar), + (b"\x1b[1 q", CursorShape::Block), + (b"\x1b[2 q", CursorShape::Block), + ] { + e.process(sequence); + assert_eq!( + e.cursor_shape(), + expected, + "{:?} selects {expected:?}", + String::from_utf8_lossy(sequence) + ); + } + } + /// A color query is answered with the session's configured color. /// /// Programs query the background to decide whether they are on a light diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index edbcb41..7b01730 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -13,6 +13,19 @@ use crate::profile::{ColorSlot, Rgb}; use crate::terminal::cell::{Color, EmuCell}; +/// The shape a terminal draws its cursor as, set with `DECSCUSR` (`CSI Ps SP q`). +/// +/// The specification defines three, each in a blinking and a steady form. The +/// blink is not represented: a screenshot is a single moment, and a blinking +/// cursor is drawn in the half of that cycle where it is visible. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum CursorShape { + #[default] + Block, + Underline, + Bar, +} + /// A headless terminal emulator: bytes in, cell grid out. /// /// Implementations must be `Send`; the daemon shares the emulator across its @@ -34,8 +47,20 @@ pub trait Emulator: Send { fn size(&self) -> (u16, u16); /// Cursor position as `(x, y)` (column, row), 0-based, clamped to screen. + /// + /// Always relative to the visible screen, never to the scrollback, so a + /// caller drawing over `full_rows` has to offset it by the history above. fn cursor(&self) -> (u16, u16); + /// Whether the cursor is being drawn, which programs toggle with + /// `DECTCEM` (`CSI ?25 h` and `l`). Full-screen programs routinely hide it + /// while repainting, so a screenshot that ignored this would show a cursor + /// parked wherever the last write happened to leave it. + fn cursor_visible(&self) -> bool; + + /// The shape the cursor is currently drawn as. + fn cursor_shape(&self) -> CursorShape; + /// Visible screen as rows of cells. Always `rows` entries of `cols` cells. fn viewable_rows(&self) -> Vec>; From f293b49c2c560c3cd8397cfbe4a5327ac79ec253 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 17:13:05 -0400 Subject: [PATCH 8/8] test(color): run the color query probe on unix only The probe puts its own terminal in raw mode so it can read a reply that arrives without a newline and must not be echoed. That needs `termios`, which Windows CPython does not ship, so the test could only ever fail there, and a fail-fast matrix let it cancel the other two platforms. Nothing about the reply is platform specific. Its format is covered by conformance cases that run against every backend, and the write that carries it to the child is the same `pty.write` that every `type` and `submit` already exercises on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use-cli/tests/session_lifecycle.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index aabc5f3..27a630f 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -349,6 +349,14 @@ fn an_unknown_profile_is_rejected() { /// A terminal that stays silent leaves them blocked until they time out and /// guess, so this drives the whole path: daemon, emulator, and the reply on /// its way back up the PTY. +/// +/// Unix only, because the probe has to put its own terminal in raw mode to +/// read a reply that arrives without a newline and must not be echoed, and +/// `termios` does not exist on Windows CPython. The reply itself is not +/// platform specific: how it is formatted is covered by conformance cases +/// that run against every backend, and the write that carries it to the child +/// is the same `pty.write` every `type` and `submit` on Windows already uses. +#[cfg(unix)] #[test] fn a_color_query_is_answered_over_the_pty() { let sandbox = Sandbox::new("osc-query");