From ed2b6e986f9e62df74cf49dcfa41351fd1f32c92 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 28 Sep 2026 20:16:31 +0000 Subject: [PATCH 1/6] fix: the picker panicked when TERM named no usable terminal skim's terminal setup looks up the terminfo entry TERM names and unwraps the result. A real terminal with TERM unset (env -i, a docker exec -t that sets none), or naming an entry the machine lacks (a terminal's own name in a container without its terminfo), aborted `dl stop`, `dl rm` and `aid resume` with a panic before a row was drawn. The picker now checks the entry first. When it cannot be used, it swaps TERM for xterm-256color while skim runs, which always resolves through the term crate's ANSI fallback, and puts the old value back afterwards so the session a pick opens inherits the user's TERM. One line says what was done once the picker has given the screen back. Claude-Session: https://claude.ai/code/session_01DiHLcVgAuJAMdsS4skh48w --- CHANGELOG.md | 8 ++++++ rust/Cargo.lock | 1 + rust/Cargo.toml | 4 +++ rust/dl/Cargo.toml | 1 + rust/dl/src/select.rs | 64 +++++++++++++++++++++++++++++++++++++++++ rust/dl/tests/picker.rs | 53 +++++++++++++++++++++++++++++++--- 6 files changed, 127 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a13a4e7e..04258821 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **The picker no longer panics when `TERM` names no usable terminal.** With `TERM` unset, + or naming a terminfo entry the machine does not have, `dl`'s workspace picker (and + `aid resume`'s) aborted with a panic before it drew a row. It now draws as + `xterm-256color`, says so once the picker closes, and gives the session it opens + your own `TERM` back. + ## [0.57.0] - 2026-09-28 ### Added diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 1c171a8f..8ea75b3b 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -520,6 +520,7 @@ dependencies = [ "serde_json", "skim", "tempfile", + "term", ] [[package]] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 66752025..5ecdf507 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -36,6 +36,10 @@ toml = "0.9" toml_edit = "0.23" clap = { version = "4", features = ["derive"] } skim = "0.20" +# The terminfo lookup skim's terminal setup makes and unwraps, made first so a +# `TERM` it would panic on can be replaced (dl's `select.rs`). The version skim +# resolves, so the answer is the one skim would have reached. +term = "0.7" # flock(2) — per open file description, kernel-released — as Python's # fcntl.flock. fd-lock 4 gets its semantics from the same rustix call, but its # guard borrows the RwLock and unlocks on drop, so an owned guard would have to diff --git a/rust/dl/Cargo.toml b/rust/dl/Cargo.toml index 4f967f71..c7cc31b4 100644 --- a/rust/dl/Cargo.toml +++ b/rust/dl/Cargo.toml @@ -18,6 +18,7 @@ libc = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } skim = { workspace = true } +term = { workspace = true } # The released package ships two binaries, and a maturin bin-wheel ships the bin # targets of exactly *one* cargo package (maturin 1.14 asks cargo metadata for diff --git a/rust/dl/src/select.rs b/rust/dl/src/select.rs index 93405fc2..989ca572 100644 --- a/rust/dl/src/select.rs +++ b/rust/dl/src/select.rs @@ -755,6 +755,7 @@ fn skim_options(arity: Arity, heading: &str) -> SkimOptions { /// The rows skim was left on: empty when the picker was quit without an answer. fn run_skim(offering: &Offering, arity: Arity) -> Vec { + let _drawable = DrawableTerm::ensure(); let options = skim_options(arity, &offering.heading); let (tx, rx): (SkimItemSender, SkimItemReceiver) = unbounded(); for row in rows_of(&offering.offers) { @@ -780,6 +781,69 @@ fn run_skim(offering: &Offering, arity: Arity) -> Vec { .collect() } +/// The `TERM` the picker is drawn under when the process's own names no terminfo +/// entry. +/// +/// Any `xterm*` name resolves, from the database or else from the `term` crate's +/// built-in ANSI entry, so this cannot fail the way the name it replaces did. And +/// it is what nearly every terminal emulator answers to. +const FALLBACK_TERM: &str = "xterm-256color"; + +/// A `TERM` swapped for [`FALLBACK_TERM`] while skim runs, put back on drop. +/// +/// skim's terminal setup looks up the terminfo entry `TERM` names and unwraps the +/// answer (`Skim::run_with`), so a real terminal with `TERM` unset, or naming an +/// entry this machine lacks (a terminal's own name inside a container without it), +/// panicked before drawing a row. skim takes no terminfo of its own, so the +/// variable is the only thing that can be changed. It is put back so that the +/// session a pick goes on to open inherits the user's `TERM`, not this one. +struct DrawableTerm { + before: Option, + /// Why the swap was made, said once the picker has given the screen back. + reason: String, +} + +impl DrawableTerm { + /// `None` when skim can already draw under the `TERM` there is. + fn ensure() -> Option { + let reason = match term::terminfo::TermInfo::from_env() { + Ok(_) => return None, + Err(term::Error::TermUnset) => "TERM is unset".to_owned(), + Err(_) => format!( + "TERM={} has no usable terminfo entry here", + std::env::var_os("TERM") + .unwrap_or_default() + .to_string_lossy() + ), + }; + let before = std::env::var_os("TERM"); + // Safety: `set_var` races only a concurrent `getenv` or `setenv`. Nothing + // else runs in this process now but the runner's pipe readers, which only + // `read`, and skim has not started its threads yet. + unsafe { std::env::set_var("TERM", FALLBACK_TERM) }; + Some(Self { before, reason }) + } +} + +impl Drop for DrawableTerm { + fn drop(&mut self) { + // After skim, not before: a line written first is on the screen the picker + // covers, and it would read as the reason nothing was drawn. + eprintln!( + "{}, so the picker was drawn as {FALLBACK_TERM}.", + self.reason + ); + // Safety: as in `ensure`. `Skim::run_with` has returned, and joined its + // input thread, by the time this runs. + unsafe { + match self.before.take() { + Some(before) => std::env::set_var("TERM", before), + None => std::env::remove_var("TERM"), + } + } + } +} + /// The offers as skim items, each carrying its own position as the item index. fn rows_of(offers: &[Offer]) -> Vec> { offers diff --git a/rust/dl/tests/picker.rs b/rust/dl/tests/picker.rs index 8c50563a..5652d246 100644 --- a/rust/dl/tests/picker.rs +++ b/rust/dl/tests/picker.rs @@ -292,6 +292,37 @@ fn taking_a_row_acts_on_the_workspace_that_rows_label_names() { ); } +#[test] +fn a_terminal_whose_term_names_no_terminfo_entry_still_gets_a_picker() { + // skim unwraps its terminal setup, and that setup reads the terminfo entry + // `TERM` names. So a real terminal with `TERM` unset (`env -i`, a `docker exec + // -t` that sets none) or naming an entry this machine does not have (a + // terminal's own name, inside a container whose terminfo lacks it) aborted the + // whole command with a panic, before a single row was drawn. + for (term, said) in [ + (None, "TERM is unset"), + ( + Some("no-such-terminal"), + "TERM=no-such-terminal has no usable terminfo entry here", + ), + ] { + let (screen, calls, afterwards) = Screen::run_under( + term, + &["stop"], + "wayfinder", + |screen| screen.row_of("blooop-devlaunch").is_none(), + Dismiss::Take, + ); + + assert!( + calls.iter().any(|call| call == "stop blooop-wayfinder"), + "{term:?}: the pick never reached devpod, which was asked {calls:?}, from \ + this screen:\n{screen}\nand then said {afterwards:?}" + ); + assert!(afterwards.contains(said), "{term:?}: {afterwards:?}"); + } +} + /// The batch. `dl rm` is the verb TAB exists for, and the heading is the only thing /// in the run that says how many rows it took — devpod's own lines arrive one at a /// time and say nothing about the extent of what was asked for. @@ -402,6 +433,17 @@ impl Screen { keys: &str, settled: impl Fn(&Screen) -> bool, dismiss: Dismiss, + ) -> (Self, Vec, String) { + Self::run_under(Some("xterm-256color"), args, keys, settled, dismiss) + } + + /// [`Self::run`] with `TERM` set to `term`, or unset for `None`. + fn run_under( + term: Option<&str>, + args: &[&str], + keys: &str, + settled: impl Fn(&Screen) -> bool, + dismiss: Dismiss, ) -> (Self, Vec, String) { let world = World::new(); let pair = native_pty_system() @@ -415,7 +457,7 @@ impl Screen { let mut child = pair .slave - .spawn_command(world.command(args)) + .spawn_command(world.command(args, term)) .expect("the dl binary runs"); // The slave is the child's now: held open here, the reader below would never // see EOF. @@ -751,8 +793,9 @@ exit 0 /// /// The same scratch `HOME`/`XDG_*`/`DEVPOD_HOME` shape `tests/read_side.rs` /// builds, for the same reason: nothing here may reach the real cache or the - /// real devpod. `TERM` is the one addition, since this run has a terminal. - fn command(&self, args: &[&str]) -> CommandBuilder { + /// real devpod. `TERM` is the one addition, since this run has a terminal, and + /// `None` leaves it unset. + fn command(&self, args: &[&str], term: Option<&str>) -> CommandBuilder { let root = self.root.display().to_string(); let mut command = CommandBuilder::new(env!("CARGO_BIN_EXE_dl")); for argument in args { @@ -774,7 +817,9 @@ exit 0 command.env("XDG_CACHE_HOME", format!("{root}/cache")); command.env("XDG_CONFIG_HOME", format!("{root}/config")); command.env("DEVPOD_HOME", format!("{root}/devpod")); - command.env("TERM", "xterm-256color"); + if let Some(term) = term { + command.env("TERM", term); + } command.env("GIT_SSH_COMMAND", "false"); command.env("GIT_CONFIG_GLOBAL", "/dev/null"); command.env("GIT_CONFIG_SYSTEM", "/dev/null"); From 7bd2ee2ce714470d849928abf994c29db218e5f0 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 28 Sep 2026 20:25:40 +0000 Subject: [PATCH 2/6] fix: the picker took a terminfo entry with no cursor movement as drawable term 0.7 answers any xterm*, screen* or tmux* name that has no database entry with a built-in ANSI entry that holds colours and no cup, and skim-tuikit writes nothing for a capability an entry lacks. So TERM=xterm-kitty in a container without that entry drew a garbled picker, and with no terminfo database at all the xterm-256color fallback was that same entry while the note said the picker was drawn as xterm-256color. The choice is now made on whether the entry has cup. When neither TERM's entry nor xterm-256color's has it, skim is not started, and dl and aid resume say why and how to name the workspace instead. Claude-Session: https://claude.ai/code/session_01DiHLcVgAuJAMdsS4skh48w --- CHANGELOG.md | 10 +- rust/Cargo.toml | 5 +- rust/dl/src/commands.rs | 21 ++++- rust/dl/src/select.rs | 197 +++++++++++++++++++++++++++++++++++----- rust/dl/tests/picker.rs | 10 +- 5 files changed, 210 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04258821..89936c09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The picker no longer panics when `TERM` names no usable terminal.** With `TERM` unset, or naming a terminfo entry the machine does not have, `dl`'s workspace picker (and - `aid resume`'s) aborted with a panic before it drew a row. It now draws as - `xterm-256color`, says so once the picker closes, and gives the session it opens - your own `TERM` back. + `aid resume`'s) aborted with a panic before it drew a row. A name like `xterm-kitty` + with no entry behind it did not panic but drew a garbled picker, because the entry it + got could not move the cursor. Now any `TERM` whose entry cannot move the cursor is + drawn as `xterm-256color`, the picker says so once it closes, and the session it opens + gets your own `TERM` back. When `xterm-256color` cannot move the cursor here either, + the picker is not started, and the command says why and how to name the workspace + instead. ## [0.57.0] - 2026-09-28 diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 5ecdf507..28e04648 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -37,8 +37,9 @@ toml_edit = "0.23" clap = { version = "4", features = ["derive"] } skim = "0.20" # The terminfo lookup skim's terminal setup makes and unwraps, made first so a -# `TERM` it would panic on can be replaced (dl's `select.rs`). The version skim -# resolves, so the answer is the one skim would have reached. +# `TERM` it would panic on, or draw nothing with, can be replaced (dl's +# `select.rs`). The version skim resolves, so the answer is the one skim would +# have reached. term = "0.7" # flock(2) — per open file description, kernel-released — as Python's # fcntl.flock. fd-lock 4 gets its semantics from the same rustix call, but its diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index 04290c07..59faf293 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -1679,6 +1679,17 @@ fn render_select<'r>( println!("{}", select::invitation(arity)); no_pick() } + select::Pick::Undrawable(reason) => { + let line = match &verb { + Verb::Attach { .. } => "dl ".to_owned(), + Verb::Run(..) => "dl -- ".to_owned(), + verb => format!("dl {}", verb.word()), + }; + eprintln!( + "{reason}, so the picker cannot be drawn. Name the workspace instead: {line}" + ); + no_pick() + } } } @@ -1691,7 +1702,8 @@ fn render_select<'r>( /// grammar. An empty list keeps dl's sentence, which says how to make a workspace. /// A run with no terminal gets one of its own in place of the invitation, which /// would ask for a pick nobody can make: it says a terminal is missing and how to -/// name the workspace instead. +/// name the workspace instead. So does a terminal that no terminfo entry here can +/// draw on. pub(crate) fn pick_one(runner: &dyn Runner) -> Result { // The picker reads each row's clone under the cache for its columns. With no // cache there is nothing to draw, and the refusal is every other command's. @@ -1719,6 +1731,13 @@ pub(crate) fn pick_one(runner: &dyn Runner) -> Result { ); Err(Ending::Refused) } + select::Pick::Undrawable(reason) => { + eprintln!( + "{reason}, so aid resume cannot draw its picker. Name the workspace instead: \ + aid resume " + ); + Err(Ending::Refused) + } } } diff --git a/rust/dl/src/select.rs b/rust/dl/src/select.rs index 989ca572..0bc61f4e 100644 --- a/rust/dl/src/select.rs +++ b/rust/dl/src/select.rs @@ -98,6 +98,7 @@ use std::borrow::Cow; use std::collections::HashMap; +use std::ffi::{OsStr, OsString}; use std::path::Path; use std::sync::Arc; @@ -106,6 +107,7 @@ use devlaunch_core::domain::workspace_id::WorkspaceId; use devlaunch_core::domain::workspace_state::NonEmpty; use devlaunch_core::flows::listing::{head_branch_of, owner_of, repo_of}; use skim::prelude::*; +use term::terminfo::TermInfo; /// One row the picker offers, and the workspace it stands for. /// @@ -631,11 +633,12 @@ pub(crate) struct Chosen { /// What the picker settled. /// -/// Four arms where Python has `Optional[str]`, because its `None` covers four -/// different situations and two of them have something to say: an empty list is -/// reported (`No workspaces found …`), and a run with no terminal cannot draw a -/// picker at all. All three of the non-answers end the same way — Python prints the -/// help and exits 1 — but which one happened is the caller's to say. +/// Five arms where Python has `Optional[str]`, because its `None` covers several +/// different situations and some of them have something to say: an empty list is +/// reported (`No workspaces found …`), and a run with no terminal, or with no +/// terminfo entry to draw on it with, cannot draw a picker at all. All four of the +/// non-answers end the same way — Python prints the help and exits 1 — but which +/// one happened is the caller's to say. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Pick { /// These workspaces, in the order skim handed the rows back. One entry always @@ -651,6 +654,10 @@ pub(crate) enum Pick { /// pipe. Python's fzf said `inappropriate ioctl for device` and answered /// nothing; this answers the same nothing without the subprocess. NoTerminal, + /// There is a terminal, but no terminfo entry here that skim can draw on it + /// with, under `TERM` or under [`FALLBACK_TERM`]. The clause says why, for the + /// caller to finish with how to name the workspace instead. + Undrawable(String), } /// Offer these workspaces and wait for one — or, under [`Arity::Several`], any @@ -666,7 +673,18 @@ pub(crate) fn pick(workspaces: &[Workspace], arity: Arity, cache_dir: &Path) -> if !a_terminal_exists() { return Pick::NoTerminal; } + let name = std::env::var_os("TERM"); + let swapped = match plan( + TermInfo::from_env(), + TermInfo::from_name(FALLBACK_TERM), + name.as_deref(), + ) { + Plan::Keep => None, + Plan::Swap { reason } => Some(DrawableTerm::swap(reason)), + Plan::Refuse { reason } => return Pick::Undrawable(reason), + }; let rows = run_skim(&offering, arity); + drop(swapped); chosen(&offering.offers, rows) } @@ -755,7 +773,6 @@ fn skim_options(arity: Arity, heading: &str) -> SkimOptions { /// The rows skim was left on: empty when the picker was quit without an answer. fn run_skim(offering: &Offering, arity: Arity) -> Vec { - let _drawable = DrawableTerm::ensure(); let options = skim_options(arity, &offering.heading); let (tx, rx): (SkimItemSender, SkimItemReceiver) = unbounded(); for row in rows_of(&offering.offers) { @@ -782,13 +799,74 @@ fn run_skim(offering: &Offering, arity: Arity) -> Vec { } /// The `TERM` the picker is drawn under when the process's own names no terminfo -/// entry. +/// entry skim can draw with. /// -/// Any `xterm*` name resolves, from the database or else from the `term` crate's -/// built-in ANSI entry, so this cannot fail the way the name it replaces did. And -/// it is what nearly every terminal emulator answers to. +/// It is what nearly every terminal emulator answers to, and nearly every terminfo +/// database carries it. Not every one: with no database at all the `term` crate +/// still answers for this name, with a built-in entry that cannot move the cursor, +/// so it is held to [`drawable`] like any other name ([`plan`]). const FALLBACK_TERM: &str = "xterm-256color"; +/// Whether skim can draw the picker with this terminfo entry. +/// +/// skim-tuikit writes every control sequence through the entry, and writes nothing +/// for a capability the entry lacks (`Output::write_cap`), so a missing one is not +/// an error but a screen that comes out wrong. `cup` is the one it cannot do +/// without, since every row is placed with it. The `term` crate's built-in ANSI +/// entry, which `TermInfo::from_name` returns for any `xterm*`, `screen*` or +/// `tmux*` name that has no database entry behind it, holds colours and bold and no +/// `cup`. The alternate screen (`smcup`) is not needed: without it skim draws over +/// the visible screen, which is still a picker. +fn drawable(entry: &TermInfo) -> bool { + entry.strings.contains_key("cup") +} + +/// What to do about `TERM` before skim runs. +#[derive(Debug, PartialEq, Eq)] +enum Plan { + /// The entry `TERM` names can draw the picker. + Keep, + /// Draw under [`FALLBACK_TERM`], and give `reason` once the picker has closed. + Swap { reason: String }, + /// Neither the entry `TERM` names nor [`FALLBACK_TERM`]'s can draw the picker, + /// and `reason` says so, for the caller to finish with what to do instead. + Refuse { reason: String }, +} + +/// The [`Plan`] for two terminfo lookups: `current` for the `TERM` there is, which +/// `name` is, and `fallback` for [`FALLBACK_TERM`]. +fn plan( + current: term::Result, + fallback: term::Result, + name: Option<&OsStr>, +) -> Plan { + if current.as_ref().is_ok_and(drawable) { + return Plan::Keep; + } + let unset = matches!(current, Err(term::Error::TermUnset)); + let missing = format!( + "TERM={} names no terminfo entry here that can move the cursor", + name.unwrap_or_default().to_string_lossy() + ); + if fallback.as_ref().is_ok_and(drawable) { + let reason = if unset { + "TERM is unset".to_owned() + } else { + missing + }; + return Plan::Swap { reason }; + } + let reason = if unset { + format!( + "TERM is unset, and {FALLBACK_TERM} names no terminfo entry here that can move \ + the cursor" + ) + } else { + format!("{missing}, and neither does {FALLBACK_TERM}") + }; + Plan::Refuse { reason } +} + /// A `TERM` swapped for [`FALLBACK_TERM`] while skim runs, put back on drop. /// /// skim's terminal setup looks up the terminfo entry `TERM` names and unwraps the @@ -798,30 +876,20 @@ const FALLBACK_TERM: &str = "xterm-256color"; /// variable is the only thing that can be changed. It is put back so that the /// session a pick goes on to open inherits the user's `TERM`, not this one. struct DrawableTerm { - before: Option, + before: Option, /// Why the swap was made, said once the picker has given the screen back. reason: String, } impl DrawableTerm { - /// `None` when skim can already draw under the `TERM` there is. - fn ensure() -> Option { - let reason = match term::terminfo::TermInfo::from_env() { - Ok(_) => return None, - Err(term::Error::TermUnset) => "TERM is unset".to_owned(), - Err(_) => format!( - "TERM={} has no usable terminfo entry here", - std::env::var_os("TERM") - .unwrap_or_default() - .to_string_lossy() - ), - }; + /// Put [`FALLBACK_TERM`] in `TERM` until this is dropped. + fn swap(reason: String) -> Self { let before = std::env::var_os("TERM"); // Safety: `set_var` races only a concurrent `getenv` or `setenv`. Nothing // else runs in this process now but the runner's pipe readers, which only // `read`, and skim has not started its threads yet. unsafe { std::env::set_var("TERM", FALLBACK_TERM) }; - Some(Self { before, reason }) + Self { before, reason } } } @@ -833,7 +901,7 @@ impl Drop for DrawableTerm { "{}, so the picker was drawn as {FALLBACK_TERM}.", self.reason ); - // Safety: as in `ensure`. `Skim::run_with` has returned, and joined its + // Safety: as in `swap`. `Skim::run_with` has returned, and joined its // input thread, by the time this runs. unsafe { match self.before.take() { @@ -2060,4 +2128,83 @@ mod tests { "the short branch ends its line rather than paying the column's width" ); } + + /// A terminfo entry holding exactly these string capabilities. + fn entry(capabilities: &[&'static str]) -> TermInfo { + TermInfo { + names: vec!["test".to_owned()], + bools: HashMap::new(), + numbers: HashMap::new(), + strings: capabilities + .iter() + .map(|capability| (*capability, b"\x1b[".to_vec())) + .collect(), + } + } + + /// A database entry, as far as [`drawable`] reads one. + fn full() -> TermInfo { + entry(&[ + "cup", "smcup", "rmcup", "clear", "el", "ed", "sgr0", "setaf", + ]) + } + + /// What `term` 0.7 answers for an `is_ansi` name with no database entry behind it + /// (`TermInfo::from_name`). + fn ansi_only() -> TermInfo { + entry(&["sgr0", "bold", "setaf", "setab"]) + } + + #[test] + fn an_entry_that_can_move_the_cursor_is_kept() { + assert_eq!( + plan(Ok(full()), Ok(full()), Some(OsStr::new("xterm-kitty"))), + Plan::Keep + ); + } + + #[test] + fn an_entry_that_cannot_move_the_cursor_is_swapped_for_one_that_can() { + assert_eq!( + plan(Ok(ansi_only()), Ok(full()), Some(OsStr::new("xterm-kitty"))), + Plan::Swap { + reason: "TERM=xterm-kitty names no terminfo entry here that can move the cursor" + .to_owned() + } + ); + } + + #[test] + fn an_unset_term_is_swapped_for_one_that_can_move_the_cursor() { + assert_eq!( + plan(Err(term::Error::TermUnset), Ok(full()), None), + Plan::Swap { + reason: "TERM is unset".to_owned() + } + ); + } + + #[test] + fn a_fallback_that_cannot_move_the_cursor_is_refused_rather_than_named() { + assert_eq!( + plan( + Ok(ansi_only()), + Ok(ansi_only()), + Some(OsStr::new("xterm-kitty")) + ), + Plan::Refuse { + reason: "TERM=xterm-kitty names no terminfo entry here that can move the cursor, \ + and neither does xterm-256color" + .to_owned() + } + ); + assert_eq!( + plan(Err(term::Error::TermUnset), Ok(ansi_only()), None), + Plan::Refuse { + reason: "TERM is unset, and xterm-256color names no terminfo entry here that can \ + move the cursor" + .to_owned() + } + ); + } } diff --git a/rust/dl/tests/picker.rs b/rust/dl/tests/picker.rs index 5652d246..d64408dc 100644 --- a/rust/dl/tests/picker.rs +++ b/rust/dl/tests/picker.rs @@ -298,12 +298,18 @@ fn a_terminal_whose_term_names_no_terminfo_entry_still_gets_a_picker() { // `TERM` names. So a real terminal with `TERM` unset (`env -i`, a `docker exec // -t` that sets none) or naming an entry this machine does not have (a // terminal's own name, inside a container whose terminfo lacks it) aborted the - // whole command with a panic, before a single row was drawn. + // whole command with a panic, before a single row was drawn. A name the `term` + // crate takes for ANSI (`xterm*`, `tmux*`, `screen*`) did not panic: it got a + // built-in entry with colours and no cursor movement, and a garbled picker. for (term, said) in [ (None, "TERM is unset"), ( Some("no-such-terminal"), - "TERM=no-such-terminal has no usable terminfo entry here", + "TERM=no-such-terminal names no terminfo entry here that can move the cursor", + ), + ( + Some("xterm-no-such-entry"), + "TERM=xterm-no-such-entry names no terminfo entry here that can move the cursor", ), ] { let (screen, calls, afterwards) = Screen::run_under( From 0ce672f953bd3768fca3af33728f9a8717e3b0d2 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 28 Sep 2026 20:27:04 +0000 Subject: [PATCH 3/6] fix: the note said TERM is unset when TERM held bytes that are not UTF-8 TermInfo::from_env reads TERM with env::var, which answers nothing for bytes that are not UTF-8, so its TermUnset came back for a TERM that was set, and the note called it unset while the restore put the bytes back. TERM is now read once, with var_os, into a Was that both the note and the restore use. from_env only decides whether the entry can draw. Claude-Session: https://claude.ai/code/session_01DiHLcVgAuJAMdsS4skh48w --- rust/dl/src/select.rs | 126 ++++++++++++++++++++++++++---------------- 1 file changed, 78 insertions(+), 48 deletions(-) diff --git a/rust/dl/src/select.rs b/rust/dl/src/select.rs index 0bc61f4e..df29efca 100644 --- a/rust/dl/src/select.rs +++ b/rust/dl/src/select.rs @@ -98,7 +98,7 @@ use std::borrow::Cow; use std::collections::HashMap; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::path::Path; use std::sync::Arc; @@ -673,14 +673,13 @@ pub(crate) fn pick(workspaces: &[Workspace], arity: Arity, cache_dir: &Path) -> if !a_terminal_exists() { return Pick::NoTerminal; } - let name = std::env::var_os("TERM"); let swapped = match plan( TermInfo::from_env(), TermInfo::from_name(FALLBACK_TERM), - name.as_deref(), + Was::read(), ) { Plan::Keep => None, - Plan::Swap { reason } => Some(DrawableTerm::swap(reason)), + Plan::Swap { was, reason } => Some(DrawableTerm::swap(was, reason)), Plan::Refuse { reason } => return Pick::Undrawable(reason), }; let rows = run_skim(&offering, arity); @@ -821,48 +820,60 @@ fn drawable(entry: &TermInfo) -> bool { entry.strings.contains_key("cup") } +/// What `TERM` held when the picker was asked for. +/// +/// Read once, with `var_os`, and used both for what the note says and for what is +/// put back. `TermInfo::from_env` reads the variable with `env::var` instead, which +/// answers nothing for bytes that are not UTF-8, so its `TermUnset` cannot be +/// trusted to mean the variable is unset. +#[derive(Clone, Debug, PartialEq, Eq)] +enum Was { + Unset, + Set(OsString), +} + +impl Was { + fn read() -> Self { + std::env::var_os("TERM").map_or(Self::Unset, Self::Set) + } +} + /// What to do about `TERM` before skim runs. #[derive(Debug, PartialEq, Eq)] enum Plan { /// The entry `TERM` names can draw the picker. Keep, - /// Draw under [`FALLBACK_TERM`], and give `reason` once the picker has closed. - Swap { reason: String }, + /// Draw under [`FALLBACK_TERM`], give `reason` once the picker has closed, and + /// put back what `TERM` `was`. + Swap { was: Was, reason: String }, /// Neither the entry `TERM` names nor [`FALLBACK_TERM`]'s can draw the picker, /// and `reason` says so, for the caller to finish with what to do instead. Refuse { reason: String }, } /// The [`Plan`] for two terminfo lookups: `current` for the `TERM` there is, which -/// `name` is, and `fallback` for [`FALLBACK_TERM`]. -fn plan( - current: term::Result, - fallback: term::Result, - name: Option<&OsStr>, -) -> Plan { +/// held what it `was`, and `fallback` for [`FALLBACK_TERM`]. +fn plan(current: term::Result, fallback: term::Result, was: Was) -> Plan { if current.as_ref().is_ok_and(drawable) { return Plan::Keep; } - let unset = matches!(current, Err(term::Error::TermUnset)); - let missing = format!( - "TERM={} names no terminfo entry here that can move the cursor", - name.unwrap_or_default().to_string_lossy() - ); + let missing = match &was { + Was::Unset => None, + Was::Set(name) => Some(format!( + "TERM={} names no terminfo entry here that can move the cursor", + name.to_string_lossy() + )), + }; if fallback.as_ref().is_ok_and(drawable) { - let reason = if unset { - "TERM is unset".to_owned() - } else { - missing - }; - return Plan::Swap { reason }; + let reason = missing.unwrap_or_else(|| "TERM is unset".to_owned()); + return Plan::Swap { was, reason }; } - let reason = if unset { - format!( + let reason = match missing { + None => format!( "TERM is unset, and {FALLBACK_TERM} names no terminfo entry here that can move \ the cursor" - ) - } else { - format!("{missing}, and neither does {FALLBACK_TERM}") + ), + Some(missing) => format!("{missing}, and neither does {FALLBACK_TERM}"), }; Plan::Refuse { reason } } @@ -876,20 +887,20 @@ fn plan( /// variable is the only thing that can be changed. It is put back so that the /// session a pick goes on to open inherits the user's `TERM`, not this one. struct DrawableTerm { - before: Option, + was: Was, /// Why the swap was made, said once the picker has given the screen back. reason: String, } impl DrawableTerm { - /// Put [`FALLBACK_TERM`] in `TERM` until this is dropped. - fn swap(reason: String) -> Self { - let before = std::env::var_os("TERM"); + /// Put [`FALLBACK_TERM`] in `TERM` until this is dropped, and then what it + /// `was`. + fn swap(was: Was, reason: String) -> Self { // Safety: `set_var` races only a concurrent `getenv` or `setenv`. Nothing // else runs in this process now but the runner's pipe readers, which only // `read`, and skim has not started its threads yet. unsafe { std::env::set_var("TERM", FALLBACK_TERM) }; - Self { before, reason } + Self { was, reason } } } @@ -904,9 +915,9 @@ impl Drop for DrawableTerm { // Safety: as in `swap`. `Skim::run_with` has returned, and joined its // input thread, by the time this runs. unsafe { - match self.before.take() { - Some(before) => std::env::set_var("TERM", before), - None => std::env::remove_var("TERM"), + match &self.was { + Was::Set(before) => std::env::set_var("TERM", before), + Was::Unset => std::env::remove_var("TERM"), } } } @@ -2155,19 +2166,22 @@ mod tests { entry(&["sgr0", "bold", "setaf", "setab"]) } + /// `TERM` set to `name`. + fn set(name: &str) -> Was { + Was::Set(name.into()) + } + #[test] fn an_entry_that_can_move_the_cursor_is_kept() { - assert_eq!( - plan(Ok(full()), Ok(full()), Some(OsStr::new("xterm-kitty"))), - Plan::Keep - ); + assert_eq!(plan(Ok(full()), Ok(full()), set("xterm-kitty")), Plan::Keep); } #[test] fn an_entry_that_cannot_move_the_cursor_is_swapped_for_one_that_can() { assert_eq!( - plan(Ok(ansi_only()), Ok(full()), Some(OsStr::new("xterm-kitty"))), + plan(Ok(ansi_only()), Ok(full()), set("xterm-kitty")), Plan::Swap { + was: set("xterm-kitty"), reason: "TERM=xterm-kitty names no terminfo entry here that can move the cursor" .to_owned() } @@ -2177,8 +2191,9 @@ mod tests { #[test] fn an_unset_term_is_swapped_for_one_that_can_move_the_cursor() { assert_eq!( - plan(Err(term::Error::TermUnset), Ok(full()), None), + plan(Err(term::Error::TermUnset), Ok(full()), Was::Unset), Plan::Swap { + was: Was::Unset, reason: "TERM is unset".to_owned() } ); @@ -2187,11 +2202,7 @@ mod tests { #[test] fn a_fallback_that_cannot_move_the_cursor_is_refused_rather_than_named() { assert_eq!( - plan( - Ok(ansi_only()), - Ok(ansi_only()), - Some(OsStr::new("xterm-kitty")) - ), + plan(Ok(ansi_only()), Ok(ansi_only()), set("xterm-kitty")), Plan::Refuse { reason: "TERM=xterm-kitty names no terminfo entry here that can move the cursor, \ and neither does xterm-256color" @@ -2199,7 +2210,7 @@ mod tests { } ); assert_eq!( - plan(Err(term::Error::TermUnset), Ok(ansi_only()), None), + plan(Err(term::Error::TermUnset), Ok(ansi_only()), Was::Unset), Plan::Refuse { reason: "TERM is unset, and xterm-256color names no terminfo entry here that can \ move the cursor" @@ -2207,4 +2218,23 @@ mod tests { } ); } + + /// `TermInfo::from_env` reads `TERM` with `env::var`, which answers nothing for + /// bytes that are not UTF-8, so the lookup says `TermUnset` for a `TERM` that is + /// set. The note is about the variable, and the variable is set; so is what is + /// put back. + #[test] + fn a_term_that_is_not_utf8_is_named_rather_than_called_unset() { + use std::os::unix::ffi::OsStringExt as _; + + let was = Was::Set(OsString::from_vec(b"xterm-\xff".to_vec())); + assert_eq!( + plan(Err(term::Error::TermUnset), Ok(full()), was.clone()), + Plan::Swap { + was, + reason: "TERM=xterm-\u{fffd} names no terminfo entry here that can move the cursor" + .to_owned() + } + ); + } } From 432dccc74af8e7f360f6b655d76d661bc1c9eab0 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 28 Sep 2026 20:28:40 +0000 Subject: [PATCH 4/6] test: the session a pick opens gets the user's TERM back Deleting the restore in DrawableTerm's drop left every test passing, because the fake devpod logged its arguments and nothing of its environment. It now also logs the TERM each call other than list ran under, to a second file so the exact matches on the call log are untouched, and the TERM test asserts the stop ran under the user's own TERM and nothing ran under xterm-256color. With the restore deleted it fails. Also: a quit picker under a swapped TERM still gives the note and stops nothing, and a TERM that can draw is not swapped. Claude-Session: https://claude.ai/code/session_01DiHLcVgAuJAMdsS4skh48w --- rust/dl/tests/picker.rs | 82 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/rust/dl/tests/picker.rs b/rust/dl/tests/picker.rs index d64408dc..c0323d48 100644 --- a/rust/dl/tests/picker.rs +++ b/rust/dl/tests/picker.rs @@ -290,6 +290,10 @@ fn taking_a_row_acts_on_the_workspace_that_rows_label_names() { said.contains("Picked blooop | blooop-wayfinder -> blooop-wayfinder"), "a pick that named nothing: {said:?}" ); + assert!( + !said.contains("so the picker was drawn as"), + "a TERM that can draw the picker was swapped: {said:?}" + ); } #[test] @@ -312,7 +316,7 @@ fn a_terminal_whose_term_names_no_terminfo_entry_still_gets_a_picker() { "TERM=xterm-no-such-entry names no terminfo entry here that can move the cursor", ), ] { - let (screen, calls, afterwards) = Screen::run_under( + let (screen, calls, afterwards, terms) = Screen::run_under( term, &["stop"], "wayfinder", @@ -326,9 +330,41 @@ fn a_terminal_whose_term_names_no_terminfo_entry_still_gets_a_picker() { this screen:\n{screen}\nand then said {afterwards:?}" ); assert!(afterwards.contains(said), "{term:?}: {afterwards:?}"); + // The picker was drawn as xterm-256color, and the workspace it picked is + // acted on under the user's own `TERM`: a session opened under the + // borrowed one would draw for a terminal the user does not have. + let own = match term { + Some(term) => format!("TERM={term} stop blooop-wayfinder"), + None => "TERM unset stop blooop-wayfinder".to_owned(), + }; + assert!( + terms.contains(&own), + "{term:?}: the stop did not run under the user's TERM: {terms:?}" + ); + assert!( + !terms + .iter() + .any(|call| call.starts_with("TERM=xterm-256color ")), + "{term:?}: a devpod call ran under the borrowed TERM: {terms:?}" + ); } } +#[test] +fn a_picker_quit_under_a_swapped_term_still_says_why_and_acts_on_nothing() { + let (screen, calls, afterwards, _) = + Screen::run_under(None, &["stop"], "", |_| true, Dismiss::Quit); + + assert!( + afterwards.contains("TERM is unset, so the picker was drawn as xterm-256color."), + "the note was not given: {afterwards:?}\nfrom this screen:\n{screen}" + ); + assert!( + !calls.iter().any(|call| call.starts_with("stop")), + "a quit picker stopped a workspace: {calls:?}" + ); +} + /// The batch. `dl rm` is the verb TAB exists for, and the heading is the only thing /// in the run that says how many rows it took — devpod's own lines arrive one at a /// time and say nothing about the extent of what was asked for. @@ -440,17 +476,20 @@ impl Screen { settled: impl Fn(&Screen) -> bool, dismiss: Dismiss, ) -> (Self, Vec, String) { - Self::run_under(Some("xterm-256color"), args, keys, settled, dismiss) + let (screen, calls, afterwards, _) = + Self::run_under(Some("xterm-256color"), args, keys, settled, dismiss); + (screen, calls, afterwards) } - /// [`Self::run`] with `TERM` set to `term`, or unset for `None`. + /// [`Self::run`] with `TERM` set to `term`, or unset for `None`, and one more + /// answer: [`World::devpod_terms`], the `TERM` each devpod call ran under. fn run_under( term: Option<&str>, args: &[&str], keys: &str, settled: impl Fn(&Screen) -> bool, dismiss: Dismiss, - ) -> (Self, Vec, String) { + ) -> (Self, Vec, String, Vec) { let world = World::new(); let pair = native_pty_system() .openpty(PtySize { @@ -534,7 +573,12 @@ impl Screen { let _ = collecting_bytes.join(); let afterwards = Self::afterwards(&drawn.lock().expect("the collected bytes").clone()); - (screen, world.devpod_calls(), afterwards) + ( + screen, + world.devpod_calls(), + afterwards, + world.devpod_terms(), + ) } /// The bytes a terminal received, as the grid it would be showing. @@ -748,6 +792,15 @@ impl World { r#"#!/bin/sh # Every call is recorded, so a test can ask which workspace a pick reached. echo "$@" >> "$DL_TEST_DEVPOD_LOG" +# And, apart from the listing, the TERM each call ran under, so a test can ask +# whether a session a pick opened got the user's back. +if [ "$1" != "list" ]; then + if [ "${TERM+set}" = set ]; then + echo "TERM=$TERM $*" >> "$DL_TEST_DEVPOD_TERM_LOG" + else + echo "TERM unset $*" >> "$DL_TEST_DEVPOD_TERM_LOG" + fi +fi if [ "$1" = "list" ]; then cat <<'JSON' [{"id": "blooop-devlaunch", "source": {"gitRepository": "https://github.com/blooop/devlaunch.git"}, @@ -795,6 +848,21 @@ exit 0 .collect() } + /// Where the fake devpod writes the `TERM` each call other than `list` ran + /// under: `TERM= `, or `TERM unset `. + fn devpod_term_log(&self) -> std::path::PathBuf { + self.root.join("devpod-terms") + } + + /// Every line of [`Self::devpod_term_log`], in the order the calls came. + fn devpod_terms(&self) -> Vec { + std::fs::read_to_string(self.devpod_term_log()) + .unwrap_or_default() + .lines() + .map(str::to_owned) + .collect() + } + /// `dl ` against this world, environment and all. /// /// The same scratch `HOME`/`XDG_*`/`DEVPOD_HOME` shape `tests/read_side.rs` @@ -819,6 +887,10 @@ exit 0 "DL_TEST_DEVPOD_LOG", self.devpod_log().display().to_string(), ); + command.env( + "DL_TEST_DEVPOD_TERM_LOG", + self.devpod_term_log().display().to_string(), + ); command.env("HOME", format!("{root}/home")); command.env("XDG_CACHE_HOME", format!("{root}/cache")); command.env("XDG_CONFIG_HOME", format!("{root}/config")); From c9ac88da6f058050083d260f2c84200be138c1a9 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 28 Sep 2026 21:20:07 +0000 Subject: [PATCH 5/6] test: the picker's refusal when no terminfo entry can draw The Pick::Undrawable arms in render_select and pick_one had no test. They are reachable: with TERMINFO_DIRS set, the term crate searches only that list, so an empty directory leaves even the xterm-256color fallback with the built-in ANSI entry, which has no cup. picker.rs runs `dl stop` and `dl` under that environment and checks the refusal line each verb gets, exit 1, no alternate screen, and no devpod call past the listing. interactive.rs does the same for `aid resume`. Both wait for the exit against a deadline, so a picker that opens anyway fails the test in place of hanging it. Claude-Session: https://claude.ai/code/session_01DiHLcVgAuJAMdsS4skh48w --- rust/aid/tests/interactive.rs | 52 +++++++++++++++++++++ rust/dl/tests/picker.rs | 88 +++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) diff --git a/rust/aid/tests/interactive.rs b/rust/aid/tests/interactive.rs index 413a2c53..e3ccd44b 100644 --- a/rust/aid/tests/interactive.rs +++ b/rust/aid/tests/interactive.rs @@ -474,6 +474,58 @@ fn a_resume_with_no_workspace_resumes_the_row_the_picker_took() { ); } +#[test] +fn a_resume_with_no_workspace_on_a_terminal_no_entry_can_draw_on_says_how_to_name_one() { + // `TERMINFO_DIRS` is the whole of the terminfo search when it is set, so an + // empty one leaves no entry, the fallback's included, that can move the + // cursor. No picker can be drawn, and aid says how to name the workspace + // instead of opening anything. + let world = World::with(&["--warm"]); + let nothing = world.root.join("terminfo"); + std::fs::create_dir_all(¬hing).expect("an empty terminfo directory"); + let mut session = PtyAid::spawn( + &world, + &["resume"], + &[ + ("TERM", "xterm-no-such-entry"), + ("TERMINFO_DIRS", ¬hing.display().to_string()), + ], + ); + let seen = Arc::clone(&session.seen); + // Bounded: a picker that opened anyway would wait on this pty for ever. + let mut code = None; + wait_for(|| { + code = session.child.try_wait().expect("aid's status"); + code.is_some() + }); + let _ = session.child.kill(); + assert_eq!(code.map(|status| status.exit_code()), Some(1)); + + // The reader thread may still hold the last bytes, so the line is waited for. + let said = || String::from_utf8_lossy(&seen.lock().expect("the pty buffer")).into_owned(); + assert!( + wait_for(|| said().contains( + "so aid resume cannot draw its picker. Name the workspace instead: \ + aid resume " + )), + "the refusal was not given; the pty said:\n{:?}", + said() + ); + assert!( + !said().contains("\x1b[?1049h"), + "a picker was opened anyway: {:?}", + said() + ); + assert!( + !world + .devpod_calls() + .iter() + .any(|call| call.starts_with("devpod ssh")), + "a refused picker opened a session: {:?}", + world.devpod_calls() + ); +} + #[test] fn the_boot_runs_while_the_prompt_is_still_being_typed() { // The overlap itself: a stopped workspace's `devpod up` is on the shim's log diff --git a/rust/dl/tests/picker.rs b/rust/dl/tests/picker.rs index c0323d48..f2bdf464 100644 --- a/rust/dl/tests/picker.rs +++ b/rust/dl/tests/picker.rs @@ -365,6 +365,94 @@ fn a_picker_quit_under_a_swapped_term_still_says_why_and_acts_on_nothing() { ); } +#[test] +fn a_terminal_no_terminfo_entry_here_can_draw_on_is_refused_with_the_line_to_type() { + // The last resort gone as well. `TERMINFO_DIRS` is the whole of the `term` + // crate's search when it is set, so an empty one leaves every name, the + // fallback included, with the built-in ANSI entry and its missing `cup`. No + // picker can be drawn then, and the run says so and names the line that needs + // none, for the verb that was asked for. + for (args, line) in [ + (vec!["stop"], "dl stop"), + (vec![], "dl "), + ] { + let (code, said, calls) = undrawable(&args); + assert!( + said.contains(&format!( + "TERM=xterm-no-such-entry names no terminfo entry here that can move the \ + cursor, and neither does xterm-256color, so the picker cannot be drawn. \ + Name the workspace instead: {line}\r\n" + )), + "{args:?}: the refusal was not given: {said:?}" + ); + assert_eq!( + code, + Some(1), + "{args:?}: a refused picker exited {code:?}: {said:?}" + ); + assert!( + !said.contains("\x1b[?1049h"), + "{args:?}: a picker was opened anyway: {said:?}" + ); + assert!( + calls.iter().all(|call| call.starts_with("list")), + "{args:?}: a refused picker still acted: {calls:?}" + ); + } +} + +/// `dl ` on a pty where no terminfo entry can draw the picker: `TERM` names +/// no entry, and `TERMINFO_DIRS` points the search at an empty directory. The +/// exit code, or `None` for a run still going at [`DEADLINE`], everything the +/// terminal was sent, and every devpod call. +/// +/// No picker should open, so nothing is waited for but the exit. +fn undrawable(args: &[&str]) -> (Option, String, Vec) { + let world = World::new(); + let nothing = world.root.join("terminfo"); + std::fs::create_dir_all(¬hing).expect("an empty terminfo directory"); + let mut command = world.command(args, Some("xterm-no-such-entry")); + command.env("TERMINFO_DIRS", nothing.display().to_string()); + let pair = native_pty_system() + .openpty(PtySize { + rows: ROWS, + cols: COLS, + pixel_width: 0, + pixel_height: 0, + }) + .expect("a pty"); + let mut child = pair + .slave + .spawn_command(command) + .expect("the dl binary runs"); + drop(pair.slave); + let mut reader = pair.master.try_clone_reader().expect("a pty reader"); + let collecting = std::thread::spawn(move || { + let mut said = Vec::new(); + let _ = reader.read_to_end(&mut said); + said + }); + // Polled against the deadline rather than waited on: a picker that opened + // anyway would sit on this pty for a key that never comes. + let gone = Instant::now() + DEADLINE; + let code = loop { + match child.try_wait().expect("dl's status") { + Some(status) => break Some(status.exit_code()), + None if Instant::now() >= gone => break None, + None => std::thread::sleep(Duration::from_millis(20)), + } + }; + let _ = child.kill(); + // Every slave fd is gone with the child, so the reader's EOF is already on its + // way and this join waits on it rather than for it. + let said = collecting.join().expect("the collected bytes"); + ( + code, + String::from_utf8_lossy(&said).into_owned(), + world.devpod_calls(), + ) +} + /// The batch. `dl rm` is the verb TAB exists for, and the heading is the only thing /// in the run that says how many rows it took — devpod's own lines arrive one at a /// time and say nothing about the extent of what was asked for. From ba0fc4ed43a0a992710a2bd8d07f758a1336bd09 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 28 Sep 2026 21:20:15 +0000 Subject: [PATCH 6/6] docs: drawable claimed an entry without smcup still gives a picker The draw works without smcup, but the entry then lacks rmcup as well, so skim's pause writes nothing to restore the screen and the picker's rows stay on it. The comment now says so. Claude-Session: https://claude.ai/code/session_01DiHLcVgAuJAMdsS4skh48w --- rust/dl/src/select.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rust/dl/src/select.rs b/rust/dl/src/select.rs index df29efca..36bba513 100644 --- a/rust/dl/src/select.rs +++ b/rust/dl/src/select.rs @@ -814,8 +814,10 @@ const FALLBACK_TERM: &str = "xterm-256color"; /// without, since every row is placed with it. The `term` crate's built-in ANSI /// entry, which `TermInfo::from_name` returns for any `xterm*`, `screen*` or /// `tmux*` name that has no database entry behind it, holds colours and bold and no -/// `cup`. The alternate screen (`smcup`) is not needed: without it skim draws over -/// the visible screen, which is still a picker. +/// `cup`. The alternate screen (`smcup`) is not needed to draw: without it skim +/// draws over the visible screen, which is still a picker. It is missed afterwards, +/// though. An entry without `smcup` has no `rmcup` either, so nothing puts the old +/// screen back when skim closes, and the picker's rows stay where they were drawn. fn drawable(entry: &TermInfo) -> bool { entry.strings.contains_key("cup") }