From 4080564cad65100d352f9d1391a68fcfb14b05af Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:29:20 +0700 Subject: [PATCH 1/6] fix: read a late terminal reply as a reply, not as keystrokes A slow terminal's answers to the start-up probe arrived after its 250 ms deadline and crossterm read the background colour as Alt+] (next year), r (reload) and two d's (cell style). The probe now waits 500 ms and gives a terminal that has started answering a second window to finish; anything later is swallowed in the key handler up to its terminator, and Alt keys no longer act as their plain letter. Fixes #140 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 12 +++++++ docs/DESIGN.md | 10 ++++++ src/app.rs | 49 ++++++++++++++++++++++++++- src/main.rs | 7 ++-- src/render_tests.rs | 82 +++++++++++++++++++++++++++++++++++++++++++++ src/term.rs | 28 ++++++++++++++-- tests/smoke.rs | 34 +++++++++++++++++++ 7 files changed, 216 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a42b087..4172c77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,18 @@ listed under a **Changed** or **Removed** heading. ## [Unreleased] +### Fixed + +- **A terminal reply that arrived after the start-up probe was read as + keystrokes** (#140). On a slow link the answers came in after the 250 ms + deadline, and crossterm read the background colour as Alt+`]` (next year) + followed by `r` (reload) and two `d`s (cell style): a dark kitty over ssh + started in snug cells, with no pixels and a guessed theme. The probe now + waits 500 ms, and a terminal that has started answering by then gets a + second window to finish; anything later still is recognised as a reply and + swallowed up to its terminator, and keys carrying Alt no longer act as + their plain letter. + ### Changed - **termlens 0.11** for the PTY suite, and the vendored skill with it. 0.11 diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 38967e2..66a154a 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -167,6 +167,16 @@ doubles as the "everything that is coming has come" marker. The read is from timeout and nothing else — no thread left blocked holding the keyboard, which is what a naive blocking read would do to the next keypress. +A reply that misses the deadline is not lost, which is the trouble with it: it +arrives on the keyboard's input, and crossterm, which has no parser for `OSC` or +`APC`, reads `ESC ] 11 ; rgb:0d0d/…` as Alt+`]` and then an `r` and two `d`s — +next year, reload, and a different cell style. So a terminal that has started +answering by the deadline gets a second window to reach the sentinel; a silent +one still costs only the first. Whatever arrives later still is swallowed by +the key handler: an Alt+`]`, Alt+`_` or Alt+`P` opens a reply, the keys after +it are dropped up to its terminator (bounded in count and in time), and no +binding in the chart answers to Alt at all. + Only `OK` counts for kitty. A terminal that knows the protocol but not that transmission medium answers `ENOTSUPPORTED`, and would then be sent images it cannot draw. diff --git a/src/app.rs b/src/app.rs index 63126e8..943d0b0 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3,6 +3,7 @@ use std::io::{self, Write}; use std::sync::mpsc::{self, Receiver, Sender}; use std::thread; +use std::time::Instant; use chrono::{Duration, Local, NaiveDate}; use ratatui::crossterm::event::{KeyCode, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; @@ -208,6 +209,9 @@ pub struct App { /// `--cell`, which outranks anything the terminal reports and survives a /// re-measure. cell_override: Option<(u16, u16)>, + /// When a late terminal reply began arriving as keys, and how many of its + /// keys have been swallowed since. See [`App::on_key`]. + reply: Option<(Instant, usize)>, seq: u64, tx: Sender, rx: Receiver, @@ -244,6 +248,7 @@ impl App { today: None, wanted: Graphics::Auto, cell_override: None, + reply: None, seq: 0, tx, rx, @@ -388,18 +393,60 @@ impl App { } /// Handle a key press. + /// + /// Not every key press is one. A terminal reply that arrives after the + /// probe stopped listening comes in on the keyboard's input, and crossterm + /// has no parser for `OSC` or `APC`: `ESC ] 11 ; rgb:0d0d/1111/1717 ESC \` + /// reaches here as Alt+`]` — "next year" — and then an `r` and two `d`s + /// (#140). So an Alt+`]`, Alt+`_` or Alt+`P` opens a reply, and what follows + /// it is swallowed up to its terminator, bounded in keys and in time so a + /// person who really typed Alt+`]` gets the keyboard back at once. pub fn on_key(&mut self, code: KeyCode, mods: KeyModifiers) { + if self.late_reply(code, mods) { + return; + } if mods.contains(KeyModifiers::CONTROL) && matches!(code, KeyCode::Char('c' | 'd')) { self.quit = true; return; } if matches!(self.mode, Mode::Input(_)) { self.on_key_input(code); - } else { + } else if !mods.contains(KeyModifiers::ALT) { + // No binding here uses Alt, and `on_key_normal` reads only the key + // code — so Alt+`]` was `]`, and any escape sequence crossterm does + // not know arrived as a binding. self.on_key_normal(code); } } + /// Whether this key belongs to a terminal reply rather than to a person. + fn late_reply(&mut self, code: KeyCode, mods: KeyModifiers) -> bool { + /// A reply is one write, so its keys arrive together; this is slack for + /// one split across packets on a slow link, and short enough that a + /// person never notices the keyboard was not listening. + const WINDOW: std::time::Duration = std::time::Duration::from_millis(250); + /// The longest reply asked for is a kitty error message, well under this. + const KEYS: usize = 128; + + let alt = mods.contains(KeyModifiers::ALT); + if let Some((since, keys)) = self.reply { + if since.elapsed() < WINDOW && keys < KEYS { + // `ESC \` (ST) or BEL, which crossterm reports as Ctrl+G. + let end = (alt && code == KeyCode::Char('\\')) + || (mods.contains(KeyModifiers::CONTROL) && code == KeyCode::Char('g')); + self.reply = (!end).then_some((since, keys + 1)); + return true; + } + self.reply = None; + } + // `ESC ]` opens an OSC, `ESC _` an APC, `ESC P` a DCS. + if alt && matches!(code, KeyCode::Char(']' | '_' | 'P')) { + self.reply = Some((Instant::now(), 0)); + return true; + } + false + } + fn on_key_normal(&mut self, code: KeyCode) { // The help is modal and forgiving: whatever you press to get out of it, // gets you out of it. Closing needs a redraw because the overlay is text diff --git a/src/main.rs b/src/main.rs index 858a1c4..3fb432b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -74,8 +74,11 @@ const VERSION: &str = concat!("mossaic ", env!("CARGO_PKG_VERSION")); /// Redraw cadence. Also bounds how long a finished fetch waits to appear. const TICK: Duration = Duration::from_millis(80); /// How long to wait for a terminal to say what it can do. A terminal that answers -/// takes a millisecond or two; this is the budget for one that never will. -const PROBE: Duration = Duration::from_millis(250); +/// takes a millisecond or two, and its device-attributes reply ends the wait; +/// this is the budget for one that never will. 250 ms was shorter than an ssh +/// round trip to the other side of the world, and an answer that misses the +/// probe is read as keystrokes (#140). +const PROBE: Duration = Duration::from_millis(500); /// Everything the command line settles before a terminal is involved. struct Invocation { diff --git a/src/render_tests.rs b/src/render_tests.rs index 93392bb..89fc0ba 100644 --- a/src/render_tests.rs +++ b/src/render_tests.rs @@ -3815,6 +3815,88 @@ fn the_probe_reads_a_reply_that_shares_its_buffer() { assert_eq!(term::parse("\x1b]11;#12345\x07").background, None); } +#[test] +fn a_slow_terminal_gets_time_to_finish_and_a_mute_one_does_not() { + use crate::term::keep_waiting; + use std::time::Duration as Wait; + let timeout = Wait::from_millis(500); + let at = Wait::from_millis; + // Inside the first window, whatever has been heard. + assert!(keep_waiting(at(10), timeout, false)); + assert!(keep_waiting(at(499), timeout, true)); + // Past it, silence is a terminal that will never answer: the deadline and + // nothing more. + assert!(!keep_waiting(at(500), timeout, false)); + // But a terminal that has started answering is slow, not mute, and what it + // has not said yet would otherwise arrive as keystrokes (#140). + assert!(keep_waiting(at(500), timeout, true)); + assert!(keep_waiting(at(999), timeout, true)); + // Bounded all the same. + assert!(!keep_waiting(at(1000), timeout, true)); +} + +#[test] +fn a_terminal_reply_that_arrives_late_is_not_read_as_keys() { + use crate::app::{CellStyle, Source}; + use ratatui::crossterm::event::{KeyCode, KeyModifiers}; + + /// What crossterm makes of bytes it has no parser for: `ESC` and a + /// character is Alt and that character, BEL is Ctrl+G, and a capital + /// carries Shift. The `CSI` replies never get here — crossterm drops a + /// sequence it cannot parse — so these are the ones that matter. + fn type_out(app: &mut App, bytes: &str) { + let mut chars = bytes.chars(); + while let Some(c) = chars.next() { + let (c, mut mods) = match c { + '\x1b' => (chars.next().expect("ESC is followed"), KeyModifiers::ALT), + '\x07' => ('g', KeyModifiers::CONTROL), + c => (c, KeyModifiers::NONE), + }; + if c.is_uppercase() { + mods |= KeyModifiers::SHIFT; + } + app.on_key(KeyCode::Char(c), mods); + } + } + // The demo, so that a reload would be visible and would not touch the network. + let fresh = || { + let mut app = ready(synthetic(2025, "octocat")); + app.source = Source::Demo; + app + }; + + // A dark kitty's ordinary answers, in both terminators. Before #140 the + // `r` reloaded and the two `d`s moved the chart to snug cells. + let mut app = fresh(); + let cursor = app.cursor; + type_out( + &mut app, + "\x1b_Gi=7379;OK\x1b\\\x1b]11;rgb:0d0d/1111/1717\x1b\\\x1b]11;rgb:ffff/ffff/ffff\x07", + ); + assert_eq!( + app.cells, + CellStyle::Auto, + "the reply changed the cell style" + ); + assert!(matches!(app.load, Load::Ready(_)), "the reply reloaded"); + assert_eq!(app.cursor, cursor, "the reply moved the cursor"); + assert!(!app.quit && !app.help); + + // And the keyboard is back the moment the reply ends. + app.on_key(KeyCode::Char('d'), KeyModifiers::NONE); + assert_eq!(app.cells, CellStyle::Rounded, "d after the reply"); + + // Alt is never a binding: Alt+`d` is not `d`. + let mut app = fresh(); + app.on_key(KeyCode::Char('d'), KeyModifiers::ALT); + app.on_key(KeyCode::Char('h'), KeyModifiers::ALT); + assert_eq!(app.cells, CellStyle::Auto); + assert_eq!(app.cursor, cursor); + // Ctrl+C still quits, Alt or not. + app.on_key(KeyCode::Char('c'), KeyModifiers::CONTROL); + assert!(app.quit); +} + #[test] fn a_cell_is_the_square_github_draws_and_lands_on_whole_pixels() { use crate::graphics; diff --git a/src/term.rs b/src/term.rs index 9f3915d..4aa9ffb 100644 --- a/src/term.rs +++ b/src/term.rs @@ -18,6 +18,13 @@ //! The device-attributes reply comes last and every terminal answers it, so it //! doubles as the "everything that is coming has come" marker: the usual round trip //! is a millisecond or two, not the timeout. +//! +//! A reply the probe stops listening for does not go away: it arrives on the +//! same input as the keyboard, and the event loop reads `ESC ] 11 ; rgb:0d0d/…` +//! as Alt+`]` followed by an `r` and two `d`s (#140). So a terminal that has +//! begun answering is given a second window to finish, and +//! [`crate::app::App::on_key`] swallows the body of anything that still arrives +//! late. use std::time::Duration; @@ -61,7 +68,8 @@ pub struct Caps { /// /// Call this with the terminal already in raw mode, or the replies arrive /// line-buffered and echoed. A terminal that never answers costs the timeout -/// and nothing else. +/// and nothing else; one that has started answering by then gets up to the +/// same again to reach the sentinel. #[cfg(unix)] pub fn probe(timeout: Duration) -> Caps { use std::fs::OpenOptions; @@ -84,7 +92,7 @@ pub fn probe(timeout: Duration) -> Caps { return Caps::default(); } - let deadline = Instant::now() + timeout; + let start = Instant::now(); let mut reply = Vec::new(); let mut chunk = [0u8; 1024]; loop { @@ -97,7 +105,7 @@ pub fn probe(timeout: Duration) -> Caps { } } Err(error) if error.kind() == ErrorKind::WouldBlock => { - if Instant::now() >= deadline { + if !keep_waiting(start.elapsed(), timeout, !reply.is_empty()) { break; } std::thread::sleep(Duration::from_millis(2)); @@ -108,6 +116,20 @@ pub fn probe(timeout: Duration) -> Caps { parse(&String::from_utf8_lossy(&reply)) } +/// Whether the probe should go on reading, `elapsed` into it, having `heard` +/// something or nothing. +/// +/// Nothing by the deadline is a mute terminal, and it costs the deadline and no +/// more. Something but not yet the sentinel is a slow one — a high-latency ssh +/// link — and stopping there hands the rest of its answers to the event loop, +/// which read them as keystrokes: a dark kitty's ordinary replies arriving late +/// pressed `d` twice and left the chart in snug cells (#140). So a terminal +/// that has begun answering gets a second window of the same length to finish. +#[cfg_attr(not(unix), allow(dead_code))] +pub(crate) fn keep_waiting(elapsed: Duration, timeout: Duration, heard: bool) -> bool { + elapsed < timeout || (heard && elapsed < timeout.saturating_mul(2)) +} + /// Ask the terminal what it can do — except here, where nothing is asked. /// /// The probe reads replies from `/dev/tty` with `O_NONBLOCK`, which is a unix diff --git a/tests/smoke.rs b/tests/smoke.rs index fd3c3b9..ee7119f 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -872,3 +872,37 @@ fn the_advice_line_names_the_window_size_not_the_drawable_area() -> termlens::Re let _ = t.wait_exit()?; Ok(()) } + +/// A terminal whose answers miss the probe: the ssh link to the other side of +/// the world. termlens is told not to answer, and then the answers a dark kitty +/// gives are delivered after the first frame, which is where a late reply lands. +/// crossterm reads `ESC ]` as Alt+`]` and the colour that follows as keys, so +/// the `r` and the two `d`s in `rgb:0d0d/…` reloaded and moved the chart from +/// rounded to snug cells (#140). +#[test] +fn a_reply_that_misses_the_probe_is_not_read_as_keys() -> termlens::Result<()> { + let mut t = spawn(&PREVIEW, SIZE, |b| b.answer_queries(false))?; + let before = t.wait_frame(loaded)?; + assert_eq!(style(&before), "rounded cells", "{before}"); + + t.send_str(concat!( + "\x1b_Gi=7379;OK\x1b\\", + "\x1b]11;rgb:0d0d/1111/1717\x1b\\", + "\x1b[6;19;9t", + "\x1b[4;646;1584t", + "\x1b[?62;4;22c", + ))?; + // A key with something to show for it, after the reply: once the frame + // carries it, every key before it has been handled. + t.send(Key::Home)?; + let after = t.wait_frame(|s| loaded(s) && s.contains("Fri, Jan 1 2027"))?; + assert_eq!( + style(&after), + "rounded cells", + "the reply was read as keys:\n{after}" + ); + assert!(is_auto(&after), "the style is still Auto's:\n{after}"); + t.send(Key::Char('q'))?; + assert!(t.wait_exit()?.success()); + Ok(()) +} From b59553df294693bee6af18a21c61ed09fbe5cd22 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:32:28 +0700 Subject: [PATCH 2/6] fix: use the terminal's own text colour when its background is unknown With no OSC 11 answer the appearance fell back to dark, and on a light terminal the chrome was #f0f6fc at about 1.07:1. Text now uses the terminal's default colour and only the cells keep GitHub's; the help overlay offers --theme light; sixel edges are cut instead of blended into a guessed dark canvas. README: native Windows never probes, so it needs --graphics sixel --cell WxH. Fixes #141 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 11 ++++ README.md | 20 ++++-- src/app.rs | 37 ++++++++--- src/graphics.rs | 47 ++++++++++--- src/render_tests.rs | 157 ++++++++++++++++++++++++++++++++++++++++++++ src/ui.rs | 118 +++++++++++++++++---------------- 6 files changed, 311 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4172c77..8a2a5ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,17 @@ listed under a **Changed** or **Removed** heading. second window to finish; anything later still is recognised as a reply and swallowed up to its terminator, and keys carrying Alt no longer act as their plain letter. +- **A terminal that did not report its background got white text on white** + (#141). The appearance fell back to dark, so on a light terminal that did + not answer `OSC 11` — every native Windows run, mosh, multiplexers that do + not forward it — the header, the month labels and the footer were `#f0f6fc` + at about 1.07:1. Text is now drawn in the terminal's own colour (dimmed for + the quieter lines) and only the cells keep GitHub's; the help overlay says + the background was not reported and offers `--theme light`. Sixel edges are + no longer blended into a dark canvas that was only a guess: they are cut + hard instead. The README no longer lists Windows Terminal among the + terminals found automatically — on Windows nothing is asked, so it needs + `--graphics sixel --cell WxH`. ### Changed diff --git a/README.md b/README.md index 9a3337b..35d4bc7 100644 --- a/README.md +++ b/README.md @@ -117,10 +117,14 @@ cells kitty ``` - **`cells text`** means the terminal draws neither protocol. kitty, Ghostty, - WezTerm, foot, Konsole, iTerm2, mlterm, Windows Terminal and - `xterm -ti vt340` all draw one of them; GNOME Terminal and Ptyxis draw - neither, because VTE ships sixel switched off. Nothing is lost but the - pixels — the chart still draws in block sextants. + WezTerm, foot, Konsole, iTerm2, mlterm and `xterm -ti vt340` all draw one + of them; GNOME Terminal and Ptyxis draw neither, because VTE ships sixel + switched off. Nothing is lost but the pixels — the chart still draws in + block sextants. +- **On native Windows nothing is asked at all** — the probe reads the + terminal's replies in a way only unix offers — so the answers are all `no`. + Windows Terminal draws sixel when told to: `mossaic --graphics sixel --cell + WxH`, measuring the cell yourself, and `--theme light` on a light background. - **A window under 112 columns** falls back too, and the chart says so under the legend, naming the number it wants. - Either way, `mossaic --png chart.png` renders the same image to a file from @@ -435,7 +439,8 @@ act on: - **Which colours.** Every value is a Primer token read out of the stylesheets github.com serves, not transcribed. Light or dark follows the terminal's own background over `OSC 11`, the way a browser follows the OS; `--theme` - overrides it. GitHub swaps the greens for a few days a year and so does + overrides it. A terminal that does not say gets its own colour for the text + and GitHub's only for the cells, which read on either background. GitHub swaps the greens for a few days a year and so does mossaic, on the same dates — `--palette` asks for them out of season. - **How a day is sized.** A day is two character columns wide and one row tall, and the square inside it keeps github.com's ratios — an 11px cell on a 14px @@ -459,8 +464,9 @@ palette is degraded on purpose, and what the capability probe actually sends. - **Motion reporting is not universal.** Terminals that report clicks but not motion (Terminal.app among them) get click-to-select-a-day and no hover. - **Sixel has no alpha.** Anti-aliased edges are composited against the background - colour the terminal reported; where it reports none, GitHub's own canvas colour - stands in, and on a terminal with a background image the corners will show a faint + colour the terminal reported; where it reports none, the canvas of the `--theme` + asked for stands in, and with neither the edges are cut rather than blended into + a guess. On a terminal with a background image the corners will show a faint halo. Kitty, which takes an alpha channel, does not have the problem. - **The tooltip sits above the grid, not over the day** — the one placement both protocols can live with. diff --git a/src/app.rs b/src/app.rs index 943d0b0..5953077 100644 --- a/src/app.rs +++ b/src/app.rs @@ -209,6 +209,9 @@ pub struct App { /// `--cell`, which outranks anything the terminal reports and survives a /// re-measure. cell_override: Option<(u16, u16)>, + /// Whether the theme is known — asked for with `--theme`, or read off the + /// background the terminal reported — rather than assumed. + appearance_known: bool, /// When a late terminal reply began arriving as keys, and how many of its /// keys have been swallowed since. See [`App::on_key`]. reply: Option<(Instant, usize)>, @@ -248,6 +251,7 @@ impl App { today: None, wanted: Graphics::Auto, cell_override: None, + appearance_known: false, reply: None, seq: 0, tx, @@ -264,10 +268,13 @@ impl App { self.today = options.today; self.wanted = options.graphics; self.cell_override = options.cell; - let appearance = options + let known = options .appearance - .or_else(|| caps.background.map(Appearance::from_background)) - .unwrap_or(Appearance::Dark); + .or_else(|| caps.background.map(Appearance::from_background)); + self.appearance_known = known.is_some(); + // Dark for the cells, which carry their own colours and read on either + // background. The chrome does not: see `appearance_known`. + let appearance = known.unwrap_or(Appearance::Dark); let season = options .season .unwrap_or_else(|| Season::on(Local::now().date_naive())); @@ -289,6 +296,16 @@ impl App { self.today } + /// Whether the theme was asked for or read off the terminal, rather than + /// assumed. When it was assumed, the renderer draws its text in the + /// terminal's own foreground: the dark theme's `#f0f6fc` on a white + /// terminal is 1.07:1, and the header and month labels vanished — on every + /// native Windows run, under mosh, and behind multiplexers that do not + /// forward `OSC 11` (#141). + pub fn appearance_known(&self) -> bool { + self.appearance_known + } + /// Whether this terminal can draw pixel cells. pub fn pixels_available(&self) -> bool { self.gfx.is_some() @@ -679,11 +696,15 @@ fn painter(caps: &Caps, options: &Options, palette: &Palette) -> Option .cell .or_else(|| term::cell_size(caps)) .filter(|(w, h)| *w >= 2 && *h >= 2)?; - Some(Painter::new( - protocol, - cell, - caps.background.unwrap_or(palette.canvas), - )) + // `--theme` names a palette, not the terminal's background, so it is only a + // stand-in for one: the canvas of the theme asked for is the best guess + // there is. With neither, a guess is what put dark halos round every sixel + // cell on a light terminal (#141), so the edges are cut instead. + Some(match (caps.background, options.appearance) { + (Some(background), _) => Painter::new(protocol, cell, background), + (None, Some(_)) => Painter::new(protocol, cell, palette.canvas), + (None, None) => Painter::over_unknown(protocol, cell), + }) } /// What the painter should be showing, given what the renderer laid out. diff --git a/src/graphics.rs b/src/graphics.rs index 90c0f88..87a16ae 100644 --- a/src/graphics.rs +++ b/src/graphics.rs @@ -460,7 +460,13 @@ fn base64(data: &[u8]) -> String { /// year of cells produces far more shades than that, so the blends are snapped to a /// coarser grid until they fit — starting at full precision, which is usually /// enough, and only losing accuracy on a palette that would not have fit anyway. -fn indexed(image: &Image, background: Rgb) -> (Vec<[u8; 3]>, Vec>) { +/// +/// With no `background` to composite against, an edge is not blended at all: a +/// pixel at least half covered is drawn in its own colour and the rest is left +/// alone. Guessing one instead put a dark halo round every cell on a light +/// terminal that had not said what it was (#141); a hard edge looks right on +/// either. +fn indexed(image: &Image, background: Option) -> (Vec<[u8; 3]>, Vec>) { /// Registers `#0`-`#255`, so all 256 are addressable. Checked *before* an /// index is handed out: `seen.len() as u8` silently wraps to 0 on the /// 257th colour, and an index that aliases another colour must never be @@ -474,12 +480,17 @@ fn indexed(image: &Image, background: Rgb) -> (Vec<[u8; 3]>, Vec>) { let mut overflowed = false; for pixel in &image.pixels { - if pixel[3] == 0 { + let alpha = f32::from(pixel[3]) / 255.0; + let colour = Rgb(pixel[0], pixel[1], pixel[2]); + let solid = match background { + _ if pixel[3] == 0 => None, + Some(background) => Some(colour.over(background, alpha)), + None => (alpha >= 0.5).then_some(colour), + }; + let Some(solid) = solid else { map.push(None); continue; - } - let alpha = f32::from(pixel[3]) / 255.0; - let solid = Rgb(pixel[0], pixel[1], pixel[2]).over(background, alpha); + }; let snap = |value: u8| { let percent = (u16::from(value) * 100 + 127) / 255; ((percent + step / 2) / step * step).min(100) as u8 @@ -510,6 +521,16 @@ fn indexed(image: &Image, background: Rgb) -> (Vec<[u8; 3]>, Vec>) { /// `P2 = 1`: a pixel nobody paints is left as it was, which is how a rounded corner /// stays rounded over a terminal background sixel cannot see. pub fn sixel(image: &Image, background: Rgb) -> String { + encode_sixel(image, Some(background)) +} + +/// A sixel for a terminal whose background is not known: edges are cut at half +/// coverage rather than blended into a colour that may be wrong. +pub fn sixel_hard_edged(image: &Image) -> String { + encode_sixel(image, None) +} + +fn encode_sixel(image: &Image, background: Option) -> String { let (palette, map) = indexed(image, background); let mut out = String::from("\x1bP0;1;0q"); out.push_str(&format!("\"1;1;{};{}", image.width, image.height)); @@ -627,7 +648,8 @@ pub struct Painter { /// Columns the terminal has, so clearing cells for a sixel cannot write past /// the last one and wrap over the rows below. pub width: u16, - background: Rgb, + /// What sixel edges are blended into, when the terminal said. + background: Option, base: Option, marks: [Option; 2], } @@ -636,11 +658,20 @@ impl Painter { /// A painter for a terminal that speaks `protocol`, with cells of `cell` /// pixels over `background`. pub fn new(protocol: Protocol, cell: (u16, u16), background: Rgb) -> Self { + Self { + background: Some(background), + ..Self::over_unknown(protocol, cell) + } + } + + /// A painter for a terminal that did not say what its background is. Kitty + /// does not care; sixel edges are cut hard rather than blended into a guess. + pub fn over_unknown(protocol: Protocol, cell: (u16, u16)) -> Self { Self { protocol, cell, width: u16::MAX, - background, + background: None, base: None, marks: [None; 2], } @@ -809,7 +840,7 @@ impl Painter { let rows = image.height.div_ceil(usize::from(self.cell.1)) as u16; kitty(image, id, columns, rows, z) } - Protocol::Sixel => sixel(image, self.background), + Protocol::Sixel => encode_sixel(image, self.background), }; // Absolute placement every time: the cursor is wherever the last frame's // text left it, and both protocols draw from wherever it is. diff --git a/src/render_tests.rs b/src/render_tests.rs index 89fc0ba..775b004 100644 --- a/src/render_tests.rs +++ b/src/render_tests.rs @@ -1041,6 +1041,163 @@ fn a_sixel_is_the_image_the_rasteriser_drew() { ); } +#[test] +fn a_sixel_over_an_unknown_background_is_cut_not_blended() { + use crate::graphics; + use crate::primer::{Appearance, Palette, Season}; + + // Blending the edges into GitHub's dark canvas, when the terminal had not + // said it was dark, put a dark halo round every cell on a light one (#141). + // With nothing to blend into, a pixel is its own colour or it is left alone. + let palette = Palette::new(Appearance::Dark, Season::Default, true); + let image = graphics::grid(&levels_of(&synthetic(2025, "octocat")), &palette, (9, 19)); + let (width, height, pixels) = decode_sixel(&graphics::sixel_hard_edged(&image)); + assert_eq!((width, height), (image.width, image.height)); + let mut edges = 0; + for y in 0..height { + for x in 0..width { + let [r, g, b, a] = image.rgba_at(x, y); + match pixels[y * width + x] { + None => assert!(a < 128, "a mostly covered pixel at {x},{y} went unpainted"), + Some(got) => { + assert!(a >= 128, "a mostly empty pixel at {x},{y} was painted"); + edges += usize::from(a < 255); + for (got, want) in got.iter().zip([r, g, b]) { + assert!(got.abs_diff(want) <= 2, "at {x},{y}: {got} vs {want}"); + } + } + } + } + } + assert!( + edges > 0, + "no edge pixels were drawn, so the cut was not tested" + ); + + // And the painter takes that path only when the terminal did not say. + use crate::app::Options; + use crate::primer::Rgb; + use crate::term::Caps; + let caps = Caps { + sixel: true, + cell: Some((9, 19)), + answered: true, + ..Caps::default() + }; + // What the painter put on the wire, and the year as each encoding writes it. + let sent = |caps: Caps| { + let mut app = ready(synthetic(2025, "octocat")); + app.configure(caps, Options::default()); + let _ = render(&mut app, 176, 34); + let mut out = Vec::new(); + app.paint(&mut out).unwrap(); + let year = graphics::grid( + &levels_of(&synthetic(2025, "octocat")), + &app.palette, + (9, 19), + ); + (String::from_utf8(out).unwrap(), year, app.palette.canvas) + }; + let (wire, year, _) = sent(caps); + assert!( + wire.contains(&graphics::sixel_hard_edged(&year)), + "an unknown background should get hard edges" + ); + let reported = Rgb::hex(0xffffff); + let (wire, year, _) = sent(Caps { + background: Some(reported), + ..caps + }); + assert!( + wire.contains(&graphics::sixel(&year, reported)), + "a reported background is still blended into" + ); +} + +#[test] +fn an_unknown_background_leaves_the_text_to_the_terminal() { + use crate::app::Options; + use crate::primer::{Appearance, Rgb}; + use crate::term::Caps; + use ratatui::buffer::Buffer; + use ratatui::style::{Color, Modifier, Style}; + + let draw = |background: Option, theme: Option, help: bool| -> Buffer { + let mut app = ready(synthetic(2025, "octocat")); + app.configure( + Caps { + background, + ..Caps::default() + }, + Options { + appearance: theme, + ..Options::default() + }, + ); + app.help = help; + let mut terminal = Terminal::new(TestBackend::new(176, 34)).unwrap(); + terminal.draw(|frame| ui::draw(frame, &mut app)).unwrap(); + terminal.backend().buffer().clone() + }; + // The style of the first cell of the first occurrence of `needle`. + let style_of = |buffer: &Buffer, needle: &str| -> Style { + let area = buffer.area; + for y in 0..area.height { + let row: String = (0..area.width) + .map(|x| buffer[(x, y)].symbol().chars().next().unwrap_or(' ')) + .collect(); + if let Some(at) = row.find(needle) { + let x = row[..at].chars().count() as u16; + return buffer[(x, y)].style(); + } + } + panic!("{needle:?} is not on screen"); + }; + + // Nothing reported and nothing asked for: the dark theme's #f0f6fc on a + // white terminal is 1.07:1, so the text goes to the terminal's own colour. + let unknown = draw(None, None, false); + for text in ["octocat", "Jan", "Mon"] { + assert_eq!(style_of(&unknown, text).fg, Some(Color::Reset), "{text}"); + } + let less = style_of(&unknown, "Less"); + assert_eq!(less.fg, Some(Color::Reset)); + assert!( + less.add_modifier.contains(Modifier::DIM), + "muted text stays a step down" + ); + // The cells keep their colours: they carry them on either background. + assert!(matches!( + style_of(&unknown, "\u{1FB2B}").fg, + Some(Color::Rgb(..)) + )); + // And the overlay says what to do about it. + let help = draw(None, None, true); + let text: String = help.content().iter().map(|cell| cell.symbol()).collect(); + assert!( + text.contains("--theme light"), + "the overlay should offer --theme light" + ); + + // Reported, or asked for, and the theme's own colours are back. + let light = draw(Some(Rgb::hex(0xffffff)), None, false); + assert_eq!( + style_of(&light, "octocat").fg, + Some(Color::Rgb(0x1f, 0x23, 0x28)) + ); + let dark = draw(None, Some(Appearance::Dark), false); + assert_eq!( + style_of(&dark, "octocat").fg, + Some(Color::Rgb(0xf0, 0xf6, 0xfc)) + ); + let help = draw(Some(Rgb::hex(0xffffff)), None, true); + let text: String = help.content().iter().map(|cell| cell.symbol()).collect(); + assert!( + !text.contains("--theme light"), + "nothing to suggest when it is known" + ); +} + #[test] fn a_kitty_transmission_carries_the_image_back() { use crate::graphics; diff --git a/src/ui.rs b/src/ui.rs index f9ba32d..b8c63f0 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -6,7 +6,7 @@ use chrono::{Datelike, Local, NaiveDate}; use ratatui::layout::Rect; -use ratatui::style::{Style, Stylize}; +use ratatui::style::{Color, Modifier, Style, Stylize}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Clear, Paragraph}; use ratatui::Frame; @@ -14,7 +14,6 @@ use ratatui::Frame; use crate::app::{App, CellStyle, Load, Mode}; use crate::calendar::{Calendar, Day}; use crate::graphics::COLUMNS_PER_DAY; -use crate::primer::Palette; use crate::thousands; /// Width of the weekday label column, e.g. "Mon ". @@ -306,9 +305,8 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App) { /// program can answer: what this terminal turned out to be able to draw. fn help(frame: &mut Frame<'_>, app: &App, inner: Rect) { let palette = &app.palette; - let key = Style::new().fg(palette.ansi(palette.fg)).bold(); - let text = Style::new().fg(palette.ansi(palette.fg)); - let muted = Style::new().fg(palette.ansi(palette.muted)); + let key = text(app).bold(); + let (text, muted) = (text(app), muted(app)); let cells = app.layout.map_or("—", |layout| layout.cells.name()); let yes_no = |flag: bool| if flag { "yes" } else { "no" }; @@ -383,6 +381,14 @@ fn help(frame: &mut Frame<'_>, app: &App, inner: Rect) { }, ), ]; + if !app.appearance_known() { + // The one guess left on screen, and the one a light terminal pays for. + lines.push(fact( + FACT, + "background", + "not reported — try --theme light".to_string(), + )); + } if !app.pixels_available() && app.caps.answered { lines.push(blank()); lines.push(( @@ -429,7 +435,13 @@ fn help(frame: &mut Frame<'_>, app: &App, inner: Rect) { let block = Block::bordered() .border_type(ratatui::widgets::BorderType::Rounded) .border_style(Style::new().fg(palette.ansi(palette.rule))) - .style(Style::new().bg(palette.ansi(palette.canvas))) + .style(match app.appearance_known() { + true => Style::new().bg(palette.ansi(palette.canvas)), + // The terminal's own background under the terminal's own text: the + // dark canvas under default text was the one pairing guaranteed + // unreadable on a light terminal. + false => Style::new().bg(Color::Reset), + }) .title(Line::styled(" help ", key)); frame.render_widget(Clear, area); frame.render_widget(Paragraph::new(lines).block(block), area); @@ -456,20 +468,13 @@ fn body(app: &App, inner: Rect) -> (Vec>, Option) { } fn header(app: &App) -> Line<'static> { - let palette = &app.palette; let mut spans = vec![ - Span::styled( - app.login.clone(), - Style::new().fg(palette.ansi(palette.fg)).bold(), - ), - separator(palette), - Span::styled( - app.year.to_string(), - Style::new().fg(palette.ansi(palette.fg)), - ), + Span::styled(app.login.clone(), text(app).bold()), + separator(app), + Span::styled(app.year.to_string(), text(app)), ]; if let Load::Ready(calendar) = &app.load { - spans.push(separator(palette)); + spans.push(separator(app)); // github.com's own wording, under its own chart — and github.com // counts what has *happened*, because it has no future data to count. // @@ -499,14 +504,33 @@ fn header(app: &App) -> Line<'static> { }; spans.push(Span::styled( format!("{} contributions in {}", thousands(counted), app.year), - Style::new().fg(palette.ansi(palette.fg)), + text(app), )); } Line::from(spans) } -fn separator(palette: &Palette) -> Span<'static> { - Span::styled(" · ", Style::new().fg(palette.ansi(palette.muted))) +fn separator(app: &App) -> Span<'static> { + Span::styled(" · ", muted(app)) +} + +/// Chrome text — labels, the header, the footer — in the theme's foreground, or +/// in the terminal's own when the theme is a guess (#141). Only the cells keep +/// their colours then: they carry them on any background, and text does not. +fn text(app: &App) -> Style { + match app.appearance_known() { + true => Style::new().fg(app.palette.ansi(app.palette.fg)), + false => Style::new().fg(Color::Reset), + } +} + +/// The quieter chrome: the theme's muted grey, or the terminal's own foreground +/// dimmed, which stays a step below [`text`] on a light terminal and a dark one. +fn muted(app: &App) -> Style { + match app.appearance_known() { + true => Style::new().fg(app.palette.ansi(app.palette.muted)), + false => Style::new().fg(Color::Reset).add_modifier(Modifier::DIM), + } } fn chart( @@ -566,7 +590,7 @@ fn note( width: usize, height: usize, ) -> Option> { - let muted = Style::new().fg(app.palette.ansi(app.palette.muted)); + let muted = muted(app); // Asking for a protocol and getting characters is the one outcome // `--capabilities` explained and the chart did not. if let Some(protocol) = app.graphics_refused() { @@ -625,7 +649,7 @@ fn note( /// a real year always yields its days, future ones included. fn empty(app: &App, calendar: &Calendar) -> Vec> { let year = calendar.year; - let muted = Style::new().fg(app.palette.ansi(app.palette.muted)); + let muted = muted(app); let reason = if calendar.starts_after(Local::now().date_naive()) { format!("{year} hasn't started yet — nothing to draw until Jan 1 {year}") } else { @@ -650,7 +674,7 @@ fn months(app: &App, calendar: &Calendar, cells: &Cells) -> Line<'static> { label.push_str(&" ".repeat(column - label.len())); label.push_str(name); } - Line::styled(label, Style::new().fg(app.palette.ansi(app.palette.fg))) + Line::styled(label, text(app)) } /// Box-bordered cells. Neighbours share a border, so each week costs two columns @@ -765,7 +789,7 @@ fn solid_rows(app: &App, calendar: &Calendar, fill: usize, gap: usize) -> Vec
  • Span<'static> { Span::styled( format!("{: Option { if app.hover == Some(date) { return Some(app.palette.ansi(app.palette.accent)); } - (app.cursor == date).then(|| app.palette.ansi(app.palette.fg)) + (app.cursor == date).then(|| text(app).fg.unwrap_or(Color::Reset)) } /// One day. Days outside the year and days still to come both draw blank, so the @@ -804,28 +828,24 @@ fn cell(app: &App, day: Option, fill: usize) -> Span<'static> { } fn detail(app: &App, calendar: &Calendar) -> Line<'static> { - let palette = &app.palette; let Some(day) = calendar.day(app.cursor) else { return Line::raw(""); }; let date = Span::styled( app.cursor.format("%a, %b %-d %Y").to_string(), - Style::new().fg(palette.ansi(palette.fg)).bold(), + text(app).bold(), ); if day.future { return Line::from(vec![ date, - separator(palette), - Span::styled( - "still to come", - Style::new().fg(palette.ansi(palette.muted)), - ), + separator(app), + Span::styled("still to come", muted(app)), ]); } Line::from(vec![ date, - separator(palette), - Span::styled(count(day.count), Style::new().fg(palette.ansi(palette.fg))), + separator(app), + Span::styled(count(day.count), text(app)), ]) } @@ -838,7 +858,7 @@ fn legend( row: u16, ) -> (Line<'static>, Option<(u16, u16)>) { let palette = &app.palette; - let muted = Style::new().fg(palette.ansi(palette.muted)); + let muted = muted(app); let lead = "Less "; let mut spans = vec![Span::styled(lead, muted)]; for level in 0..5u8 { @@ -884,7 +904,7 @@ fn legend( } fn summary(app: &App, calendar: &Calendar) -> Line<'static> { - let muted = Style::new().fg(app.palette.ansi(app.palette.muted)); + let muted = muted(app); if !calendar.has_elapsed_days() { return Line::styled(format!("{} hasn't started yet", calendar.year), muted); } @@ -995,42 +1015,28 @@ fn loading(app: &App) -> Line<'static> { let frame = SPINNER[(app.tick / 2) as usize % SPINNER.len()]; Line::from(vec![ Span::styled(frame, Style::new().fg(app.palette.level(4))), - Span::styled( - format!(" loading {} {}…", app.login, app.year), - Style::new().fg(app.palette.ansi(app.palette.muted)), - ), + Span::styled(format!(" loading {} {}…", app.login, app.year), muted(app)), ]) } fn failure(app: &App, message: &str) -> Vec> { - let palette = &app.palette; vec![ Line::from(vec![ Span::styled("!", Style::new().fg(ratatui::style::Color::Red).bold()), - Span::styled( - format!(" {message}"), - Style::new().fg(palette.ansi(palette.fg)), - ), + Span::styled(format!(" {message}"), text(app)), ]), Line::raw(""), - Line::styled( - "r retry · u change user", - Style::new().fg(palette.ansi(palette.muted)), - ), + Line::styled("r retry · u change user", muted(app)), ] } fn footer(app: &App) -> Line<'static> { - let palette = &app.palette; - let muted = Style::new().fg(palette.ansi(palette.muted)); + let muted = muted(app); match &app.mode { Mode::Input(buffer) => Line::from(vec![ Span::styled("user ", muted), - Span::styled( - buffer.clone(), - Style::new().fg(palette.ansi(palette.fg)).bold(), - ), - Span::styled("▏", Style::new().fg(palette.ansi(palette.fg))), + Span::styled(buffer.clone(), text(app).bold()), + Span::styled("▏", text(app)), Span::styled(" enter load · esc cancel", muted), ]), Mode::Normal => { From 0a4628a285ff7abec4fa26b6a8d4270aae8f575c Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:36:14 +0700 Subject: [PATCH 3/6] fix: keep the way out on screen in a small window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The source label trimmed q quit off the footer before itself, and a short window lost the note and the footer under the statistics. Short windows now give up the summary, spacing, detail and legend first; below the smallest chart they say "terminal too small (needs W×H) — q quits". The failure screen offers u only where it works, and the tooltip marker no longer overwrites a month label. Fixes #139 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 11 +++ README.md | 5 +- docs/DESIGN.md | 4 +- src/render_tests.rs | 121 ++++++++++++++++++++++++ src/ui.rs | 226 ++++++++++++++++++++++++++++++++------------ tests/smoke.rs | 33 +++++++ 6 files changed, 340 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a2a5ab..a4c68a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,17 @@ listed under a **Changed** or **Removed** heading. hard instead. The README no longer lists Windows Terminal among the terminals found automatically — on Windows nothing is asked, so it needs `--graphics sixel --cell WxH`. +- **In a small window the way out was the first thing to go** (#139). The + demo's and a file's source label trimmed `q quit` off the footer before + itself (56×24 showed only `? help · demo — …`); at 80×18 the note pushed + the footer off, and at 60×14 and 40×10 there was neither a footer nor any + word of why. A short window now gives up the statistics, then the spacing, + the detail line and the legend, and keeps the note and the keys; below + 59×14, where no chart fits beside them, it says `terminal too small (needs + 59×14) — q quits`. A style asked for that is taller than the window loses + its bottom rows instead of the footer. The failure screen offers `u` only + where it works — not for `--file` or `--demo` — and the tooltip's `▼` no + longer overwrites a letter of the month label beneath it. ### Changed diff --git a/README.md b/README.md index 35d4bc7..781f05e 100644 --- a/README.md +++ b/README.md @@ -476,7 +476,10 @@ palette is degraded on purpose, and what the capability probe actually sends. - **A 53-week year needs a 112×19 window** for pixel or square cells, 165 columns for rounded corners, 166×27 for the bordered grid. Below that `Auto` drops to a borderless style, where a future day and a day off the end of the year both look - blank; `d` still forces the others if you would rather they clipped. + blank; `d` still forces the others if you would rather they clipped. A shorter + window gives up the statistics line first, then the legend and the spacing, + and keeps the keys; below 59×14 there is no room for a chart and the way out + together, and the chart says so instead. - Private contributions appear only if the authenticated user can see them. ## Install diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 66a154a..aefe663 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -202,7 +202,9 @@ repairing it means re-emitting a slice of the image on a frame boundary that does not exist. One placement that works everywhere beat two that did not. What is kept: the wording is github.com's exactly, ordinals included, and a `▼` -points at the hovered week's column so the association is unambiguous. +points at the hovered week's column so the association is unambiguous — from a +column of that week clear of the month labels it shares a row with, or not at +all, since a `▼` drawn over one turned `Jan` into `J▼n`. ## 8. Two pointers, not one diff --git a/src/render_tests.rs b/src/render_tests.rs index 775b004..634fcff 100644 --- a/src/render_tests.rs +++ b/src/render_tests.rs @@ -1712,6 +1712,127 @@ fn hovering_a_day_reads_it_out_the_way_github_does() { assert!(!frame.contains("▼"), "the tooltip should be gone:\n{frame}"); } +#[test] +fn the_tooltip_marker_keeps_off_the_month_labels() { + use crate::app::CellStyle; + // `▼` shares its row with the month labels, and drawn over one it read + // `J▼n` (#139). Squares put a week every two columns, so "Jan" covers the + // whole of week 0 and half of week 1. + let mut app = ready(synthetic(2025, "octocat")); + app.cells = CellStyle::Squares; + render(&mut app, 176, 24); + let layout = app.layout.unwrap(); + let months_row = (layout.y - 1) as usize; + let Load::Ready(calendar) = &app.load else { + unreachable!() + }; + let (first, later) = ( + calendar.weeks[0].days[3].unwrap().date, + calendar.weeks[10].days[3].unwrap().date, + ); + + app.hover = Some(first); + let lines = rows(&render(&mut app, 176, 24)); + assert!( + lines[months_row].contains("Jan"), + "the label was written over: {:?}", + lines[months_row] + ); + assert!( + lines[months_row - 1].contains("on January"), + "the tooltip still names the day" + ); + + // Clear of a label, the marker points at the week as before. + app.hover = Some(later); + let lines = rows(&render(&mut app, 176, 24)); + let marker = lines[months_row].chars().position(|c| c == '▼'); + let column = (layout.x + 10 * 2) as usize; + assert!( + marker.is_some_and(|at| (column..column + 2).contains(&at)), + "the marker should sit over week 10: {:?}", + lines[months_row] + ); +} + +#[test] +fn a_small_window_keeps_the_way_out_on_screen() { + use crate::app::{CellStyle, Source}; + + // The demo's source label sat after `q quit` in the trim order, so at 56 + // columns the footer read only `? help · demo — …` (#139). + let mut demo = ready(synthetic(2025, "demo")); + demo.source = Source::Demo; + let frame = render(&mut demo, 56, 24); + assert!( + frame.contains("q quit") && frame.contains("? help"), + "{frame}" + ); + + // At 80x18 the note showed and the footer under it did not; the statistics + // are worth less than either. + let mut app = ready(synthetic(2025, "octocat")); + let frame = render(&mut app, 80, 18); + assert!(frame.contains("q quit"), "{frame}"); + assert!(frame.contains("the chart needs 19 rows"), "{frame}"); + assert!( + !frame.contains("active days"), + "the summary goes first:\n{frame}" + ); + assert!( + frame.contains("Less"), + "the legend need not go yet:\n{frame}" + ); + + // At 60x14 there was neither the footer nor anything saying why: it is + // the smallest window that holds the grid, the note and the way out. + let frame = render(&mut app, 60, 14); + assert!(frame.contains("q quit") && frame.contains("Mon"), "{frame}"); + assert!(frame.contains("the chart needs 19 rows"), "{frame}"); + assert!(app.layout.is_some()); + + // One row less, and a chart would have to give up the way out to fit, so it + // says how big a window it needs instead — in the units a terminal reports. + let frame = render(&mut app, 60, 13); + assert!( + frame.contains("terminal too small (needs 59×14) — q quits"), + "{frame}" + ); + assert!(app.layout.is_none(), "nothing for the mouse or the painter"); + let frame = render(&mut app, 40, 10); + assert!( + frame.contains("terminal too small") && frame.contains("quits"), + "wrapped, not cut off:\n{frame}" + ); + + // A style asked for and taller than the window gives up its bottom rows, + // not the footer — and the rows it gave up are not days to the mouse. + app.cells = CellStyle::Grid; + let frame = render(&mut app, 176, 20); + assert!(frame.contains("q quit"), "{frame}"); + let layout = app.layout.unwrap(); + assert!(layout.bottom < 19, "the grid was not cut: {layout:?}"); + assert_eq!(layout.hit(layout.x, layout.bottom), None); + assert!(layout.hit(layout.x, layout.bottom - 1).is_some()); +} + +#[test] +fn the_failure_screen_offers_only_keys_that_work() { + use crate::app::Source; + // `u` is ignored for a file and the demo, and a missing file is the + // commonest failure there is (#139). + let mut file = App::new("preview".into(), 2027, Source::File("nope.json".into())); + file.load = Load::Failed("could not read nope.json".into()); + let frame = render(&mut file, 80, 12); + assert!(frame.contains("r retry"), "{frame}"); + assert!(!frame.contains("u change user"), "{frame}"); + + let mut account = App::new("octocat".into(), 2027, Source::GitHub); + account.load = Load::Failed("Could not resolve to a User".into()); + let frame = render(&mut account, 80, 12); + assert!(frame.contains("u change user"), "{frame}"); +} + #[test] fn a_day_still_to_come_has_no_tooltip() { use ratatui::crossterm::event::{KeyModifiers, MouseEvent, MouseEventKind}; diff --git a/src/ui.rs b/src/ui.rs index b8c63f0..0f1ff9f 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -8,7 +8,7 @@ use chrono::{Datelike, Local, NaiveDate}; use ratatui::layout::Rect; use ratatui::style::{Color, Modifier, Style, Stylize}; use ratatui::text::{Line, Span}; -use ratatui::widgets::{Block, Clear, Paragraph}; +use ratatui::widgets::{Block, Clear, Paragraph, Wrap}; use ratatui::Frame; use crate::app::{App, CellStyle, Load, Mode}; @@ -58,6 +58,18 @@ pub(crate) const CHROME_ROWS: usize = 10; /// not fit. The number a reader can act on is the one their terminal reports. pub(crate) const BORDER: usize = 2; +/// Rows the chart keeps however short the window: the header and the blank +/// under it, the month labels, the note and the footer. Everything else under +/// the grid is given up first, the statistics before anything — at 80×18 the +/// footer used to be the line that fell off, and at 60×14 the note with it, +/// leaving no way out on screen (#139). Below this plus the narrowest grid, the +/// chart is replaced by one line saying how big a window it needs. +pub(crate) const KEPT_ROWS: usize = 5; + +/// The style the chart falls back to when nothing else fits, and so the one the +/// smallest usable window is measured against. +const SMALLEST: Cells = Cells::Solid { fill: 1, gap: 0 }; + /// A concrete cell layout, resolved from a [`CellStyle`] and the space available. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Cells { @@ -217,6 +229,10 @@ impl Layout { /// gap beside a cell, or on a grid rule, snaps to the day it borders rather than /// falling through, because a two-column target is small enough already. pub fn hit(&self, column: u16, row: u16) -> Option<(usize, usize)> { + // A grid cut short for the footer's sake has no days under the cut. + if row >= self.bottom { + return None; + } let dx = usize::from(column.checked_sub(self.x)?); let dy = usize::from(row.checked_sub(self.y)?); let week = dx / self.cells.stride(); @@ -289,8 +305,15 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App) { frame.render_widget(block, area); app.footer_width = inner.width; - let (lines, layout) = body(app, inner); - frame.render_widget(Paragraph::new(lines), inner); + let (lines, layout, wrap) = body(app, inner); + let paragraph = Paragraph::new(lines); + frame.render_widget( + match wrap { + true => paragraph.wrap(Wrap { trim: true }), + false => paragraph, + }, + inner, + ); app.layout = layout; if let Some(layout) = layout { @@ -447,24 +470,45 @@ fn help(frame: &mut Frame<'_>, app: &App, inner: Rect) { frame.render_widget(Paragraph::new(lines).block(block), area); } -fn body(app: &App, inner: Rect) -> (Vec>, Option) { +/// The lines to draw, where the grid landed, and whether the lines should wrap — +/// which only the one-line "too small" message wants. +fn body(app: &App, inner: Rect) -> (Vec>, Option, bool) { let mut lines = vec![header(app), Line::raw("")]; - let mut layout = None; match &app.load { Load::Loading => lines.push(loading(app)), Load::Failed(message) => lines.extend(failure(app, message)), - Load::Ready(calendar) => { + Load::Ready(calendar) if !calendar.weeks.is_empty() => { // The chart starts two lines down, which is what turns a line index into // a screen row. let origin = (inner.x, inner.y + lines.len() as u16); - let (chart, resolved) = chart(app, calendar, inner, origin); + // What is left between those two lines and the footer. + let room = usize::from(inner.height).saturating_sub(lines.len() + 1); + let Some((chart, layout)) = chart(app, calendar, inner, origin, room) else { + return (vec![too_small(app, calendar.weeks.len())], None, true); + }; lines.extend(chart); - layout = resolved; + lines.push(footer(app)); + return (lines, Some(layout), false); } + Load::Ready(calendar) => lines.extend(empty(app, calendar)), } lines.push(Line::raw("")); lines.push(footer(app)); - (lines, layout) + (lines, None, false) +} + +/// Instead of a chart with no room for the way out. Measured against the +/// narrowest style and the rows [`KEPT_ROWS`] keeps, in the units a terminal +/// reports, so resizing to the number it names is enough. +fn too_small(app: &App, weeks: usize) -> Line<'static> { + let (width, height) = ( + SMALLEST.width(weeks) + BORDER, + SMALLEST.height() + KEPT_ROWS + BORDER, + ); + Line::styled( + format!("terminal too small (needs {width}×{height}) — q quits"), + text(app), + ) } fn header(app: &App) -> Line<'static> { @@ -533,53 +577,92 @@ fn muted(app: &App) -> Style { } } +/// The month labels, the grid and everything under it, in `room` rows — or +/// `None` when even the narrowest grid and the rows [`KEPT_ROWS`] keeps do not +/// fit, and the window is too small for a chart at all. fn chart( app: &App, calendar: &Calendar, inner: Rect, origin: (u16, u16), -) -> (Vec>, Option) { - if calendar.weeks.is_empty() { - return (empty(app, calendar), None); - } - + room: usize, +) -> Option<(Vec>, Layout)> { let weeks = calendar.weeks.len(); let (width, height) = (inner.width as usize, inner.height as usize); let cells = resolve(app.cells, weeks, width, height, app.pixels_available()); + let note = note(app, &cells, weeks, width, height); - let mut lines = Vec::with_capacity(cells.height() + 5); - lines.push(months(app, calendar, &cells)); - let grid_row = origin.1 + lines.len() as u16; - match cells { + // The month labels and the note go nowhere; the grid needs its seven rows + // in the narrowest style, which `d` can always reach. + let kept = 1 + usize::from(note.is_some()); + if kept + SMALLEST.height() > room { + return None; + } + + let mut grid = match cells { // Nothing but the gutter: the seven rows below belong to the painter, and // staying blank is what keeps ratatui's diff from writing over the image. - Cells::Pixels => lines.extend((0..7).map(|row| Line::from(weekday(app, row)))), - Cells::Rounded { gap } => lines.extend(filled_rows(app, calendar, ROUND, gap)), - Cells::Squares => lines.extend(filled_rows(app, calendar, SQUARE, 1)), - Cells::Grid { fill } => lines.extend(grid_rows(app, calendar, fill)), - Cells::Solid { fill, gap } => lines.extend(solid_rows(app, calendar, fill, gap)), + Cells::Pixels => (0..7).map(|row| Line::from(weekday(app, row))).collect(), + Cells::Rounded { gap } => filled_rows(app, calendar, ROUND, gap), + Cells::Squares => filled_rows(app, calendar, SQUARE, 1), + Cells::Grid { fill } => grid_rows(app, calendar, fill), + Cells::Solid { fill, gap } => solid_rows(app, calendar, fill, gap), + }; + // Only a style asked for can be taller than the window — Auto takes one + // that fits — and it loses its bottom rows rather than the footer. + let cut = grid.len() > room - kept; + grid.truncate(room - kept); + + // Under the grid, each line with what it is worth; the cheapest goes first + // until the rest fit. `KEEP` never goes. + const KEEP: u8 = u8::MAX; + let (legend, legend_x) = legend(app, calendar, &cells, inner.x); + let mut under: Vec<(u8, Line<'static>)> = vec![ + (3, Line::raw("")), + (4, detail(app, calendar)), + (5, legend), + (1, summary(app, calendar)), + ]; + if let Some(note) = note { + under.push((KEEP, note)); } - lines.push(Line::raw("")); - lines.push(detail(app, calendar)); - - let legend_row = origin.1 + lines.len() as u16; - let (legend, legend_at) = legend(app, calendar, &cells, inner.x, legend_row); - lines.push(legend); - lines.push(summary(app, calendar)); - if let Some(note) = note(app, &cells, weeks, width, height) { - lines.push(note); + under.push((2, Line::raw(""))); + while 1 + grid.len() + under.len() > room { + let Some(cheapest) = under + .iter() + .enumerate() + .filter(|(_, (rank, _))| *rank != KEEP) + .min_by_key(|(_, (rank, _))| *rank) + .map(|(index, _)| index) + else { + break; + }; + under.remove(cheapest); } + let grid_row = origin.1 + 1; + let legend_at = under + .iter() + .position(|(rank, _)| *rank == 5) + .and_then(|index| Some((legend_x?, grid_row + (grid.len() + index) as u16))); let layout = Layout { x: origin.0 + (GUTTER + cells.offset()) as u16, y: grid_row, weeks: weeks as u16, cells, legend: legend_at, - bottom: inner.bottom(), + bottom: match cut { + true => grid_row + grid.len() as u16, + false => inner.bottom(), + }, right: inner.right(), }; - (lines, Some(layout)) + + let mut lines = Vec::with_capacity(1 + grid.len() + under.len()); + lines.push(months(app, calendar, &cells)); + lines.extend(grid); + lines.extend(under.into_iter().map(|(_, line)| line)); + Some((lines, layout)) } /// The line under the chart that explains why it does not look its best. @@ -655,10 +738,16 @@ fn empty(app: &App, calendar: &Calendar) -> Vec> { } else { format!("no contribution data for {year}") }; + // A file or the demo has one year and one user, so neither key does + // anything there; offering them is how a reader learns to distrust the rest. + let keys = match app.previewing() { + true => "r reload", + false => "[ previous year · u change user", + }; vec![ Line::styled(reason, muted), Line::raw(""), - Line::styled("[ previous year · u change user", muted), + Line::styled(keys, muted), ] } @@ -849,14 +938,8 @@ fn detail(app: &App, calendar: &Calendar) -> Line<'static> { ]) } -/// Returns the line and, in pixel mode, where the swatch image goes. -fn legend( - app: &App, - calendar: &Calendar, - cells: &Cells, - x: u16, - row: u16, -) -> (Line<'static>, Option<(u16, u16)>) { +/// Returns the line and, in pixel mode, the column the swatch image goes in. +fn legend(app: &App, calendar: &Calendar, cells: &Cells, x: u16) -> (Line<'static>, Option) { let palette = &app.palette; let muted = muted(app); let lead = "Less "; @@ -899,7 +982,7 @@ fn legend( }, muted, )); - let at = matches!(cells, Cells::Pixels).then(|| (x + lead.len() as u16, row)); + let at = matches!(cells, Cells::Pixels).then(|| x + lead.len() as u16); (Line::from(spans), at) } @@ -980,15 +1063,27 @@ fn tooltip(frame: &mut Frame<'_>, app: &App, layout: &Layout, inner: Rect) { Span::styled("▌", Style::new().fg(background)), ]); frame.render_widget(Paragraph::new(pill), Rect::new(x, text_row, width, 1)); - frame.render_widget( - Paragraph::new(Line::from(Span::styled("▼", Style::new().fg(background)))), - Rect::new( - point.min(inner.right().saturating_sub(1)), - text_row + 1, - 1, - 1, - ), - ); + + // The marker shares its row with the month labels, and drawn over one it + // turned `Jan` into `J▼n` (#139). So it takes a column of the day's cell + // that is clear of them — the middle first — and is left out where there is + // none: the pill above it still names the day. + let cell = layout.x + week as u16 * layout.cells.stride() as u16; + let marker = std::iter::once(point) + .chain(cell..cell + layout.cells.stride() as u16) + .filter(|column| *column < inner.right()) + .find(|column| { + frame + .buffer_mut() + .cell((*column, text_row + 1)) + .is_some_and(|under| under.symbol() == " ") + }); + if let Some(column) = marker { + frame.render_widget( + Paragraph::new(Line::from(Span::styled("▼", Style::new().fg(background)))), + Rect::new(column, text_row + 1, 1, 1), + ); + } } /// "No contributions" / "1 contribution" / "97 contributions", as github.com writes it. @@ -1026,7 +1121,15 @@ fn failure(app: &App, message: &str) -> Vec> { Span::styled(format!(" {message}"), text(app)), ]), Line::raw(""), - Line::styled("r retry · u change user", muted(app)), + // `u` does nothing for a file or the demo, which is where a failure + // most often happens: a path that does not exist yet (#139). + Line::styled( + match app.previewing() { + true => "r retry", + false => "r retry · u change user", + }, + muted(app), + ), ] } @@ -1062,7 +1165,8 @@ fn footer(app: &App) -> Line<'static> { keys.push("r reload"); keys.push("q quit"); keys.push("? help"); - if let Some(label) = app.source_label() { + let label = app.source_label(); + if let Some(label) = label { keys.push(label); } @@ -1077,11 +1181,17 @@ fn footer(app: &App) -> Line<'static> { <= width }; // Dropped from the front, where the movement hints are — those are the - // guessable ones — and never past the last two, which are `q quit` - // and `? help`. - while !fits(&keys) && keys.len() > 2 { + // guessable ones — then the source label, and never `q quit` or + // `? help`. The label sits after them on the line but goes before + // them: trimming from the front alone took `q quit` first, so a + // 56-column demo said only `? help · demo — …` (#139). + let floor = 2 + usize::from(label.is_some()); + while !fits(&keys) && keys.len() > floor { keys.remove(0); } + if !fits(&keys) && label.is_some() { + keys.pop(); + } Line::styled(keys.join(JOIN), muted) } } diff --git a/tests/smoke.rs b/tests/smoke.rs index ee7119f..cdde943 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -906,3 +906,36 @@ fn a_reply_that_misses_the_probe_is_not_read_as_keys() -> termlens::Result<()> { assert!(t.wait_exit()?.success()); Ok(()) } + +/// The sizes #139 measured: at 80x18 the note showed and the footer did not, at +/// 60x14 neither did, and at 40x10 nothing said the window was the problem. +#[test] +fn a_short_window_gives_up_the_statistics_before_the_way_out() -> termlens::Result<()> { + for size in [(80u16, 18u16), (60, 14)] { + let mut t = spawn(&PREVIEW, size, |b| b)?; + let screen = t.wait_frame(|s| s.contains("contributions in") && s.contains("needs"))?; + assert!( + screen.contains("q quit") && screen.contains("the chart needs 19 rows"), + "at {size:?} the note or the footer is missing:\n{screen}" + ); + t.send(Key::Char('q'))?; + assert!(t.wait_exit()?.success()); + } + + // The demo's label trimmed `q quit` away before itself at 56 columns. + let mut t = spawn(&["--demo"], (56, 24), |b| b)?; + let screen = t.wait_frame(|s| s.contains("contributions in") && s.contains("? help"))?; + assert!(screen.contains("q quit"), "{screen}"); + t.send(Key::Char('q'))?; + assert!(t.wait_exit()?.success()); + + let mut t = spawn(&PREVIEW, (40, 10), |b| b)?; + let screen = t.wait_frame(|s| s.contains("terminal too small"))?; + assert!( + screen.contains("59×14"), + "it names the size it needs:\n{screen}" + ); + t.send(Key::Char('q'))?; + assert!(t.wait_exit()?.success(), "and q is still the way out"); + Ok(()) +} From 1354fcb1dbe70ae57370ab1c1c828f80a5fffd03 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:39:21 +0700 Subject: [PATCH 4/6] feat: add --cells, and draw forced pixel cells in a narrow window --cells STYLE starts the chart in a named cell style, and mossaic-glyphs points at it. Pixel cells forced into a window too narrow for them drew nothing; they now draw the weeks that fit, and d from a style that does not fit goes to the best one that does. A terminal that speaks kitty or sixel without reporting its cell size is told to pass --cell WxH. Fixes #142 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 19 ++++++++ README.md | 9 +++- docs/DESIGN.md | 5 +- src/app.rs | 99 ++++++++++++++++++++++++++++++++++++++- src/bin/mossaic-glyphs.rs | 9 ++-- src/main.rs | 15 +++++- src/render_tests.rs | 28 +++++++++++ src/ui.rs | 47 +++++++++++++++++-- tests/chart_cli.rs | 12 +++++ tests/pixels.rs | 61 ++++++++++++++++++++++++ tests/smoke.rs | 16 +++++++ 11 files changed, 305 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4c68a1..8f0814d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ listed under a **Changed** or **Removed** heading. ## [Unreleased] +### Added + +- **`--cells STYLE`** (#142): start the chart in a cell style — `auto`, + `pixel`, `rounded`, `snug`, `squares`, `grid`, `spaced`, `blocks`, `slim` or + `compact`, the names the legend prints. A font with no block sextants meant + pressing `d` on every launch to get off rounded; `mossaic-glyphs`, the + check for exactly that font, now names `--cells squares`. `Options` gains a + `cells` field for library users. + ### Fixed - **A terminal reply that arrived after the start-up probe was read as @@ -42,6 +51,16 @@ listed under a **Changed** or **Removed** heading. its bottom rows instead of the footer. The failure screen offers `u` only where it works — not for `--file` or `--demo` — and the tooltip's `▼` no longer overwrites a letter of the month label beneath it. +- **Pixel cells forced into a narrow window drew nothing** (#142). After `d` + at under 112 columns the grid and the legend were blank, no image was sent + at all, and the undrawn days still answered the mouse. The weeks that fit + are drawn now, clipped at the edge as text is, and only those are days to + the mouse. `d` from any style that does not fit goes to the best one that + does rather than on round the cycle, which could be wider still. A terminal + that speaks kitty or sixel but does not report its cell size is told to + pass `--cell WxH` — under the legend and in the help overlay — where before + the hint appeared only for a forced `--graphics`, and the overlay suggested + another terminal. ### Changed diff --git a/README.md b/README.md index 781f05e..86773f7 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,11 @@ cells kitty Windows Terminal draws sixel when told to: `mossaic --graphics sixel --cell WxH`, measuring the cell yourself, and `--theme light` on a light background. - **A window under 112 columns** falls back too, and the chart says so under the - legend, naming the number it wants. + legend, naming the number it wants. Forced with `d`, pixel cells draw the weeks + that fit. +- **`cell unknown`** with `kitty yes` or `sixel yes` means the terminal can draw + but did not say how big a character cell is, and an image cannot be lined up + without it. The chart says so too; measure it and pass `--cell WxH`. - Either way, `mossaic --png chart.png` renders the same image to a file from any terminal at all. @@ -198,6 +202,7 @@ mossaic --capabilities # what your terminal can draw mossaic --png chart.png # the chart as an image, from any terminal mossaic --graphics sixel # force a protocol, or `text` for no pixels mossaic --cell 10x20 # when the terminal will not say how big a cell is +mossaic --cells squares # start in a cell style, instead of the best that fits mossaic --today 2027-06-30 # read the year as of a day, not the clock mossaic --theme light # instead of following the terminal's background mossaic --palette winter # GitHub's own seasonal colours @@ -392,7 +397,7 @@ kitty protocol. | `Home` / `End` | first / last day in range | | `?` | keys, mouse, and what this terminal can draw | | `u` | type a different username (`Enter` load, `Esc` cancel) | -| `d` | cycle cell style — auto / pixel / rounded / snug / squares / grid / spaced / blocks / slim / compact. The legend names it, and says `auto:` while the chart is still choosing | +| `d` | cycle cell style — auto / pixel / rounded / snug / squares / grid / spaced / blocks / slim / compact. The legend names it, and says `auto:` while the chart is still choosing. From a style that does not fit, it goes to the best one that does. `--cells NAME` starts in one | | `m` | mouse reporting on or off — off gives the terminal its own selection back | | `r` | reload | | `q` | quit — `Esc` cancels the username prompt and closes the help overlay, and does nothing else | diff --git a/docs/DESIGN.md b/docs/DESIGN.md index aefe663..13adeab 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -183,8 +183,9 @@ cannot draw. **Without a cell size there are no pixels.** An image that cannot be lined up with the labels around it is worse than no image, so a terminal that will not say -gets text cells — and the chart says so under the legend rather than leaving a -`--graphics kitty` looking ignored. There are three places the size can come +gets text cells — and the chart says so under the legend, naming `--cell WxH`, +whether the protocol was forced with `--graphics` or only answered for: text +cells from a terminal that said yes to kitty look like a terminal that said no. There are three places the size can come from, in order: `TIOCGWINSZ`, the `CSI 16 t` reply, and the window size from `CSI 14 t` divided by the grid. Or you measure it yourself and pass `--cell 10x20`, which outranks all three. A resize re-asks, because that is also diff --git a/src/app.rs b/src/app.rs index 5953077..9920e99 100644 --- a/src/app.rs +++ b/src/app.rs @@ -13,7 +13,7 @@ use crate::github; use crate::graphics::{Mark, Painter, Protocol, Ring, Scene, COLUMNS_PER_DAY}; use crate::primer::{Appearance, Palette, Season}; use crate::term::{self, Caps}; -use crate::ui::{Cells, Layout}; +use crate::ui::{self, Cells, Layout}; /// Where the calendar comes from. #[derive(Debug, Clone)] @@ -69,6 +69,9 @@ pub struct Options { /// Which day to read the year as of. `None` is the clock — and, for a saved /// calendar, means every day in it counts as elapsed. pub today: Option, + /// The cell style to start in, which `--cells` names. A font with no block + /// sextants meant pressing `d` on every launch to get away from them. + pub cells: CellStyle, } impl Default for Options { @@ -80,6 +83,7 @@ impl Default for Options { mouse: true, cell: None, today: None, + cells: CellStyle::Auto, } } } @@ -115,6 +119,32 @@ pub enum CellStyle { } impl CellStyle { + /// Every style by the name the legend gives it, in the order `d` visits + /// them — which is what `--cells` accepts and `--help` lists. + pub const NAMES: [(&'static str, CellStyle); 10] = [ + ("auto", Self::Auto), + ("pixel", Self::Pixels), + ("rounded", Self::Rounded), + ("snug", Self::Snug), + ("squares", Self::Squares), + ("grid", Self::Grid), + ("spaced", Self::Spaced), + ("blocks", Self::Blocks), + ("slim", Self::Slim), + ("compact", Self::Compact), + ]; + + /// The style a name means, as the legend writes it. `pixels` is accepted + /// too, being what the README calls them. + pub fn parse(name: &str) -> Option { + let name = name.trim().to_ascii_lowercase(); + let name = if name == "pixels" { "pixel" } else { &name }; + Self::NAMES + .iter() + .find(|(known, _)| *known == name) + .map(|(_, style)| *style) + } + /// The next style in the cycle. Pixels are skipped where the terminal cannot /// draw them, since asking for them there resolves to rounded and pressing `d` /// would look like it had done nothing. @@ -188,6 +218,9 @@ pub struct App { /// Columns the footer may use, set by the renderer each frame so it can drop /// what will not fit rather than being cut mid-word. pub footer_width: u16, + /// The area the chart was laid out in last frame, columns by rows, so `d` + /// can tell a style that fits from one that does not. + pub(crate) area: (u16, u16), /// Whether mouse reporting is on; `m` toggles it. pub mouse: bool, /// Whether the help overlay is up. `?` opens it, any key closes it. @@ -242,6 +275,7 @@ impl App { gfx: None, layout: None, footer_width: 0, + area: (0, 0), mouse: false, help: false, redraw: false, @@ -268,6 +302,7 @@ impl App { self.today = options.today; self.wanted = options.graphics; self.cell_override = options.cell; + self.cells = options.cells; let known = options .appearance .or_else(|| caps.background.map(Appearance::from_background)); @@ -311,6 +346,24 @@ impl App { self.gfx.is_some() } + /// The protocol the terminal speaks, or was told to, when it could not be + /// used because nobody said how big a character cell is — which is the only + /// reason a protocol goes unused. A terminal that answered the kitty query + /// and not the size got text cells silently, and a help overlay suggesting + /// it try another terminal (#142). + pub fn cell_size_missing(&self) -> Option<&'static str> { + if self.gfx.is_some() { + return None; + } + match self.wanted { + Graphics::Kitty => Some("kitty"), + Graphics::Sixel => Some("sixel"), + Graphics::Auto if self.caps.kitty => Some("kitty"), + Graphics::Auto if self.caps.sixel => Some("sixel"), + Graphics::Auto | Graphics::Text => None, + } + } + /// Whether a protocol was asked for and could not be used. The chart says so /// under the legend: asking for pixels and silently getting characters is the /// one outcome `--capabilities` explains and the chart did not. @@ -492,7 +545,7 @@ impl App { // A style change moves or removes the image, so the screen has to go // with it: pixels the text layer never wrote are pixels it cannot clear. KeyCode::Char('d') => { - self.cells = self.cells.next(self.pixels_available()); + self.cells = self.next_cells(); self.redraw = true; } KeyCode::Char('?') | KeyCode::F(1) => { @@ -512,6 +565,41 @@ impl App { } } + /// Where `d` goes from here. Round the cycle — except from a style that + /// does not fit, where the next one in the cycle was as likely to be wider + /// as narrower and "press d for a smaller style" walked through bigger ones + /// (#142). From there it goes to the best style that does fit, in Auto's + /// order of preference. + fn next_cells(&self) -> CellStyle { + let pixels = self.pixels_available(); + let next = self.cells.next(pixels); + let Load::Ready(calendar) = &self.load else { + return next; + }; + let weeks = calendar.weeks.len(); + let (width, height) = (usize::from(self.area.0), usize::from(self.area.1)); + let fits = |style: CellStyle| { + ui::resolve(style, weeks, width, height, pixels).fits(weeks, width, height) + }; + if fits(self.cells) { + return next; + } + [ + CellStyle::Pixels, + CellStyle::Rounded, + CellStyle::Squares, + CellStyle::Grid, + CellStyle::Spaced, + CellStyle::Blocks, + CellStyle::Slim, + CellStyle::Compact, + ] + .into_iter() + .filter(|style| *style != CellStyle::Pixels || pixels) + .find(|style| fits(*style)) + .unwrap_or(next) + } + fn on_key_input(&mut self, code: KeyCode) { let Mode::Input(buffer) = &mut self.mode else { return; @@ -726,9 +814,12 @@ fn scene<'a>( return None; }; + // The weeks the layout found room for, which is the whole year unless the + // style was forced into a window too narrow for it. let levels: Vec<[Option; 7]> = calendar .weeks .iter() + .take(usize::from(layout.weeks)) .map(|week| { let mut column = [None; 7]; for (weekday, day) in week.days.iter().enumerate() { @@ -743,6 +834,10 @@ fn scene<'a>( let mark = |date: Option, ring: Ring| -> Option { let date = date?; let (week, weekday) = calendar.position(date)?; + // A ring on a week that was not drawn would land past the edge. + if week >= usize::from(layout.weeks) { + return None; + } Some(Mark { week: week as u16, weekday: weekday as u16, diff --git a/src/bin/mossaic-glyphs.rs b/src/bin/mossaic-glyphs.rs index 7fc0120..760a17e 100644 --- a/src/bin/mossaic-glyphs.rs +++ b/src/bin/mossaic-glyphs.rs @@ -7,8 +7,8 @@ //! The `rounded` style needs block sextants (the U+1FB00 block, Unicode 13). //! Terminals that draw box characters themselves — VTE 0.66+, kitty, foot, //! WezTerm — render them whatever the font holds. If the rounded row below shows -//! boxes, question marks or blanks instead of small squares, press `d` in -//! mossaic to fall back to `squares`, which needs nothing beyond U+2580. +//! boxes, question marks or blanks instead of small squares, start mossaic with +//! `--cells squares`, which needs nothing beyond U+2580. //! //! None of this applies when the terminal draws pixels: `mossaic //! --capabilities` says whether yours does. @@ -101,6 +101,9 @@ fn main() { println!(" Each row should read as separated small squares, dark to bright green."); println!(" The rounded row should have visibly softened corners."); println!(); + // Named, rather than "press d": the chart picks rounded again on every + // launch, so a font without sextants meant pressing it every time (#142). println!(" If rounded looks wrong, everything else still works:"); - println!(" mossaic then press d to cycle styles"); + println!(" mossaic --cells squares start in squares, or any row above"); + println!(" mossaic then press d to cycle styles"); } diff --git a/src/main.rs b/src/main.rs index 3fb432b..de17310 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,7 +15,7 @@ use ratatui::crossterm::execute; use ratatui::crossterm::terminal::{BeginSynchronizedUpdate, EndSynchronizedUpdate}; use ratatui::DefaultTerminal; -use mossaic::app::{App, Graphics, Load, Options, Source}; +use mossaic::app::{App, CellStyle, Graphics, Load, Options, Source}; use mossaic::cli::Args; use mossaic::primer::{Appearance, Palette, Season}; use mossaic::{github, graphics, png, term, ui}; @@ -43,6 +43,9 @@ options: -g, --graphics M auto | kitty | sixel | text (default: ask the terminal) --cell WxH size of one character cell in pixels, when the terminal will not say (e.g. --cell 10x20) + --cells STYLE the cell style to start in: auto | pixel | rounded | snug | + squares | grid | spaced | blocks | slim | compact + (default: auto, the most faithful that fits; d cycles) --today DATE what \"today\" means, as YYYY-MM-DD (default: the clock). It decides which days count as still to come — and, with --file, makes a saved year show them at all @@ -388,6 +391,16 @@ fn parse_args() -> Option { } "--no-mouse" => options.mouse = false, "--cell" => options.cell = Some(parse_cell(&args.value("--cell"))), + "--cells" => { + let raw = args.value("--cells"); + options.cells = CellStyle::parse(&raw).unwrap_or_else(|| { + let names: Vec<&str> = CellStyle::NAMES.iter().map(|(name, _)| *name).collect(); + fail(&format!( + "unknown cell style {raw:?} — {}", + names.join(", ") + )) + }); + } "--png" => png = Some(args.value("--png")), "--capabilities" => capabilities = true, // The same hole, in the same shared parser: `mossaic -- -weirdlogin` diff --git a/src/render_tests.rs b/src/render_tests.rs index 634fcff..b6da4f7 100644 --- a/src/render_tests.rs +++ b/src/render_tests.rs @@ -215,6 +215,34 @@ fn auto_picks_the_most_faithful_style_that_fits() { } } +#[test] +fn a_cell_style_is_named_the_way_the_legend_names_it() { + use crate::app::{CellStyle, Options}; + use crate::ui::resolve; + // `--cells` takes the word the legend prints, so what a reader sees is what + // they type (#142). Pixels resolve to "pixel" only where they can be drawn. + for (name, style) in CellStyle::NAMES { + assert_eq!(CellStyle::parse(name), Some(style), "{name}"); + if style != CellStyle::Auto { + assert_eq!(resolve(style, 53, 300, 60, true).name(), name, "{name}"); + } + } + assert_eq!(CellStyle::parse("pixels"), Some(CellStyle::Pixels)); + assert_eq!(CellStyle::parse("Squares"), Some(CellStyle::Squares)); + assert_eq!(CellStyle::parse("hexagons"), None); + + // And the chart starts in it. + let mut app = ready(synthetic(2025, "octocat")); + app.configure( + crate::term::Caps::default(), + Options { + cells: CellStyle::Squares, + ..Options::default() + }, + ); + assert!(render(&mut app, 200, 30).contains(" · squares cells")); +} + /// Column of each drawn cell on every weekday row, measured from the frame. fn cell_columns(frame: &str, glyph: char) -> Vec> { frame diff --git a/src/ui.rs b/src/ui.rs index 0f1ff9f..f830f27 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -185,7 +185,8 @@ impl Cells { } } - const fn fits(&self, weeks: usize, width: usize, height: usize) -> bool { + /// Whether the whole chart fits `width` by `height` in these cells. + pub(crate) const fn fits(&self, weeks: usize, width: usize, height: usize) -> bool { self.width(weeks) <= width && self.height() + CHROME_ROWS <= height } } @@ -198,7 +199,8 @@ pub struct Layout { pub x: u16, /// Row of the first cell row. pub y: u16, - /// Columns in the grid. + /// Columns in the grid that are on screen: all of them, except where pixel + /// cells were asked for in a window too narrow for the year. pub weeks: u16, /// The style the chart resolved to. pub cells: Cells, @@ -305,6 +307,7 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App) { frame.render_widget(block, area); app.footer_width = inner.width; + app.area = (inner.width, inner.height); let (lines, layout, wrap) = body(app, inner); let paragraph = Paragraph::new(lines); frame.render_widget( @@ -412,7 +415,21 @@ fn help(frame: &mut Frame<'_>, app: &App, inner: Rect) { "not reported — try --theme light".to_string(), )); } - if !app.pixels_available() && app.caps.answered { + if let Some(protocol) = app.cell_size_missing() { + // Not "try another terminal": this one can draw, and one flag says how. + lines.push(blank()); + lines.push(( + DECISION, + Line::styled( + format!(" {protocol} is here, but the cell size is not reported:"), + muted, + ), + )); + lines.push(( + DECISION, + Line::styled(" pass --cell WxH, measured in pixels.", muted), + )); + } else if !app.pixels_available() && app.caps.answered { lines.push(blank()); lines.push(( HINT, @@ -641,14 +658,26 @@ fn chart( } let grid_row = origin.1 + 1; + let x = origin.0 + (GUTTER + cells.offset()) as u16; + // Pixel cells forced into a window too narrow for them drew nothing at all: + // the image would not fit, so none was sent, and the reserved rows stayed + // blank while their days still answered the mouse (#142). Text clips at the + // right edge, and so do they now — the weeks that fit are drawn, and only + // those are days to the mouse and the painter. + let shown = match cells { + Cells::Pixels => weeks.min(usize::from( + inner.right().saturating_sub(x) / COLUMNS_PER_DAY, + )), + _ => weeks, + }; let legend_at = under .iter() .position(|(rank, _)| *rank == 5) .and_then(|index| Some((legend_x?, grid_row + (grid.len() + index) as u16))); let layout = Layout { - x: origin.0 + (GUTTER + cells.offset()) as u16, + x, y: grid_row, - weeks: weeks as u16, + weeks: shown as u16, cells, legend: legend_at, bottom: match cut { @@ -682,6 +711,14 @@ fn note( muted, )); } + // And the same when nobody asked: a terminal that speaks a protocol but + // will not say how big a cell is got text, silently (#142). + if let Some(protocol) = app.cell_size_missing() { + return Some(Line::styled( + format!("(this terminal speaks {protocol}, but not its cell size — pass --cell WxH)"), + muted, + )); + } if !cells.fits(weeks, width, height) { // Telling someone to press `d` for something smaller only helps while // something smaller exists. From `Auto` at its narrowest, `d` goes to diff --git a/tests/chart_cli.rs b/tests/chart_cli.rs index 3253273..e688326 100644 --- a/tests/chart_cli.rs +++ b/tests/chart_cli.rs @@ -165,3 +165,15 @@ fn a_written_png_reports_success_even_when_the_reader_has_gone() { ); let _ = std::fs::remove_file(&png); } + +/// `--cells` takes the names the legend uses, and a name it does not know is +/// refused with the list rather than ignored (#142). +#[test] +fn an_unknown_cell_style_is_refused_with_the_names() { + let out = chart(&["--demo", "--cells", "hexagons"]); + assert_eq!(out.status.code(), Some(2), "{}", stderr(&out)); + let text = stderr(&out); + for name in ["auto", "pixel", "rounded", "squares", "compact"] { + assert!(text.contains(name), "{name} is missing from:\n{text}"); + } +} diff --git a/tests/pixels.rs b/tests/pixels.rs index 1597b6d..89b4a28 100644 --- a/tests/pixels.rs +++ b/tests/pixels.rs @@ -716,3 +716,64 @@ fn a_year_that_has_not_happened_can_be_asked_what_it_will_look_like() -> termlen ); Ok(()) } + +#[test] +fn pixel_cells_forced_into_a_narrow_window_draw_the_weeks_that_fit() -> termlens::Result<()> { + // At 80 columns Auto takes compact cells and says pixel cells need 112; + // `d` forces them. The image would not fit, so none was sent at all: the + // grid and the legend were blank, and their days still answered the mouse + // (#142). Text clips at the edge, and so do they now: 37 weeks fit beside + // the gutter in 78 columns. + for (graphics, label) in [(Graphics::Kitty, "kitty"), (Graphics::Sixel, "sixel")] { + let mut terminal = chart(graphics, Some(CELL), (80, 34), &PREVIEW)?; + let screen = terminal.wait_frame(loaded)?; + assert_eq!(style(&screen), "compact cells", "{screen}"); + assert!(screen.graphics().is_empty()); + + terminal.send(Key::Char('d'))?; + let forced = terminal.wait_frame(|s| style(s).starts_with("pixel"))?; + let seen = forced.graphics(); + assert_eq!( + year(&seen).size(), + Some((37 * 2 * u32::from(CELL.0), 7 * u32::from(CELL.1))), + "{label}: the weeks that fit should go out: {seen:?}" + ); + + // And `d` from a style that overflows goes to the best one that fits, + // not on round the cycle to rounded, which is wider still. + terminal.send(Key::Char('d'))?; + let next = terminal.wait_frame(|s| !style(s).starts_with("pixel"))?; + assert_eq!(style(&next), "compact cells", "{label}:\n{next}"); + assert!( + !is_auto(&next), + "{label}: chosen, not left to Auto:\n{next}" + ); + terminal.send(Key::Char('q'))?; + let _ = terminal.wait_exit()?; + } + Ok(()) +} + +#[test] +fn a_terminal_that_draws_but_will_not_say_its_cell_size_is_told_the_flag() -> termlens::Result<()> { + // The kitty query answered OK and the size went unreported: text cells, + // silently, and an overlay suggesting another terminal. The `--cell WxH` + // hint was there only when `--graphics` had been forced (#142). + for (graphics, protocol) in [(Graphics::Kitty, "kitty"), (Graphics::Sixel, "sixel")] { + let mut terminal = chart(graphics, None, SIZE, &PREVIEW)?; + let screen = terminal.wait_frame(loaded)?; + assert!( + screen.contains(&format!("this terminal speaks {protocol}")) + && screen.contains("pass --cell WxH"), + "{screen}" + ); + terminal.send(Key::Char('?'))?; + let help = terminal.wait_frame(|s| s.contains("drawing with"))?; + assert!(help.contains("pass --cell WxH"), "{help}"); + assert!( + !help.contains("draw one protocol or the other"), + "this terminal does draw one:\n{help}" + ); + } + Ok(()) +} diff --git a/tests/smoke.rs b/tests/smoke.rs index cdde943..7cd0343 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -939,3 +939,19 @@ fn a_short_window_gives_up_the_statistics_before_the_way_out() -> termlens::Resu assert!(t.wait_exit()?.success(), "and q is still the way out"); Ok(()) } + +/// `--cells` names the style to start in. Auto takes rounded at this width, and +/// a font with no block sextants meant pressing `d` on every launch (#142). +#[test] +fn the_cell_style_can_be_named_on_the_command_line() -> termlens::Result<()> { + for (name, shown) in [("squares", "squares cells"), ("compact", "compact cells")] { + let args = [&PREVIEW[..], &["--cells", name]].concat(); + let mut t = chart(&args)?; + let screen = t.wait_frame(loaded)?; + assert_eq!(style(&screen), shown, "{screen}"); + assert!(!is_auto(&screen), "named, so not Auto's:\n{screen}"); + t.send(Key::Char('q'))?; + let _ = t.wait_exit()?; + } + Ok(()) +} From dcf45576401464b890a31ebb0fe9183ea88bfb24 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:42:49 +0700 Subject: [PATCH 5/6] fix: colour the editor and mossaic-glyphs the way the chart would Both always sent truecolour on the dark theme. They now use the chart's palette: the 256-colour ramp unless COLORTERM says otherwise, and the theme of the background the terminal reports, which --draw and an interactive mossaic-glyphs now ask for. The late-reply filter moves to term::LateReplies so the editor can drop a reply instead of painting it. Fixes #143 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 10 ++++ src/app.rs | 52 ++++--------------- src/bin/mossaic-art.rs | 12 ++++- src/bin/mossaic-glyphs.rs | 57 ++++++++++++++++----- src/draw.rs | 30 ++++++++--- src/main.rs | 8 +-- src/term.rs | 60 ++++++++++++++++++++-- tests/canvas_pty.rs | 105 ++++++++++++++++++++++++++++++++++++++ tests/chart_cli.rs | 25 +++++++++ 9 files changed, 286 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0814d..7728c3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,16 @@ listed under a **Changed** or **Removed** heading. pass `--cell WxH` — under the legend and in the help overlay — where before the hint appeared only for a forced `--graphics`, and the overlay suggested another terminal. +- **The art editor and `mossaic-glyphs` always sent truecolour on a dark + theme** (#143). `env -u COLORTERM mossaic-glyphs --color always` wrote + `\e[38;2;21;27;35m`, so the checker for terminals without truecolour — + Terminal.app among them — showed colours the chart never draws there. Both + now take the chart's palette: the 256-colour ramp unless `COLORTERM` says + otherwise, and the light theme on a terminal that reports a light + background. To know that, `mossaic-art --draw` and an interactive + `mossaic-glyphs` now ask the terminal as the chart does, and the editor + drops a reply that arrives late rather than painting its digits. + `draw::palette_over` and `term::LateReplies` are new for library users. ### Changed diff --git a/src/app.rs b/src/app.rs index 9920e99..efa7a10 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3,7 +3,6 @@ use std::io::{self, Write}; use std::sync::mpsc::{self, Receiver, Sender}; use std::thread; -use std::time::Instant; use chrono::{Duration, Local, NaiveDate}; use ratatui::crossterm::event::{KeyCode, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; @@ -12,7 +11,7 @@ use crate::calendar::Calendar; use crate::github; use crate::graphics::{Mark, Painter, Protocol, Ring, Scene, COLUMNS_PER_DAY}; use crate::primer::{Appearance, Palette, Season}; -use crate::term::{self, Caps}; +use crate::term::{self, Caps, LateReplies}; use crate::ui::{self, Cells, Layout}; /// Where the calendar comes from. @@ -245,9 +244,9 @@ pub struct App { /// Whether the theme is known — asked for with `--theme`, or read off the /// background the terminal reported — rather than assumed. appearance_known: bool, - /// When a late terminal reply began arriving as keys, and how many of its - /// keys have been swallowed since. See [`App::on_key`]. - reply: Option<(Instant, usize)>, + /// Keys that are a late terminal reply rather than a person. See + /// [`App::on_key`]. + replies: LateReplies, seq: u64, tx: Sender, rx: Receiver, @@ -286,7 +285,7 @@ impl App { wanted: Graphics::Auto, cell_override: None, appearance_known: false, - reply: None, + replies: LateReplies::default(), seq: 0, tx, rx, @@ -464,15 +463,12 @@ impl App { /// Handle a key press. /// - /// Not every key press is one. A terminal reply that arrives after the - /// probe stopped listening comes in on the keyboard's input, and crossterm - /// has no parser for `OSC` or `APC`: `ESC ] 11 ; rgb:0d0d/1111/1717 ESC \` - /// reaches here as Alt+`]` — "next year" — and then an `r` and two `d`s - /// (#140). So an Alt+`]`, Alt+`_` or Alt+`P` opens a reply, and what follows - /// it is swallowed up to its terminator, bounded in keys and in time so a - /// person who really typed Alt+`]` gets the keyboard back at once. + /// Not every key press is one: a terminal reply that arrives after the + /// probe stopped listening comes in on the keyboard's input, and its + /// `OSC 11` reached here as Alt+`]` — "next year" — and then an `r` and two + /// `d`s (#140). [`LateReplies`] drops those. pub fn on_key(&mut self, code: KeyCode, mods: KeyModifiers) { - if self.late_reply(code, mods) { + if self.replies.swallow(code, mods) { return; } if mods.contains(KeyModifiers::CONTROL) && matches!(code, KeyCode::Char('c' | 'd')) { @@ -489,34 +485,6 @@ impl App { } } - /// Whether this key belongs to a terminal reply rather than to a person. - fn late_reply(&mut self, code: KeyCode, mods: KeyModifiers) -> bool { - /// A reply is one write, so its keys arrive together; this is slack for - /// one split across packets on a slow link, and short enough that a - /// person never notices the keyboard was not listening. - const WINDOW: std::time::Duration = std::time::Duration::from_millis(250); - /// The longest reply asked for is a kitty error message, well under this. - const KEYS: usize = 128; - - let alt = mods.contains(KeyModifiers::ALT); - if let Some((since, keys)) = self.reply { - if since.elapsed() < WINDOW && keys < KEYS { - // `ESC \` (ST) or BEL, which crossterm reports as Ctrl+G. - let end = (alt && code == KeyCode::Char('\\')) - || (mods.contains(KeyModifiers::CONTROL) && code == KeyCode::Char('g')); - self.reply = (!end).then_some((since, keys + 1)); - return true; - } - self.reply = None; - } - // `ESC ]` opens an OSC, `ESC _` an APC, `ESC P` a DCS. - if alt && matches!(code, KeyCode::Char(']' | '_' | 'P')) { - self.reply = Some((Instant::now(), 0)); - return true; - } - false - } - fn on_key_normal(&mut self, code: KeyCode) { // The help is modal and forgiving: whatever you press to get out of it, // gets you out of it. Closing needs a redraw because the overlay is text diff --git a/src/bin/mossaic-art.rs b/src/bin/mossaic-art.rs index 00a4fbe..43a1968 100644 --- a/src/bin/mossaic-art.rs +++ b/src/bin/mossaic-art.rs @@ -445,9 +445,14 @@ fn run_editor(options: &Options, grid: &Grid, name: &str, canvas: art::Canvas) { // until 0.7.0 had neither a panic hook nor a signal handler — so both of // the exits nobody writes code for left the terminal borrowed. mossaic::restore::guard_terminal(); + // Asked, as the chart asks, so the drawing is in the colours the chart + // would show it in on this terminal (#143). Raw mode is on, so the replies + // come back unechoed; one that misses the deadline is dropped below. + let caps = mossaic::term::probe(mossaic::term::TIMEOUT); + let palette = mossaic::draw::palette_over(caps.background); + let mut replies = mossaic::term::LateReplies::default(); let mut out = std::io::stdout(); let _ = execute!(out, EnableMouseCapture); - let palette = mossaic::draw::palette(); let outcome = (|| -> std::io::Result<()> { loop { @@ -473,6 +478,11 @@ fn run_editor(options: &Options, grid: &Grid, name: &str, canvas: art::Canvas) { let origin = mossaic::draw::grid_origin(body); match event::read()? { + // A late reply's `rgb:0d0d/…` would paint level 0 here; and no + // key the editor binds uses Alt, so Alt+`0` is not `0`. + Event::Key(key) + if replies.swallow(key.code, key.modifiers) + || key.modifiers.contains(KeyModifiers::ALT) => {} Event::Key(key) if key.kind == KeyEventKind::Press => { let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); match editor.on_key(key.code, ctrl) { diff --git a/src/bin/mossaic-glyphs.rs b/src/bin/mossaic-glyphs.rs index 760a17e..9074160 100644 --- a/src/bin/mossaic-glyphs.rs +++ b/src/bin/mossaic-glyphs.rs @@ -13,9 +13,12 @@ //! None of this applies when the terminal draws pixels: `mossaic //! --capabilities` says whether yours does. +use std::io::IsTerminal; + use mossaic::cli::Args; use mossaic::primer::{Appearance, Palette, Rgb, Season}; use mossaic::Colour; +use ratatui::style::Color; const STYLES: [(&str, &str, &str); 3] = [ ( @@ -71,28 +74,31 @@ fn main() { } } + // The colours the chart would use on this terminal, since this is the check + // for the terminals that differ: truecolour only where `COLORTERM` says so, + // the 256-colour ramp otherwise, and the theme of the background the + // terminal reports. It was always the dark theme in truecolour, so on + // Terminal.app it showed colours the chart never draws there (#143). let colourful = colour.enabled(); - let palette = Palette::new(Appearance::Dark, Season::Default, true); - let paint = |glyph: &str, colour: Rgb| { + let background = colourful.then(background).flatten(); + let palette = Palette::new( + background.map_or(Appearance::Dark, Appearance::from_background), + Season::Default, + Palette::truecolor_env(), + ); + let paint = |glyph: &str, level: u8| { if !colourful { return glyph.to_string(); } - format!( - "\x1b[38;2;{};{};{}m{glyph}\x1b[0m", - colour.0, colour.1, colour.2 - ) + format!("\x1b[{}m{glyph}\x1b[0m", foreground(palette.level(level))) }; println!("Show the glyphs mossaic falls back to, to check what this terminal renders.\n"); for (name, glyph, note) in STYLES { - let legend: Vec = palette - .levels - .iter() - .map(|colour| paint(glyph, *colour)) - .collect(); + let legend: Vec = (0..5).map(|level| paint(glyph, level)).collect(); // A short stretch of cells, the way the chart draws them. - let run: Vec = (0..12) - .map(|index| paint(glyph, palette.levels[(index * 7) % 5])) + let run: Vec = (0..12u8) + .map(|index| paint(glyph, (index * 7) % 5)) .collect(); println!(" {name} {} {}", legend.join(" "), run.join(" ")); println!(" {note}"); @@ -107,3 +113,28 @@ fn main() { println!(" mossaic --cells squares start in squares, or any row above"); println!(" mossaic then press d to cycle styles"); } + +/// The background the terminal reports, when there is a terminal on both ends +/// to ask — never into a pipe, where the question would be written into +/// somebody's file and nothing would answer it — and on unix, the only place +/// the probe asks at all. +fn background() -> Option { + use ratatui::crossterm::terminal; + if !cfg!(unix) || !(std::io::stdin().is_terminal() && std::io::stdout().is_terminal()) { + return None; + } + // Raw mode, or the replies come back echoed and line-buffered. + terminal::enable_raw_mode().ok()?; + let caps = mossaic::term::probe(mossaic::term::TIMEOUT); + let _ = terminal::disable_raw_mode(); + caps.background +} + +/// The SGR parameters for a foreground colour, as [`Palette::level`] gives one. +fn foreground(colour: Color) -> String { + match colour { + Color::Rgb(r, g, b) => format!("38;2;{r};{g};{b}"), + Color::Indexed(index) => format!("38;5;{index}"), + _ => "39".to_string(), + } +} diff --git a/src/draw.rs b/src/draw.rs index bb32a5b..052e3a8 100644 --- a/src/draw.rs +++ b/src/draw.rs @@ -21,7 +21,7 @@ use ratatui::Frame; use crate::art::{self, Canvas, Grid, CANVAS_ROWS}; use crate::plural; -use crate::primer::{Appearance, Legibility, Palette, Season}; +use crate::primer::{Appearance, Legibility, Palette, Rgb, Season}; use crate::thousands; /// How deep the undo stack goes. @@ -503,11 +503,13 @@ pub fn render(frame: &mut Frame<'_>, editor: &Editor, palette: &Palette) { )]; for week in 0..editor.canvas.width() { let level = editor.canvas.at(week, row); - let rgb = palette.levels[usize::from(level).min(4)]; let outside = editor.date_at(week, row).is_none(); let here = editor.cursor == (week, row); let glyph = if outside { '·' } else { '█' }; - let mut style = Style::default().fg(Color::Rgb(rgb.0, rgb.1, rgb.2)); + // Through the palette, which falls back to its chosen 256-colour + // ramp where the terminal takes no truecolour — the same cells the + // chart draws there, rather than escapes it cannot show (#143). + let mut style = Style::default().fg(palette.level(level)); if outside { style = Style::default().fg(Color::DarkGray); } @@ -539,7 +541,6 @@ pub fn render(frame: &mut Frame<'_>, editor: &Editor, palette: &Palette) { stats.push(Line::from("")); for level in (0..=4u8).rev() { let count = histogram[usize::from(level)]; - let rgb = palette.levels[usize::from(level)]; let total: usize = histogram.iter().sum(); let bar = "█".repeat(count * 30 / total.max(1)); stats.push(Line::from(vec![ @@ -547,7 +548,7 @@ pub fn render(frame: &mut Frame<'_>, editor: &Editor, palette: &Palette) { " level {level} {count:>4} {:<6} ", plural(count, "day", "days") )), - Span::styled(bar, Style::default().fg(Color::Rgb(rgb.0, rgb.1, rgb.2))), + Span::styled(bar, Style::default().fg(palette.level(level))), Span::raw(if level == 0 { " (must stay dark)".to_string() } else { @@ -648,10 +649,25 @@ fn render_help(frame: &mut Frame<'_>, area: Rect) { ); } -/// The palette the editor paints with. +/// The palette the editor paints with, on a terminal that has not said what +/// its background is: the dark theme, in whatever colour depth `COLORTERM` +/// claims. #[must_use] pub fn palette() -> Palette { - Palette::new(Appearance::Dark, Season::Default, true) + palette_over(None) +} + +/// The palette the editor paints with, over the background the terminal +/// reported — the theme the chart would pick for it, so a drawing looks the +/// same in both. It used to be the dark theme in truecolour whatever the +/// terminal was, unlike the chart beside it (#143). +#[must_use] +pub fn palette_over(background: Option) -> Palette { + Palette::new( + background.map_or(Appearance::Dark, Appearance::from_background), + Season::Default, + Palette::truecolor_env(), + ) } /// Where a drawing goes when nobody says. diff --git a/src/main.rs b/src/main.rs index de17310..433b4b9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -76,12 +76,8 @@ const VERSION: &str = concat!("mossaic ", env!("CARGO_PKG_VERSION")); /// Redraw cadence. Also bounds how long a finished fetch waits to appear. const TICK: Duration = Duration::from_millis(80); -/// How long to wait for a terminal to say what it can do. A terminal that answers -/// takes a millisecond or two, and its device-attributes reply ends the wait; -/// this is the budget for one that never will. 250 ms was shorter than an ssh -/// round trip to the other side of the world, and an answer that misses the -/// probe is read as keystrokes (#140). -const PROBE: Duration = Duration::from_millis(500); +/// How long to wait for a terminal to say what it can do. +const PROBE: Duration = term::TIMEOUT; /// Everything the command line settles before a terminal is involved. struct Invocation { diff --git a/src/term.rs b/src/term.rs index 4aa9ffb..d4ed8f4 100644 --- a/src/term.rs +++ b/src/term.rs @@ -22,14 +22,22 @@ //! A reply the probe stops listening for does not go away: it arrives on the //! same input as the keyboard, and the event loop reads `ESC ] 11 ; rgb:0d0d/…` //! as Alt+`]` followed by an `r` and two `d`s (#140). So a terminal that has -//! begun answering is given a second window to finish, and -//! [`crate::app::App::on_key`] swallows the body of anything that still arrives -//! late. +//! begun answering is given a second window to finish, and [`LateReplies`] +//! swallows the body of anything that still arrives late. -use std::time::Duration; +use std::time::{Duration, Instant}; + +use ratatui::crossterm::event::{KeyCode, KeyModifiers}; use crate::primer::Rgb; +/// How long the binaries wait for a terminal to say what it can do. A terminal +/// that answers takes a millisecond or two, and its device-attributes reply +/// ends the wait; this is the budget for one that never will. 250 ms was +/// shorter than an ssh round trip to the other side of the world, and an answer +/// that misses the probe is read as keystrokes (#140). +pub const TIMEOUT: Duration = Duration::from_millis(500); + /// Arbitrary id for the kitty query. `a=q` only asks, so nothing is ever stored. const PROBE_ID: u32 = 7379; @@ -130,6 +138,50 @@ pub(crate) fn keep_waiting(elapsed: Duration, timeout: Duration, heard: bool) -> elapsed < timeout || (heard && elapsed < timeout.saturating_mul(2)) } +/// Tells a terminal reply that arrived as keys from a person typing. +/// +/// A reply that misses [`probe`] comes in on the keyboard's input, and crossterm +/// has no parser for `OSC` or `APC`: `ESC ] 11 ; rgb:0d0d/1111/1717 ESC \` +/// arrives as Alt+`]` and then an `r` and two `d`s (#140). So an Alt+`]`, +/// Alt+`_` or Alt+`P` opens a reply, and the keys after it are swallowed up to +/// its terminator — bounded in count and in time, so a person who really typed +/// Alt+`]` has the keyboard back at once. +#[derive(Debug, Default)] +pub struct LateReplies { + /// When the reply being swallowed began, and how many keys it has cost. + open: Option<(Instant, usize)>, +} + +impl LateReplies { + /// A reply is one write, so its keys arrive together; this is slack for one + /// split across packets on a slow link, and short enough that a person never + /// notices the keyboard was not listening. + const WINDOW: Duration = Duration::from_millis(250); + /// The longest reply asked for is a kitty error message, well under this. + const KEYS: usize = 128; + + /// Whether this key belongs to a terminal reply, and should be dropped. + pub fn swallow(&mut self, code: KeyCode, mods: KeyModifiers) -> bool { + let alt = mods.contains(KeyModifiers::ALT); + if let Some((since, keys)) = self.open { + if since.elapsed() < Self::WINDOW && keys < Self::KEYS { + // `ESC \` (ST) or BEL, which crossterm reports as Ctrl+G. + let end = (alt && code == KeyCode::Char('\\')) + || (mods.contains(KeyModifiers::CONTROL) && code == KeyCode::Char('g')); + self.open = (!end).then_some((since, keys + 1)); + return true; + } + self.open = None; + } + // `ESC ]` opens an OSC, `ESC _` an APC, `ESC P` a DCS. + if alt && matches!(code, KeyCode::Char(']' | '_' | 'P')) { + self.open = Some((Instant::now(), 0)); + return true; + } + false + } +} + /// Ask the terminal what it can do — except here, where nothing is asked. /// /// The probe reads replies from `/dev/tty` with `O_NONBLOCK`, which is a unix diff --git a/tests/canvas_pty.rs b/tests/canvas_pty.rs index c9789ab..9f564c5 100644 --- a/tests/canvas_pty.rs +++ b/tests/canvas_pty.rs @@ -491,3 +491,108 @@ fn sigint_gives_the_terminal_back() -> termlens::Result<()> { fn sighup_gives_the_terminal_back() -> termlens::Result<()> { a_signal_gives_the_terminal_back(termlens::Signal::Hup, "Hangup") } + +// ------------------------------------------------------- colour + +/// The colour of the first cell on screen holding `glyph`. +fn colour_of(screen: &Screen, glyph: char) -> Option { + let (row, col) = screen.find_by(|cell| cell.contents().contains(glyph))?; + Some(screen.cell(row, col)?.style().fg) +} + +/// The editor and the glyph checker drew the dark theme in truecolour +/// whatever the terminal was, while the chart beside them followed both +/// `COLORTERM` and the background (#143). +#[test] +fn the_editor_paints_in_the_colours_the_chart_would_use() -> termlens::Result<()> { + let editor = |truecolor: bool, background: (u8, u8, u8)| { + let mut builder = Terminal::builder() + .size(SIZE.0, SIZE.1) + .env_clear() + .current_dir(env!("CARGO_MANIFEST_DIR")) + .timeout(Duration::from_secs(20)) + .background_rgb(background.0, background.1, background.2) + .args(["--draw", "--year", "2027", "--plan", "/dev/null"]); + if truecolor { + builder = builder.env("COLORTERM", "truecolor"); + } + builder.spawn(env!("CARGO_BIN_EXE_mossaic-art")) + }; + + // No COLORTERM: the chart's 256-colour ramp, empty day first. + let mut t = editor(false, (0, 0, 0))?; + let screen = t.wait_frame(ready)?; + assert_eq!( + colour_of(&screen, '█'), + Some(termlens::Color::Indexed(236)), + "{screen}" + ); + + // A white terminal: Primer's light theme, whose empty day is #eff2f5. + let mut t = editor(true, (0xff, 0xff, 0xff))?; + let screen = t.wait_frame(ready)?; + assert_eq!( + colour_of(&screen, '█'), + Some(termlens::Color::Rgb(0xef, 0xf2, 0xf5)), + "{screen}" + ); + Ok(()) +} + +#[test] +fn the_glyph_checker_shows_the_cells_the_chart_would_draw() -> termlens::Result<()> { + let glyphs = |truecolor: bool, background: (u8, u8, u8)| -> termlens::Result { + let mut builder = Terminal::builder() + .size(SIZE.0, SIZE.1) + .env_clear() + .timeout(Duration::from_secs(20)) + .background_rgb(background.0, background.1, background.2); + if truecolor { + builder = builder.env("COLORTERM", "truecolor"); + } + let mut t = builder.spawn(env!("CARGO_BIN_EXE_mossaic-glyphs"))?; + assert!(t.wait_exit()?.success()); + Ok(t.screen()) + }; + let rounded = '\u{1FB2B}'; + assert_eq!( + colour_of(&glyphs(false, (0, 0, 0))?, rounded), + Some(termlens::Color::Indexed(236)) + ); + assert_eq!( + colour_of(&glyphs(true, (0xff, 0xff, 0xff))?, rounded), + Some(termlens::Color::Rgb(0xef, 0xf2, 0xf5)), + "a light terminal gets the light theme's empty day" + ); + Ok(()) +} + +/// The editor asks the terminal what it is now, so it inherits the chart's +/// problem with an answer that misses the probe: `11;rgb:0d0d/1111/1717` +/// arrives as keys, and here digits paint (#140, #143). +#[test] +fn a_late_reply_does_not_paint_the_canvas() -> termlens::Result<()> { + let mut t = Terminal::builder() + .size(SIZE.0, SIZE.1) + .env_clear() + .env("COLORTERM", "truecolor") + .current_dir(env!("CARGO_MANIFEST_DIR")) + .timeout(Duration::from_secs(20)) + .answer_queries(false) + .args(["--draw", "--year", "2027", "--plan", "/dev/null"]) + .spawn(env!("CARGO_BIN_EXE_mossaic-art"))?; + t.wait_frame(ready)?; + // Onto a day the year holds, so a paint would be counted. + t.send(Key::Char('l'))?; + t.send(Key::Char('l'))?; + t.wait_frame(|s| s.contains("Sun 10 Jan 2027"))?; + + t.send_str("\x1b_Gi=7379;OK\x1b\\\x1b]11;rgb:0d0d/1111/1717\x1b\\")?; + t.send(Key::Char('j'))?; + let screen = t.wait_frame(|s| s.contains("Mon 11 Jan 2027"))?; + assert!( + screen.contains("brush 4") && !screen.contains("level 1 1 day"), + "the reply painted:\n{screen}" + ); + Ok(()) +} diff --git a/tests/chart_cli.rs b/tests/chart_cli.rs index e688326..5875ebc 100644 --- a/tests/chart_cli.rs +++ b/tests/chart_cli.rs @@ -177,3 +177,28 @@ fn an_unknown_cell_style_is_refused_with_the_names() { assert!(text.contains(name), "{name} is missing from:\n{text}"); } } + +/// The glyph checker is the diagnostic for terminals without truecolour — +/// Terminal.app among them — and it sent them 24-bit escapes the chart never +/// does there, so it showed different colours from the chart it was checking +/// for (#143). +#[test] +fn the_glyph_checker_follows_colorterm_like_the_chart() { + let glyphs = |truecolor: bool| { + let mut command = Command::new(env!("CARGO_BIN_EXE_mossaic-glyphs")); + command.args(["--color", "always"]).env_remove("COLORTERM"); + if truecolor { + command.env("COLORTERM", "truecolor"); + } + let out = command.output().expect("the glyph checker runs"); + assert!(out.status.success(), "{}", stderr(&out)); + String::from_utf8_lossy(&out.stdout).into_owned() + }; + let ramp = glyphs(false); + assert!( + ramp.contains("\x1b[38;5;236m") && !ramp.contains("\x1b[38;2;"), + "without COLORTERM the levels come from the 256-colour ramp:\n{ramp:?}" + ); + // Primer's dark level 0, #151b23, where the terminal says it can. + assert!(glyphs(true).contains("\x1b[38;2;21;27;35m")); +} From 49d96be3cedabde55f9db037979e330663397547 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:54:55 +0700 Subject: [PATCH 6/6] test: compare colours in the form this terminal takes them an_unknown_background_leaves_the_text_to_the_terminal expected truecolour, which it got only where COLORTERM says so. CI's runners set none, so the palette fell back to the 256-colour ramp and the test failed on every OS and in every stress shard. It now expects each Primer value converted the way the app converts it here, and a cell colour of either kind. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- src/render_tests.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/render_tests.rs b/src/render_tests.rs index 3e7aaf4..8cc2e21 100644 --- a/src/render_tests.rs +++ b/src/render_tests.rs @@ -1195,9 +1195,11 @@ fn an_unknown_background_leaves_the_text_to_the_terminal() { "muted text stays a step down" ); // The cells keep their colours: they carry them on either background. + // Truecolour or the 256-colour ramp, by `COLORTERM` — either is a colour + // of the cell's own, which is the point. assert!(matches!( style_of(&unknown, "\u{1FB2B}").fg, - Some(Color::Rgb(..)) + Some(Color::Rgb(..) | Color::Indexed(..)) )); // And the overlay says what to do about it. let help = draw(None, None, true); @@ -1207,16 +1209,18 @@ fn an_unknown_background_leaves_the_text_to_the_terminal() { "the overlay should offer --theme light" ); - // Reported, or asked for, and the theme's own colours are back. + // Reported, or asked for, and the theme's own colours are back — in the + // form this terminal takes them, truecolour or the 256-colour ramp. + let truecolor = crate::primer::Palette::truecolor_env(); let light = draw(Some(Rgb::hex(0xffffff)), None, false); assert_eq!( style_of(&light, "octocat").fg, - Some(Color::Rgb(0x1f, 0x23, 0x28)) + Some(Rgb::hex(0x1f2328).ansi(truecolor)) ); let dark = draw(None, Some(Appearance::Dark), false); assert_eq!( style_of(&dark, "octocat").fg, - Some(Color::Rgb(0xf0, 0xf6, 0xfc)) + Some(Rgb::hex(0xf0f6fc).ansi(truecolor)) ); let help = draw(Some(Rgb::hex(0xffffff)), None, true); let text: String = help.content().iter().map(|cell| cell.symbol()).collect();