diff --git a/CHANGELOG.md b/CHANGELOG.md index a13a4e7e..89936c09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ 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. 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 ### 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..28e04648 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -36,6 +36,11 @@ 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, 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 # guard borrows the RwLock and unlocks on drop, so an owned guard would have to 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/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/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 93405fc2..36bba513 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::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,17 @@ pub(crate) fn pick(workspaces: &[Workspace], arity: Arity, cache_dir: &Path) -> if !a_terminal_exists() { return Pick::NoTerminal; } + let swapped = match plan( + TermInfo::from_env(), + TermInfo::from_name(FALLBACK_TERM), + Was::read(), + ) { + Plan::Keep => None, + Plan::Swap { was, reason } => Some(DrawableTerm::swap(was, reason)), + Plan::Refuse { reason } => return Pick::Undrawable(reason), + }; let rows = run_skim(&offering, arity); + drop(swapped); chosen(&offering.offers, rows) } @@ -780,6 +797,134 @@ 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 skim can draw with. +/// +/// 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 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") +} + +/// 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`], 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 +/// 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 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 = missing.unwrap_or_else(|| "TERM is unset".to_owned()); + return Plan::Swap { was, reason }; + } + let reason = match missing { + None => format!( + "TERM is unset, and {FALLBACK_TERM} names no terminfo entry here that can move \ + the cursor" + ), + Some(missing) => 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 +/// 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 { + 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, 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 { was, 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 `swap`. `Skim::run_with` has returned, and joined its + // input thread, by the time this runs. + unsafe { + match &self.was { + Was::Set(before) => std::env::set_var("TERM", before), + Was::Unset => 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 @@ -1996,4 +2141,102 @@ 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"]) + } + + /// `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()), 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()), 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() + } + ); + } + + #[test] + fn an_unset_term_is_swapped_for_one_that_can_move_the_cursor() { + assert_eq!( + plan(Err(term::Error::TermUnset), Ok(full()), Was::Unset), + Plan::Swap { + was: Was::Unset, + 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()), set("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()), Was::Unset), + Plan::Refuse { + reason: "TERM is unset, and xterm-256color names no terminfo entry here that can \ + move the cursor" + .to_owned() + } + ); + } + + /// `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() + } + ); + } } diff --git a/rust/dl/tests/picker.rs b/rust/dl/tests/picker.rs index 8c50563a..f2bdf464 100644 --- a/rust/dl/tests/picker.rs +++ b/rust/dl/tests/picker.rs @@ -290,6 +290,167 @@ 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] +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. 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 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, terms) = 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 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:?}" + ); +} + +#[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 @@ -403,6 +564,20 @@ impl Screen { settled: impl Fn(&Screen) -> bool, dismiss: Dismiss, ) -> (Self, Vec, String) { + 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`, 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, Vec) { let world = World::new(); let pair = native_pty_system() .openpty(PtySize { @@ -415,7 +590,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. @@ -486,7 +661,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. @@ -700,6 +880,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"}, @@ -747,12 +936,28 @@ 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` /// 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 { @@ -770,11 +975,17 @@ 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")); 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");