diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 56e1f45b7..374fa2e35 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -676,10 +676,12 @@ Inline entry and resume clear the final viewport through Ratatui's `resize()` ev `Skim::init_tui_with()` sets the private `Tui::no_clear` flag when attaching either the default or a caller-provided backend, before event handling can exit the terminal. `Tui::exit()` records a completed exit so repeated calls, including `Drop`, do not repeat cleanup or cursor movement. Entering or resuming the terminal resets this state. `--no-clear` preserves the final interface but still restores terminal input modes. In both inline and fullscreen modes, exit moves the cursor to the last viewport row and writes a newline before stdout or the shell resumes. This prevents output from overwriting the retained interface. Scrolling can move its top rows off screen, particularly in fullscreen mode. A panic hook is installed once (`PANIC_HOOK_SET: Once`) to ensure `cleanup_terminal(false)` always restores the screen on panics. -**Foreground `execute` actions:** the `execute(cmd)` action must hand the terminal to a child process (e.g. an editor or an interactive TUI like `ncdu`). `handle_action` only expands the command and returns `Event::RunExecute(cmd)`; the actual run happens in `App::handle_event` (which owns the `Tui`) via the `run_foreground(tui, cmd)` helper in `src/tui/app.rs`. `run_foreground` calls `Tui::stop_and_join()` — which cancels the event-pump task **and blocks until it has dropped its `EventStream`** — so skim's reader stops consuming terminal input before the child starts; otherwise the two race for keystrokes and interactive children appear to freeze. It then leaves the alternate screen / raw mode, spawns the child with its **own** stdin opened from the controlling terminal (`/dev/tty`, or `CONIN$` on Windows; falls back to inheriting skim's stdin), waits for it, restores terminal modes, and calls `Tui::start()` to respawn the reader. Giving the child its own tty stdin is what lets `execute` work when skim's own stdin is a pipe (`find | sk`). `execute-silent(cmd)` needs no terminal and is still spawned directly inside `handle_action` with stdout/stderr sent to `/dev/null`. +**Foreground `execute` actions:** the `execute(cmd)` action must hand the terminal to a child process (e.g. an editor or an interactive TUI like `ncdu`). `handle_action` only expands the command and returns `Event::RunExecute(cmd, env, unset)` with the environment snapshot taken at dispatch; the actual run happens in `App::handle_event` (which owns the `Tui`) via the `run_foreground(tui, cmd)` helper in `src/tui/app.rs`. `run_foreground` calls `Tui::stop_and_join()` — which cancels the event-pump task **and blocks until it has dropped its `EventStream`** — so skim's reader stops consuming terminal input before the child starts; otherwise the two race for keystrokes and interactive children appear to freeze. It then leaves the alternate screen / raw mode, spawns the child with its **own** stdin opened from the controlling terminal (`/dev/tty`, or `CONIN$` on Windows; falls back to inheriting skim's stdin), waits for it, restores terminal modes, and calls `Tui::start()` to respawn the reader. Giving the child its own tty stdin is what lets `execute` work when skim's own stdin is a pipe (`find | sk`). `execute-silent(cmd)` needs no terminal and is still spawned directly inside `handle_action` with stdout/stderr sent to `/dev/null`. Two subtleties make the resume correct. First, `Tui::start` installs a **fresh** `CancellationToken` on every call: a token stays cancelled once cancelled, so reusing the one `stop_and_join` cancelled would make the respawned reader observe the cancellation immediately and exit without reading input. Second, the post-execute repaint uses `Tui::force_full_redraw` (which resets both of ratatui's diff buffers) followed by `Event::Render`, rather than `Event::Redraw`/`tui.clear()`: ratatui's `Terminal::clear` first queries the cursor position, and crossterm writes that query (`ESC [ 6 n`) to **stdout**. Since skim renders to stderr and its stdout is routinely redirected (`sk > file`, `find | sk | …`), that query would reach no terminal, get no reply, and stall the UI for seconds before erroring out. `force_full_redraw` performs no cursor query and works for both fullscreen and inline viewports. +**Child-process environment (issue #1195):** `App::child_env()` snapshots the TUI state (query, current and selected item text, counts, cursor position, area geometry, list direction, input state, prompt/pointer, item wrap, preview geometry, last action and key) and returns it as `(name, value)` pairs. `App::dispatch_action` records the canonical name of the last action, and `App::handle_key` records the fzf-style name of the last real key press (via `binds::key_name`); synthetic events routed through the keymap do not overwrite it. Every value is exported under both a `SKIM_` and an `FZF_` prefix so fzf-style scripts work unchanged. Variables that do not apply are omitted rather than exported empty, matching fzf: `CURRENT_ITEM` when there is no current item, `NTH`/`WITH_NTH` when unset, `WRAP` when wrapping is off, and the `PREVIEW_*` group when no preview is configured. `App::child_env_unset` returns the names the snapshot omits; internal command paths use `App::child_env_snapshot` to build both lists once, and `tui::util::apply_child_env` removes them from the inherited environment before setting the pairs, so an enclosing skim/fzf process cannot leak stale values into a child. The plain preview and `Tui::run_execute` call it; PTY previews apply the same removals and assignments with `CommandBuilder`, `execute-silent` calls it inline, and the reload / `--cmd` path forwards both lists through `Reader::set_child_env` → `CommandCollector::set_env` (a defaulted trait method), which `SkimItemReader` applies to the command it spawns. `Skim::start` snapshots the initial state before the reader starts. Reload events capture their state in `App::reload_event` before actions clear the selection; `Skim::handle_reload` takes the next snapshot from `App::pending_reload_env`, or captures the current state for a direct call. Reload snapshots stay in dispatch order in `App::pending_reload_env`, so later queued actions cannot overwrite them; `execute` carries its snapshot inside `Event::RunExecute(cmd, env, unset)`. `LINES`/`COLUMNS` come from `App::term_area`, recorded on each render. + Image preview protocol detection is owned by `Skim::enter()`, not `Tui::enter_terminal()`: `--image=detect` temporarily ensures an alternate screen is active, queries `ratatui_image::picker::Picker::from_query_stdio()`, falls back to `Picker::halfblocks()` on failure, then stores the picker in both `SkimOptions.image_picker` and the `Preview` widget. `--image=halfblocks` skips detection and installs `Picker::halfblocks()` directly. ### Event Loop @@ -724,6 +726,7 @@ Frame rate is capped at 120 fps (`FRAME_TIME_MS = 1000/120`). `App::handle_event | `layout` | `AppLayout` | Last-frame widget areas (updated in render) | | `needs_render` | `Arc` | Signal from matcher → event loop | | `yank_register` | `String` | Cut/yank buffer | +| `term_area` | `Rect` | Area recorded on every render; source of `LINES`/`COLUMNS` in `child_env` | | `query_history` / `cmd_history` | `Vec` | History for ↑/↓ navigation | | `last_left_click` | `Option` | Detect two left clicks within the 500 ms `double-click` window | @@ -733,7 +736,7 @@ Frame rate is capped at 120 fps (`FRAME_TIME_MS = 1000/120`). `App::handle_event Event::Render → tui.draw(|f| f.render_widget(&mut self, f.area())) Event::Heartbeat → update_spinner(); check pending_matcher_restart; throttled render Event::RunPreview → run_preview(tui) -Event::RunExecute(cmd) → run_foreground(tui, cmd); force_full_redraw(); send Render +Event::RunExecute(cmd, env, unset) → run_foreground(tui, cmd); force_full_redraw(); send Render Event::Key(k) → handle_key(k) → [Action…] → tui.event_tx.send(Event::Action) Event::Action(a) → handle_action(a) → [Event…] → tui.event_tx.send(…) Event::Paste(t) → input.insert_str(cleaned); on_query_changed() diff --git a/Cargo.toml b/Cargo.toml index 22af2922b..fb2880e4b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,6 +115,7 @@ insta = "1.47" rand = "0.10.0" serde_json = { version = "=1.0.154" } serial_test = "=3.5.0" +shlex = "2.0.1" [[bench]] name = "read_and_match" diff --git a/README.md b/README.md index 7518a9fc3..44e82f7a1 100644 --- a/README.md +++ b/README.md @@ -503,6 +503,43 @@ You can configure key bindings to start external processes without leaving Skim sk --bind 'f1:execute(less -f {}),ctrl-y:execute-silent(echo {} | pbcopy)+abort' ``` +### Environment variables exported to child processes + +Commands run by `--preview`, `execute`, `execute-silent` and `reload` inherit a snapshot of +skim's state as environment variables. Each value is exported twice: once under a `SKIM_` +prefix and once under the same name with an `FZF_` prefix, so fzf-style scripts work +unchanged. This avoids quoting issues and keeps commands short. + +| Variable | Value | +|---|---| +| `QUERY` | Current query input | +| `CURRENT_ITEM` | Text of the current item (ANSI-stripped); unset if there is none | +| `SELECTED_ITEMS` | Selected items, newline-separated | +| `POS` | 1-based position of the current item | +| `TOTAL_COUNT` | Total number of items | +| `MATCH_COUNT` | Number of matched items | +| `SELECT_COUNT` | Number of selected items | +| `LINES`, `COLUMNS` | Height and width of the skim area | +| `DIRECTION` | List direction (`up` or `down`) | +| `INPUT_STATE` | `enabled` or `disabled` | +| `PROMPT` | Current prompt string | +| `POINTER` | Current pointer (selector) string | +| `ACTION`, `KEY` | Name of the running action and of the last key pressed (empty before any input); a follow-up such as `act-up:execute-silent(...)` reports `execute-silent` | +| `WRAP` | `char` when item wrapping is on | +| `PREVIEW_LINES`, `PREVIEW_COLUMNS`, `PREVIEW_TOP`, `PREVIEW_LEFT` | Geometry of the preview window (only set when a preview is configured) | +| `NTH`, `WITH_NTH` | Current `--nth` and `--with-nth` values (only set when configured) | + +```sh +# Equivalent to --preview 'echo {}', without the quoting pitfalls of {} +sk --preview 'printf "%s" "$SKIM_CURRENT_ITEM"' +``` + +A variable that does not apply to the current state (no preview, no `--nth`, an oversized value, +…) is removed from the child's environment instead of being left at its inherited value, so a skim +started inside another skim or fzf never leaks the enclosing process's values into its children. +The same variables are passed to the very first `--cmd` and to a `reload`, the latter capturing the +focused and selected items as they were when the reload was triggered. + ## Algorithms Skim offers multiple algorithms, check the help or manpage for an exhaustive list. Among them are: diff --git a/flake.nix b/flake.nix index 536d26904..a57aaebde 100644 --- a/flake.nix +++ b/flake.nix @@ -67,7 +67,17 @@ export VAGRANT_LIBVIRT_OVMF_CODE="${pkgs.OVMF.fd}/FV/OVMF_CODE.fd" ''; - mkShell = packages: shellHook: pkgs.mkShell { inherit packages shellHook; }; + mkShell = + packages: shellHook: + pkgs.mkShell { + inherit packages; + shellHook = '' + echo "Making sure the rust toolchains are available" + ${lib.getExe pkgs.rustup} -q install --no-update --no-self-update + ${lib.getExe pkgs.rustup} -q install --no-update --no-self-update nightly + ${shellHook} + ''; + }; in { default = mkShell base ""; diff --git a/man/man1/sk.1 b/man/man1/sk.1 index e4f77c564..732dce20d 100644 --- a/man/man1/sk.1 +++ b/man/man1/sk.1 @@ -1171,6 +1171,10 @@ In the `preview` flag, `execute`, `reload`, `set\-query`... binds, sk will expan item(s). .br +.br +Instead of placeholders, child commands can also read the same state from the `SKIM_*`/`FZF_*` variables listed under ENVIRONMENT VARIABLES. +.br + .SS "Field index expression" .br @@ -1227,6 +1231,56 @@ Example: If set to a non\-empty value, will disable coloring .br +.SS "Variables exported to child processes" + +.br +Commands spawned by `\-\-preview`, `execute`, `execute\-silent` and `reload` inherit these variables. +.br +Every value is exported twice: under a `SKIM_` prefix and under the same name with an `FZF_` prefix. +.br +* `QUERY`: current query input. +.br +* `CURRENT_ITEM`: text of the current item, with ANSI sequences stripped; unset when there is none. +.br +* `SELECTED_ITEMS`: selected items, newline\-separated. +.br +* `POS`: 1\-based position of the current item in the list. +.br +* `TOTAL_COUNT`: total number of items. +.br +* `MATCH_COUNT`: number of matched items. +.br +* `SELECT_COUNT`: number of selected items. +.br +* `LINES`, `COLUMNS`: height and width of the skim area. +.br +* `DIRECTION`: list direction, `up` or `down`. +.br +* `INPUT_STATE`: `enabled` or `disabled`. +.br +* `PROMPT`, `POINTER`: current prompt and pointer (selector) strings. +.br +* `ACTION`, `KEY`: name of the last action performed and last key pressed (empty before any input). +.br + `ACTION` is the action that is running, so a child spawned by a follow\-up +.br + (`act\-up:execute\-silent(...)`) sees `execute\-silent`. +.br +* `WRAP`: `char` when item wrapping is enabled, otherwise unset. +.br +* `PREVIEW_LINES`, `PREVIEW_COLUMNS`, `PREVIEW_TOP`, `PREVIEW_LEFT`: geometry of the preview +.br + window; only set when a preview is configured. +.br +* `NTH`, `WITH_NTH`: current `\-\-nth` and `\-\-with\-nth` values, when set. +.br +A variable that does not apply to the current state is removed from the child\*(Aqs environment, so a +.br +skim started inside another skim or fzf never passes the enclosing process\*(Aqs values through. +.br +Example: `sk \-\-preview \*(Aqprintf %s "$SKIM_QUERY"\*(Aq`. +.br + .SH THEME .br diff --git a/src/binds.rs b/src/binds.rs index c7decfca5..77e25e79d 100644 --- a/src/binds.rs +++ b/src/binds.rs @@ -82,6 +82,40 @@ impl SkimEvent { _ => None, } } + + /// The event's name, as accepted by [`Self::from_name`]. + #[must_use] + pub const fn name(self) -> &'static str { + match self { + SkimEvent::Start => "start", + SkimEvent::Load => "load", + SkimEvent::Change => "change", + SkimEvent::Result => "result", + SkimEvent::Focus => "focus", + SkimEvent::Zero => "zero", + SkimEvent::One => "one", + SkimEvent::DoubleClick => "double-click", + } + } + + /// Reverse of [`Self::key_code`]: the event carried by a reserved key code. + /// + /// Returns `None` for real keys, so callers can tell a key press from a + /// synthetic event routed through the keymap. + #[must_use] + pub const fn from_key_code(code: KeyCode) -> Option { + match code { + KeyCode::F(255) => Some(SkimEvent::Change), + KeyCode::F(254) => Some(SkimEvent::Start), + KeyCode::F(253) => Some(SkimEvent::Load), + KeyCode::F(252) => Some(SkimEvent::Result), + KeyCode::F(251) => Some(SkimEvent::Focus), + KeyCode::F(250) => Some(SkimEvent::Zero), + KeyCode::F(249) => Some(SkimEvent::One), + KeyCode::F(248) => Some(SkimEvent::DoubleClick), + _ => None, + } + } } impl From for KeyEvent { @@ -297,6 +331,73 @@ pub fn parse_key(key: &str) -> Result { Ok(KeyEvent::new(keycode, mods)) } +/// Formats a key event as an fzf-style name (`ctrl-a`, `alt-b`, `shift-tab`, +/// `up`, `f1`, `space`). +/// +/// Synthetic `SkimEvent`s are named after the event (see +/// [`SkimEvent::from_key_code`]). This is the value exported as +/// `SKIM_KEY`/`FZF_KEY` to child processes. +#[must_use] +pub fn key_name(key: &KeyEvent) -> String { + if let Some(event) = SkimEvent::from_key_code(key.code) { + return event.name().to_string(); + } + + // Crossterm can report back-tab with every modifier set; that is still shift-tab. + if key.code == KeyCode::BackTab && key.modifiers == KeyModifiers::all() { + return "shift-tab".to_string(); + } + + let mut name = String::new(); + if key.modifiers.contains(KeyModifiers::CONTROL) { + name.push_str("ctrl-"); + } + if key.modifiers.contains(KeyModifiers::ALT) { + name.push_str("alt-"); + } + // `BackTab` already means shift-tab, so don't double it up. + if key.modifiers.contains(KeyModifiers::SHIFT) && !matches!(key.code, KeyCode::Char(_) | KeyCode::BackTab) { + name.push_str("shift-"); + } + + let body = match key.code { + KeyCode::Char(' ') => "space".to_string(), + KeyCode::Char(c) => { + // `parse_key` folds a shift into an upper-case letter, so reproduce + // that; a shift on a non-letter keeps the explicit prefix. + if key.modifiers.contains(KeyModifiers::SHIFT) { + if c.is_ascii_alphabetic() { + c.to_ascii_uppercase().to_string() + } else { + name.push_str("shift-"); + c.to_string() + } + } else { + c.to_string() + } + } + KeyCode::Enter => "enter".to_string(), + KeyCode::Backspace => "backspace".to_string(), + KeyCode::Tab => "tab".to_string(), + KeyCode::BackTab => "shift-tab".to_string(), + KeyCode::Esc => "esc".to_string(), + KeyCode::Up => "up".to_string(), + KeyCode::Down => "down".to_string(), + KeyCode::Left => "left".to_string(), + KeyCode::Right => "right".to_string(), + KeyCode::Home => "home".to_string(), + KeyCode::End => "end".to_string(), + KeyCode::PageUp => "page-up".to_string(), + KeyCode::PageDown => "page-down".to_string(), + KeyCode::Insert => "insert".to_string(), + KeyCode::Delete => "delete".to_string(), + KeyCode::F(n) => format!("f{n}"), + other => format!("{other:?}").to_lowercase(), + }; + name.push_str(&body); + name +} + /// Parse an iterator of keymaps into a `KeyMap` pub fn parse_keymaps<'a, T>(maps: T) -> KeyMap where diff --git a/src/binds_tests.rs b/src/binds_tests.rs index f95f24c07..a0ebe0ccc 100644 --- a/src/binds_tests.rs +++ b/src/binds_tests.rs @@ -183,6 +183,57 @@ fn skim_event_name_roundtrip() { } } +#[test] +fn key_name_formats_fzf_style() { + // Canonical names round-trip through `parse_key`. + for name in [ + "a", + "A", + "1", + "ctrl-a", + "alt-b", + "ctrl-alt-c", + "ctrl-space", + "space", + "alt-enter", + "shift-tab", + "esc", + "up", + "down", + "left", + "right", + "home", + "end", + "f10", + ] { + let key = parse_key(name).unwrap(); + assert_eq!(key_name(&key), name, "{name}"); + } + // Keys that `parse_key` only knows by an alias still get the canonical name. + assert_eq!(key_name(&parse_key("pgup").unwrap()), "page-up"); + assert_eq!(key_name(&parse_key("bspace").unwrap()), "backspace"); + assert_eq!(key_name(&parse_key("alt-bspace").unwrap()), "alt-backspace"); + // A shift on a non-character key keeps the prefix. + assert_eq!(key_name(&KeyEvent::new(KeyCode::Tab, KeyModifiers::SHIFT)), "shift-tab"); + assert_eq!( + key_name(&KeyEvent::new(KeyCode::BackTab, KeyModifiers::all())), + "shift-tab" + ); + // Synthetic events report their event name. + for event in [ + SkimEvent::Start, + SkimEvent::Load, + SkimEvent::Change, + SkimEvent::Result, + SkimEvent::Focus, + SkimEvent::Zero, + SkimEvent::One, + SkimEvent::DoubleClick, + ] { + assert_eq!(key_name(&event.key_event()), event.name()); + } +} + #[test] fn double_click_accepts_by_default() { assert_eq!( diff --git a/src/helper/item_reader.rs b/src/helper/item_reader.rs index 6ab448a13..83fde8cc9 100644 --- a/src/helper/item_reader.rs +++ b/src/helper/item_reader.rs @@ -196,6 +196,11 @@ pub struct SkimItemReader { /// [`with_thread_pool`](Self::with_thread_pool) or /// [`set_thread_pool`](Self::set_thread_pool). thread_pool: Arc, + /// Environment variables exported to the spawned command (see issue #1195). + env: Vec<(String, String)>, + /// State variables to remove from the inherited environment before spawning, + /// because the current snapshot omits them (see issue #1195). + env_unset: Vec, } fn default_thread_pool() -> Arc { @@ -209,6 +214,8 @@ impl Default for SkimItemReader { Self { option: Arc::new(Default::default()), thread_pool: default_thread_pool(), + env: Vec::new(), + env_unset: Vec::new(), } } } @@ -220,6 +227,8 @@ impl SkimItemReader { Self { option: Arc::new(option), thread_pool: default_thread_pool(), + env: Vec::new(), + env_unset: Vec::new(), } } @@ -527,20 +536,32 @@ impl CommandCollector for SkimItemReader { components_to_stop: Arc, ) -> (SkimItemReceiver, crate::prelude::Sender) { let send_error = self.option.show_error; - let (child, source) = get_command_output(cmd, send_error).expect("command not found"); + let (child, source) = + get_command_output(cmd, send_error, &self.env, &self.env_unset).expect("command not found"); self.parallel_bufread(source, child, &components_to_stop) } fn set_thread_pool(&mut self, pool: Arc) { self.thread_pool = pool; } + + fn set_env(&mut self, env: &[(String, String)], unset: &[String]) { + self.env = env.to_vec(); + self.env_unset = unset.to_vec(); + } } type CommandOutput = (Option, Box); -fn get_command_output(cmd: &str, send_error: bool) -> Result> { +fn get_command_output( + cmd: &str, + send_error: bool, + env: &[(String, String)], + unset: &[String], +) -> Result> { let (reader, writer) = std::io::pipe()?; let mut command = crate::shell_cmd(cmd); + crate::tui::util::apply_child_env(&mut command, env, unset); command.stdout(writer.try_clone()?); if send_error { trace!("redirecting stderr to the output"); diff --git a/src/helper/item_reader_tests.rs b/src/helper/item_reader_tests.rs index 081ed78a0..5bc3c183c 100644 --- a/src/helper/item_reader_tests.rs +++ b/src/helper/item_reader_tests.rs @@ -123,6 +123,20 @@ fn invoke_with_show_error_redirects_stderr() { assert_eq!(drain(rx), vec!["oops"]); } +#[test] +fn invoke_exports_child_env() { + // `set_env` (issue #1195) makes the spawned command inherit the variables. + let cmd = if cfg!(windows) { + "echo %SKIM_QUERY%" + } else { + "printf '%s' \"$SKIM_QUERY\"" + }; + let mut reader = SkimItemReader::default(); + reader.set_env(&[("SKIM_QUERY".to_string(), "hi".to_string())], &[]); + let (rx, _tx) = reader.invoke(cmd, Arc::new(AtomicUsize::new(0))); + assert_eq!(drain(rx), vec!["hi"]); +} + #[test] fn read0_false_restores_newline_ending() { let opt = SkimItemReaderOption::default().read0(true).read0(false).build(); diff --git a/src/manpage.rs b/src/manpage.rs index 48ff2b3cf..b5f60304f 100644 --- a/src/manpage.rs +++ b/src/manpage.rs @@ -357,6 +357,8 @@ In the `preview` flag, `execute`, `reload`, `set-query`... binds, sk will expand item. * {+FIELD_INDEX_EXPRESSION} will be expanded to the field index expression run against the {+} item(s). + +Instead of placeholders, child commands can also read the same state from the `SKIM_*`/`FZF_*` variables listed under ENVIRONMENT VARIABLES. ", ); subsection( @@ -407,6 +409,36 @@ Example: "If set to a non-empty value, will disable coloring", ); + subsection( + &mut custom, + "Variables exported to child processes", + " +Commands spawned by `--preview`, `execute`, `execute-silent` and `reload` inherit these variables. +Every value is exported twice: under a `SKIM_` prefix and under the same name with an `FZF_` prefix. +* `QUERY`: current query input. +* `CURRENT_ITEM`: text of the current item, with ANSI sequences stripped; unset when there is none. +* `SELECTED_ITEMS`: selected items, newline-separated. +* `POS`: 1-based position of the current item in the list. +* `TOTAL_COUNT`: total number of items. +* `MATCH_COUNT`: number of matched items. +* `SELECT_COUNT`: number of selected items. +* `LINES`, `COLUMNS`: height and width of the skim area. +* `DIRECTION`: list direction, `up` or `down`. +* `INPUT_STATE`: `enabled` or `disabled`. +* `PROMPT`, `POINTER`: current prompt and pointer (selector) strings. +* `ACTION`, `KEY`: name of the last action performed and last key pressed (empty before any input). + `ACTION` is the action that is running, so a child spawned by a follow-up + (`act-up:execute-silent(...)`) sees `execute-silent`. +* `WRAP`: `char` when item wrapping is enabled, otherwise unset. +* `PREVIEW_LINES`, `PREVIEW_COLUMNS`, `PREVIEW_TOP`, `PREVIEW_LEFT`: geometry of the preview + window; only set when a preview is configured. +* `NTH`, `WITH_NTH`: current `--nth` and `--with-nth` values, when set. +A variable that does not apply to the current state is removed from the child's environment, so a +skim started inside another skim or fzf never passes the enclosing process's values through. +Example: `sk --preview 'printf %s \"$SKIM_QUERY\"'`. +", + ); + section(&mut custom, "THEME", THEME_SECTION); #[cfg(feature = "listen")] @@ -482,6 +514,26 @@ mod tests { } } + #[test] + fn manpage_documents_child_env_vars() { + let out = manpage_str(); + assert!(out.contains("Variables exported to child processes")); + for var in [ + "SKIM_", + "FZF_", + "CURRENT_ITEM", + "SELECTED_ITEMS", + "DIRECTION", + "INPUT_STATE", + "ACTION", + "KEY", + "PREVIEW_LINES", + "PREVIEW_TOP", + ] { + assert!(out.contains(var), "manpage should mention child env var '{var}'"); + } + } + #[test] fn manpage_documents_default_key_bindings() { let out = manpage_str(); diff --git a/src/reader.rs b/src/reader.rs index 329e568f4..f8c6714e6 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -34,6 +34,11 @@ pub trait CommandCollector { /// than spawning additional OS threads. The default implementation is a /// no-op; collectors that support pool-based I/O should override it. fn set_thread_pool(&mut self, _pool: Arc) {} + + /// Sets the environment variables exported to the commands this collector + /// spawns. The default implementation is a no-op; collectors that spawn + /// processes should override it. + fn set_env(&mut self, _env: &[(String, String)], _unset: &[String]) {} } /// Handle for controlling a running reader @@ -118,6 +123,12 @@ impl Reader { self.cmd_collector.borrow_mut().set_thread_pool(pool); } + /// Forwards environment variables (see issue #1195) to the underlying + /// [`CommandCollector`] so the next command it spawns inherits them. + pub fn set_child_env(&mut self, env: &[(String, String)], unset: &[String]) { + self.cmd_collector.borrow_mut().set_env(env, unset); + } + /// Starts the reader and returns a control handle pub fn run(&mut self, app_tx: Sender>>, cmd: &str) -> ReaderControl { let components_to_stop: Arc = Arc::new(AtomicUsize::new(0)); diff --git a/src/skim.rs b/src/skim.rs index 16e2ccd82..1c6997ec6 100644 --- a/src/skim.rs +++ b/src/skim.rs @@ -241,7 +241,10 @@ where /// Start the reader and matcher, but do not enter the TUI yet pub fn start(&mut self) { debug!("Starting reader with initial_cmd: {:?}", self.initial_cmd); - self.start_reader(&self.initial_cmd.clone()); + // Give the initial command the same contract as a reload: it sees the + // state skim starts with (`--query`, counts, geometry, …). + let (env, unset) = self.app.child_env_snapshot(); + self.start_reader(&self.initial_cmd.clone(), &env, &unset); self.app.restart_matcher(true); // If the TUI is already available (e.g. test harnesses that build the // TUI before starting), fire the `start` event now. In the normal @@ -273,6 +276,14 @@ where /// be reused by test harnesses without reimplementing it. pub fn handle_reload(&mut self, new_cmd: &str) { debug!("reloading with cmd {new_cmd}"); + // Export the state that triggered the reload (issue #1195) *before* + // clearing anything, so the command still sees the focused and + // selected items and the counts they imply. + let (env, unset) = self + .app + .pending_reload_env + .pop_front() + .unwrap_or_else(|| self.app.child_env_snapshot()); // Kill the current reader if let Some(rc) = self.reader_control.as_mut() { rc.kill(); @@ -285,10 +296,10 @@ where } self.app.item_list.keep_on_reload = self.app.options.no_clear_start || self.app.options.no_clear_if_empty; self.app.restart_matcher(true); - self.start_reader(new_cmd); + self.start_reader(new_cmd, &env, &unset); } - fn start_reader(&mut self, cmd: &str) { + fn start_reader(&mut self, cmd: &str, env: &[(String, String)], unset: &[String]) { let query_too_short = self.app.options.interactive && self.app.options.cmd.is_some() && self.app.query_below_min_length(); self.reader_control = if query_too_short { @@ -296,6 +307,7 @@ where self.app.item_list.clear(); None } else { + self.reader.set_child_env(env, unset); Some(self.reader.collect(self.app.item_pool.clone(), cmd)) }; self.reader_done = query_too_short; diff --git a/src/skim_tests.rs b/src/skim_tests.rs index a7545b102..0e7d8166b 100644 --- a/src/skim_tests.rs +++ b/src/skim_tests.rs @@ -235,6 +235,59 @@ fn startup_runs_command_when_length_gate_does_not_apply() { } } +#[cfg(unix)] +#[test] +fn initial_command_receives_the_startup_state() { + // The very first `--cmd` must see the same variables as a later reload. + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("initial"); + let mut options = SkimOptions::default(); + options.query = Some("seeded".to_string()); + options.cmd = Some(format!( + "printf '%s' \"$SKIM_QUERY\" > '{}'", + log.to_string_lossy().replace('\'', "'\\''") + )); + let mut skim = Skim::::init(options.build(), None).unwrap(); + skim.start(); + wait_until(|| skim.check_reader()); + assert_eq!(std::fs::read_to_string(log).unwrap(), "seeded"); +} + +#[cfg(unix)] +#[test] +fn reload_command_sees_the_state_before_the_list_is_cleared() { + // A reload must export the focused and selected items that triggered it, + // not the empty list left behind by clearing. + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("reload"); + let cmd = format!( + "printf '%s|%s|%s|%s' \"$SKIM_CURRENT_ITEM\" \"$SKIM_SELECTED_ITEMS\" \"$SKIM_QUERY\" \"$SKIM_ACTION\" > '{}'", + log.to_string_lossy().replace('\'', "'\\''") + ); + let mut skim = started_skim(&["alpha", "beta"]); + skim.app.item_list.items.push(crate::item::MatchedItem::new( + Arc::new("alpha".to_string()), + crate::Rank::default(), + None, + &crate::item::RankBuilder::default(), + )); + skim.app.item_list.current = 0; + skim.app.item_list.toggle(); + let tui = skim.tui.as_mut().unwrap(); + skim.app + .handle_event(tui, &Event::Action(Action::Reload(Some(cmd)))) + .unwrap(); + let cmd = loop { + if let Event::Reload(cmd) = tui.event_rx.try_recv().unwrap() { + break cmd; + } + }; + skim.app.input.value = "later".to_string(); + skim.handle_reload(&cmd); + wait_until(|| std::fs::read_to_string(&log).is_ok_and(|text| !text.is_empty())); + assert_eq!(std::fs::read_to_string(&log).unwrap(), "alpha|alpha||reload"); +} + #[test] fn should_enter_is_false_in_filter_mode() { let mut options = SkimOptions::default(); diff --git a/src/tui/app.rs b/src/tui/app.rs index a89a23e9f..d88179fc1 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -1,8 +1,10 @@ +use std::collections::VecDeque; use std::process::Stdio; use std::rc::Rc; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; +use crate::helper::item::strip_ansi; use crate::item::{ItemPool, MatchedItem}; use crate::matcher::{Matcher, MatcherControl}; use crate::prelude::ExactOrFuzzyEngineFactory; @@ -42,6 +44,14 @@ static NUM_THREADS: LazyLock = LazyLock::new(|| { }); const MATCHER_DEBOUNCE_MS: u128 = 200; +/// Largest value skim exports to child processes. Mirrors fzf's 64 KiB cap on +/// `FZF_CURRENT_ITEM`: a bigger value could exceed the environment size limit +/// and break every child command. +const MAX_ENV_VALUE_BYTES: usize = 64 * 1024; + +/// The exported pairs plus the names to remove from the inherited environment +/// (see [`App::child_env`] and [`App::child_env_unset`]). +type ChildEnv = (Vec<(String, String)>, Vec); const HIDE_GRACE_MS: u128 = 500; const DOUBLE_CLICK_INTERVAL: std::time::Duration = std::time::Duration::from_millis(500); @@ -58,9 +68,18 @@ pub struct App { pub should_quit: bool, /// The terminating action, including one dispatched inside a follow-up or conditional chain. pub(crate) final_action: Option, + /// Name of the last action performed, exported as `SKIM_ACTION`/`FZF_ACTION`. + pub(crate) last_action: Option, + /// Name of the last real key pressed, exported as `SKIM_KEY`/`FZF_KEY`. + pub(crate) last_key: Option, + /// Environment snapshots for reload events, captured before actions clear the selection. + pub(crate) pending_reload_env: VecDeque, /// Current cursor position (x, y) pub cursor_pos: (u16, u16), + /// The area skim draws into, recorded on every render so child processes + /// can be told the current terminal size (see [`Self::child_env`]). + pub(crate) term_area: Rect, /// Control handle for the matcher thread pub matcher_control: MatcherControl, /// The matcher for filtering items @@ -151,6 +170,7 @@ pub struct App { impl Widget for &mut App { fn render(self, area: Rect, buf: &mut Buffer) { let mut res = SkimRender::default(); + self.term_area = area; let has_border = self.options.border.is_some(); // Update header with reserved items (from --header-lines). When @@ -252,7 +272,11 @@ impl Default for App { theme, should_quit: false, final_action: None, + last_action: None, + last_key: None, + pending_reload_env: VecDeque::new(), cursor_pos: (0, 0), + term_area: Rect::default(), matcher: Matcher::builder(Rc::new(ExactOrFuzzyEngineFactory::builder().build())) .case(crate::CaseMatching::default()) .build(), @@ -326,7 +350,11 @@ impl App { theme, should_quit: false, final_action: None, + last_action: None, + last_key: None, + pending_reload_env: VecDeque::new(), cursor_pos: (0, 0), + term_area: Rect::default(), matcher: Matcher::from_options(&options), yank_register: String::new(), matcher_control: MatcherControl::default(), @@ -489,7 +517,7 @@ impl App { // In interactive mode with --cmd, execute the command with {} substitution if self.options.interactive && self.options.cmd.is_some() { return match self.expand_shell_cmd(&self.cmd, true) { - Ok(cmd) => vec![Event::Reload(cmd)], + Ok(cmd) => vec![self.reload_event(cmd)], Err(err) => { log::warn!("Command expansion rejected: {err}"); vec![] @@ -580,13 +608,20 @@ impl App { #[cfg(not(feature = "image"))] let quote_cmd = true; match preview { - ItemPreview::Command(cmd) => self.preview.spawn(tui, &self.expand_shell_cmd(&cmd, quote_cmd)?)?, + ItemPreview::Command(cmd) => { + let (env, unset) = self.child_env_snapshot(); + self.preview + .spawn(tui, &self.expand_shell_cmd(&cmd, quote_cmd)?, &env, &unset)?; + } ItemPreview::Text(t) | ItemPreview::AnsiText(t) => { self.preview.content(&t.bytes().collect::>())?; } ItemPreview::CommandWithPos(cmd, preview_position) => { // Execute command and apply position after content is ready - self.preview.spawn(tui, &self.expand_shell_cmd(&cmd, quote_cmd)?)?; + let (env, unset) = self.child_env_snapshot(); + self.preview + .spawn(tui, &self.expand_shell_cmd(&cmd, quote_cmd)?, &env, &unset)?; + // Apply position offsets let v_scroll = match preview_position.v_scroll { crate::tui::Size::Fixed(n) => n, @@ -624,9 +659,11 @@ impl App { ItemPreview::TextWithPos(t, preview_position) | ItemPreview::AnsiWithPos(t, preview_position) => self .preview .content_with_position(&t.bytes().collect::>(), preview_position)?, - ItemPreview::Global => self - .preview - .spawn(tui, &self.expand_shell_cmd(preview_opt, quote_cmd)?)?, + ItemPreview::Global => { + let (env, unset) = self.child_env_snapshot(); + self.preview + .spawn(tui, &self.expand_shell_cmd(preview_opt, quote_cmd)?, &env, &unset)?; + } } if preview_ready { let _ = tui.event_tx.try_send(Event::PreviewReady); @@ -716,8 +753,8 @@ impl App { warn!("RunPreview: error {e:?}"); } } - Event::RunExecute(cmd) => { - tui.run_execute(cmd)?; + Event::RunExecute(cmd, env, unset) => { + tui.run_execute(cmd, env, unset)?; self.handle_event(tui, &Event::Redraw)?; } Event::Clear => { @@ -828,6 +865,12 @@ impl App { let normalized_key = KeyEvent::new(key.code, key.modifiers); debug!("key event: {key:?}, normalized: {normalized_key:?}"); + // Record real key presses for `SKIM_KEY`/`FZF_KEY`, but not the + // synthetic `start`/`load`/`change`/… events routed through the keymap. + if crate::binds::SkimEvent::from_key_code(key.code).is_none() { + self.last_key = Some(crate::binds::key_name(key)); + } + if let Some(act) = &self.options.keymap.get(&normalized_key) { debug!("{act:?}"); return act.iter().map(|a| Event::Action(a.clone())).collect(); @@ -904,6 +947,11 @@ impl App { #[allow(clippy::enum_glob_use)] use Action::*; use ratatui::widgets::ListDirection::{BottomToTop, TopToBottom}; + // `SKIM_ACTION`/`FZF_ACTION` report the action that is running, so a + // child spawned by this action sees its own name even when the action + // was reached through a follow-up (`act-up:execute-silent(…)`) or a + // conditional chain. + self.last_action = Some(act.name().to_string()); match act { Abort | Accept(_) => { self.should_quit = true; @@ -1012,12 +1060,15 @@ impl App { // the event loop, which runs it via `Event::RunExecute`. let expanded_cmd = self.expand_shell_cmd(cmd, true)?; debug!("execute: {expanded_cmd}"); - return Ok(vec![Event::RunExecute(expanded_cmd)]); + let (env, unset) = self.child_env_snapshot(); + return Ok(vec![Event::RunExecute(expanded_cmd, env, unset)]); } ExecuteSilent(cmd) => { let expanded_cmd = self.expand_shell_cmd(cmd, true)?; debug!("execute-silent: {expanded_cmd}"); let mut command = crate::shell_cmd(&expanded_cmd); + let (env, unset) = self.child_env_snapshot(); + super::util::apply_child_env(&mut command, &env, &unset); command.stdout(Stdio::null()).stderr(Stdio::null()); let _ = command.spawn(); } @@ -1199,19 +1250,20 @@ impl App { return Ok(self.on_query_changed()); } Redraw => return Ok(vec![Event::Clear]), - Reload(Some(s)) => { - self.item_list.clear_selection(); - return Ok(vec![Event::Reload(self.expand_shell_cmd(s, true)?)]); - } - Reload(None) => { + Reload(cmd) => { + let expanded_cmd = match cmd { + Some(cmd) => self.expand_shell_cmd(cmd, true)?, + None => self.cmd.clone(), + }; + let event = self.reload_event(expanded_cmd); self.item_list.clear_selection(); - return Ok(vec![Event::Reload(self.cmd.clone())]); + return Ok(vec![event]); } RefreshCmd => { // Refresh the command (reload in interactive mode) if self.options.interactive { let expanded_cmd = self.expand_shell_cmd(&self.cmd, true)?; - return Ok(vec![Event::Reload(expanded_cmd)]); + return Ok(vec![self.reload_event(expanded_cmd)]); } } RefreshPreview => { @@ -1261,9 +1313,10 @@ impl App { // immediately re-run it. self.cmd.clone_from(cmd); self.options.cmd = Some(cmd.clone()); - self.item_list.clear_selection(); let expanded_cmd = self.expand_shell_cmd(cmd, true)?; - return Ok(vec![Event::Reload(expanded_cmd)]); + let event = self.reload_event(expanded_cmd); + self.item_list.clear_selection(); + return Ok(vec![event]); } SetHeader(opt_header) => { opt_header.clone_into(&mut self.options.header); @@ -1508,6 +1561,164 @@ impl App { ) } + fn reload_event(&mut self, cmd: String) -> Event { + self.pending_reload_env.push_back(self.child_env_snapshot()); + Event::Reload(cmd) + } + + /// Environment variables skim exports to its child processes (preview, + /// execute, execute-silent, reload). + /// + /// This mirrors the variables fzf exports to its child processes, under + /// both a `SKIM_` and an `FZF_` prefix. Item text is ANSI-stripped so it + /// matches what `{}` expands to. Like fzf, some variables are omitted when + /// they do not apply: `CURRENT_ITEM` when there is no current item, + /// `NTH`/`WITH_NTH` when unset, `WRAP` when wrapping is off, and the + /// `PREVIEW_*` group when no preview is configured. + /// + /// `LINES`/`COLUMNS` come from the last rendered area (see + /// [`Self::term_area`]). Values containing a NUL byte, or longer than + /// [`MAX_ENV_VALUE_BYTES`], are omitted because `exec(2)` cannot carry them + /// and a failure would break the child spawn. + #[must_use] + pub fn child_env(&self) -> Vec<(String, String)> { + // The list length, used to clamp `POS` the way fzf does. + let item_count = self.item_list.items.len(); + let mut vars: Vec<(&str, String)> = vec![ + ("QUERY", self.input.value.clone()), + ("POS", (self.item_list.current + 1).min(item_count).to_string()), + ("TOTAL_COUNT", self.item_pool.len().to_string()), + ("MATCH_COUNT", self.matcher_control.get_num_matched().to_string()), + ("SELECT_COUNT", self.item_list.selection.len().to_string()), + ("LINES", self.term_area.height.to_string()), + ("COLUMNS", self.term_area.width.to_string()), + // `Default` lays the list out bottom-up (fzf's "up"). + ( + "DIRECTION", + match self.options.layout { + TuiLayout::Default => "up", + TuiLayout::Reverse | TuiLayout::ReverseList => "down", + } + .to_string(), + ), + // skim only has enabled/disabled; fzf's "hidden" (`--no-input`) is + // parsed but not implemented, so the input is enabled in practice. + ( + "INPUT_STATE", + if self.options.disabled { "disabled" } else { "enabled" }.to_string(), + ), + ("PROMPT", self.input.prompt.clone()), + ("POINTER", self.options.selector_icon.clone()), + ("ACTION", self.last_action.clone().unwrap_or_default()), + ("KEY", self.last_key.clone().unwrap_or_default()), + ]; + if let Some(item) = self.item_list.selected() { + vars.push(("CURRENT_ITEM", strip_ansi(&item.output()).0)); + } + // Join lazily and stop as soon as the value is too large to export: + // `--select-all` on a huge list must not strip every item first. The + // variable is omitted entirely rather than truncated. + let mut joined = String::new(); + let mut too_large = false; + for (index, item) in self.item_list.selection.iter().enumerate() { + if index > 0 { + joined.push('\n'); + } + joined.push_str(&strip_ansi(&item.output()).0); + if joined.len() > MAX_ENV_VALUE_BYTES { + too_large = true; + break; + } + } + if !too_large { + vars.push(("SELECTED_ITEMS", joined)); + } + if !self.options.nth.is_empty() { + vars.push(("NTH", self.options.nth.join(","))); + } + if !self.options.with_nth.is_empty() { + vars.push(("WITH_NTH", self.options.with_nth.join(","))); + } + if self.options.wrap_items { + // skim only implements character wrapping. + vars.push(("WRAP", "char".to_string())); + } + if self.options.preview.is_some() || self.options.preview_fn.is_some() { + let preview_area = self.layout.preview_area; + vars.push(("PREVIEW_LINES", self.preview.rows.to_string())); + vars.push(("PREVIEW_COLUMNS", self.preview.cols.to_string())); + vars.push(("PREVIEW_TOP", preview_area.map_or(0, |a| a.y).to_string())); + vars.push(("PREVIEW_LEFT", preview_area.map_or(0, |a| a.x).to_string())); + } + + let mut env = Vec::with_capacity(vars.len() * 2); + for (name, value) in &vars { + // `exec(2)` cannot carry a NUL byte, and one oversized value can + // exhaust the environment size limit and break every child command. + // Omit such values (like fzf does for FZF_CURRENT_ITEM) instead of + // making the child fail to spawn. + if value.contains('\0') || value.len() > MAX_ENV_VALUE_BYTES { + continue; + } + env.push((format!("SKIM_{name}"), value.clone())); + env.push((format!("FZF_{name}"), value.clone())); + } + env + } + + /// The state variable names that [`Self::child_env`] omits from the current + /// snapshot, in both prefixes. + /// + /// A skipped pair does not unset an inherited variable, so spawn paths must + /// remove these names: otherwise a child of a skim started inside fzf would + /// still see the enclosing process's `FZF_CURRENT_ITEM` and friends. + #[must_use] + pub fn child_env_unset(&self) -> Vec { + self.child_env_snapshot().1 + } + + /// Build exported values and omitted names from the same state snapshot. + pub(crate) fn child_env_snapshot(&self) -> ChildEnv { + let exported = self.child_env(); + let unset = Self::child_env_names() + .into_iter() + .filter(|name| !exported.iter().any(|(set, _)| set == name)) + .collect(); + (exported, unset) + } + + /// Every state variable name [`Self::child_env`] can export, in both prefixes. + #[must_use] + pub fn child_env_names() -> Vec { + [ + "QUERY", + "SELECTED_ITEMS", + "POS", + "TOTAL_COUNT", + "MATCH_COUNT", + "SELECT_COUNT", + "LINES", + "COLUMNS", + "DIRECTION", + "INPUT_STATE", + "PROMPT", + "POINTER", + "ACTION", + "KEY", + "CURRENT_ITEM", + "NTH", + "WITH_NTH", + "WRAP", + "PREVIEW_LINES", + "PREVIEW_COLUMNS", + "PREVIEW_TOP", + "PREVIEW_LEFT", + ] + .into_iter() + .flat_map(|name| [format!("SKIM_{name}"), format!("FZF_{name}")]) + .collect() + } + /// Restart matcher with debouncing to avoid excessive restarts during rapid typing fn restart_matcher_debounced(&mut self) { const DEBOUNCE_MS: u64 = 10; diff --git a/src/tui/app_tests.rs b/src/tui/app_tests.rs index 3650d4a07..51fe0a756 100644 --- a/src/tui/app_tests.rs +++ b/src/tui/app_tests.rs @@ -673,7 +673,7 @@ fn execute_action_expands_and_defers_to_event_loop() { app.input.value = "hello".to_string(); let events = act(&mut app, Action::Execute("echo {q}".to_string())); assert!( - matches!(events.as_slice(), [Event::RunExecute(cmd)] if cmd.contains("hello")), + matches!(events.as_slice(), [Event::RunExecute(cmd, ..)] if cmd.contains("hello")), "expected RunExecute with expanded query, got {events:?}" ); } @@ -699,7 +699,7 @@ fn run_execute_event_runs_command_and_restarts_reader() { let noop = if cfg!(windows) { "rem" } else { "true" }; rt.block_on(async { - app.handle_event(&mut tui, &Event::RunExecute(noop.to_string())) + app.handle_event(&mut tui, &Event::RunExecute(noop.to_string(), Vec::new(), Vec::new())) .expect("handle_event failed"); }); @@ -1009,6 +1009,394 @@ fn expand_cmd_substitutes_query() { assert!(expanded.contains("myquery")); } +#[test] +fn child_env_exports_core_state_under_both_prefixes() { + let mut app = app_with_items(&["a", "b", "c"]); + app.input.value = "qry".to_string(); + app.item_list.select_row(0); + app.term_area = Rect::new(0, 0, 80, 24); + app.preview.rows = 10; + app.preview.cols = 40; + app.options.nth = vec!["2".to_string()]; + app.options.with_nth = vec!["1".to_string()]; + app.options.preview = Some("echo {}".to_string()); + app.layout.preview_area = Some(Rect::new(3, 20, 80, 4)); + + let env = app.child_env(); + let get = |key: &str| { + env.iter() + .find(|(name, _)| name == key) + .map(|(_, value)| value.as_str()) + }; + + // Every value is exported under both prefixes. + for (skim, fzf, value) in [ + ("SKIM_QUERY", "FZF_QUERY", "qry"), + ("SKIM_CURRENT_ITEM", "FZF_CURRENT_ITEM", "a"), + ("SKIM_SELECTED_ITEMS", "FZF_SELECTED_ITEMS", "a"), + ("SKIM_SELECT_COUNT", "FZF_SELECT_COUNT", "1"), + ("SKIM_POS", "FZF_POS", "1"), + ("SKIM_LINES", "FZF_LINES", "24"), + ("SKIM_COLUMNS", "FZF_COLUMNS", "80"), + ("SKIM_DIRECTION", "FZF_DIRECTION", "up"), + ("SKIM_INPUT_STATE", "FZF_INPUT_STATE", "enabled"), + ("SKIM_PROMPT", "FZF_PROMPT", "> "), + ("SKIM_POINTER", "FZF_POINTER", ">"), + ("SKIM_PREVIEW_LINES", "FZF_PREVIEW_LINES", "10"), + ("SKIM_PREVIEW_COLUMNS", "FZF_PREVIEW_COLUMNS", "40"), + ("SKIM_PREVIEW_TOP", "FZF_PREVIEW_TOP", "20"), + ("SKIM_PREVIEW_LEFT", "FZF_PREVIEW_LEFT", "3"), + ("SKIM_NTH", "FZF_NTH", "2"), + ("SKIM_WITH_NTH", "FZF_WITH_NTH", "1"), + ] { + assert_eq!(get(skim), Some(value), "{skim}"); + assert_eq!(get(fzf), Some(value), "{fzf}"); + } +} + +#[test] +fn child_env_omits_variables_that_do_not_apply() { + // No preview, no wrap, no nth, and an empty list: the matching variables + // are absent, not empty. + let app = App::default(); + let env = app.child_env(); + let get = |key: &str| env.iter().find(|(name, _)| name == key).map(|(_, v)| v.as_str()); + for key in [ + "SKIM_CURRENT_ITEM", + "FZF_CURRENT_ITEM", + "SKIM_NTH", + "FZF_NTH", + "SKIM_WITH_NTH", + "FZF_WITH_NTH", + "SKIM_WRAP", + "FZF_WRAP", + "SKIM_PREVIEW_LINES", + "FZF_PREVIEW_LINES", + "SKIM_PREVIEW_TOP", + "FZF_PREVIEW_TOP", + ] { + assert_eq!(get(key), None, "{key}"); + } + // The always-on variables are still present. + assert_eq!(get("SKIM_POS"), Some("0")); + assert_eq!(get("SKIM_DIRECTION"), Some("up")); + assert_eq!(get("SKIM_INPUT_STATE"), Some("enabled")); +} + +#[test] +fn child_env_wrap_and_disabled_reflect_options() { + let mut app = app_with_items(&["a"]); + app.options.wrap_items = true; + app.options.disabled = true; + app.options.layout = TuiLayout::Reverse; + let env = app.child_env(); + let get = |key: &str| env.iter().find(|(name, _)| name == key).map(|(_, v)| v.as_str()); + assert_eq!(get("SKIM_WRAP"), Some("char")); + assert_eq!(get("FZF_WRAP"), Some("char")); + assert_eq!(get("SKIM_INPUT_STATE"), Some("disabled")); + assert_eq!(get("SKIM_DIRECTION"), Some("down")); +} + +#[test] +fn child_env_exports_last_action_and_key() { + let mut app = app_with_items(&["a"]); + let get = |app: &App, key: &str| { + app.child_env() + .into_iter() + .find(|(name, _)| name == key) + .map(|(_, value)| value) + }; + + // Present but empty before any input, like fzf. + assert_eq!(get(&app, "SKIM_ACTION"), Some(String::new())); + assert_eq!(get(&app, "FZF_KEY"), Some(String::new())); + + app.handle_key(&KeyEvent::new(KeyCode::Char('b'), KeyModifiers::NONE)); + assert_eq!(get(&app, "SKIM_KEY"), Some("b".to_string())); + assert_eq!(get(&app, "FZF_KEY"), Some("b".to_string())); + + app.handle_action(&Action::Down(1)).unwrap(); + assert_eq!(get(&app, "SKIM_ACTION"), Some("down".to_string())); + assert_eq!(get(&app, "FZF_ACTION"), Some("down".to_string())); + + // Synthetic events routed through the keymap must not overwrite the key. + app.handle_key(&crate::binds::SkimEvent::Change.into()); + assert_eq!(get(&app, "SKIM_KEY"), Some("b".to_string())); +} + +#[test] +fn follow_up_action_overwrites_last_action() { + // `act-up:execute-silent(…)` dispatches `execute-silent` as a follow-up, so + // the child must see `ACTION=execute-silent`, not `up`. + let mut options = SkimOptions::default(); + options.bind = vec!["act-up:ignore".to_string()]; + let options = options.build(); + let mut app = App::from_options(options, Arc::new(crate::theme::ColorTheme::default()), String::new()); + + app.handle_action(&Action::Up(1)).unwrap(); + assert_eq!(app.last_action.as_deref(), Some("ignore")); +} + +#[test] +fn execute_captures_env_before_a_later_action_runs() { + // The `execute` event is handled later by the event loop, so the snapshot + // must be taken while `execute` is still the running action. + let mut app = app_with_items(&["a"]); + app.input.value = "kept".to_string(); + let events = app.handle_action(&Action::Execute("true".to_string())).unwrap(); + act(&mut app, Action::SetQuery("later".to_string())); + + let [Event::RunExecute(_, env, _)] = events.as_slice() else { + panic!("expected one RunExecute, got {events:?}"); + }; + let query = env + .iter() + .find(|(name, _)| name == "SKIM_QUERY") + .map(|(_, v)| v.clone()); + let action = env + .iter() + .find(|(name, _)| name == "SKIM_ACTION") + .map(|(_, v)| v.clone()); + assert_eq!(query.as_deref(), Some("kept")); + assert_eq!(action.as_deref(), Some("execute")); +} + +#[test] +fn queued_execute_actions_keep_snapshots_in_dispatch_order() { + let mut app = app_with_items(&["a"]); + let events = act( + &mut app, + Action::IfQueryEmpty("execute(true)+toggle+execute(true)".to_string(), None), + ); + let envs: Vec<_> = events + .iter() + .filter_map(|e| match e { + Event::RunExecute(_, env, _) => Some(env), + _ => None, + }) + .collect(); + assert_eq!(envs.len(), 2); + for (env, expected_count) in envs.into_iter().zip(["0", "1"]) { + for prefix in ["SKIM", "FZF"] { + let get = |suffix| { + env.iter() + .find(|(name, _)| name == &format!("{prefix}_{suffix}")) + .unwrap() + .1 + .as_str() + }; + assert_eq!(get("SELECT_COUNT"), expected_count); + assert_eq!(get("ACTION"), "execute"); + } + } +} + +#[cfg(unix)] +#[test] +fn execute_silent_exports_environment() { + use std::time::{Duration, Instant}; + + let output = tempfile::NamedTempFile::new().unwrap(); + let mut app = app_with_items(&["a"]); + app.input.value = "kept".to_string(); + act( + &mut app, + Action::ExecuteSilent(format!( + "printf '%s:%s:%s' \"$SKIM_QUERY\" \"$FZF_QUERY\" \"$SKIM_ACTION\" > {}", + shlex::try_quote(output.path().to_str().unwrap()).unwrap(), + )), + ); + let started = Instant::now(); + loop { + if std::fs::read_to_string(output.path()).unwrap() == "kept:kept:execute-silent" { + break; + } + assert!( + started.elapsed() < Duration::from_secs(2), + "execute-silent environment not received" + ); + std::thread::sleep(Duration::from_millis(10)); + } +} + +#[test] +fn child_env_unset_lists_omitted_names() { + let app = app_with_items(&["a"]); + let unset = app.child_env_unset(); + // No preview and no `--nth`: those groups are absent from the snapshot, so + // they must be removed from the inherited environment. + for name in [ + "SKIM_PREVIEW_LINES", + "FZF_PREVIEW_TOP", + "SKIM_NTH", + "FZF_WITH_NTH", + "SKIM_WRAP", + ] { + assert!(unset.iter().any(|n| n == name), "{name} should be unset"); + } + // Exported names are not in the unset list. + for name in ["SKIM_QUERY", "FZF_QUERY", "SKIM_CURRENT_ITEM", "FZF_KEY"] { + assert!(!unset.iter().any(|n| n == name), "{name} should be kept"); + } + // Every name is either exported or unset, never both and never neither. + let exported: Vec = app.child_env().into_iter().map(|(name, _)| name).collect(); + for name in App::child_env_names() { + assert_eq!( + exported.contains(&name), + !unset.contains(&name), + "{name} must be in exactly one of exported/unset" + ); + } +} + +#[test] +fn child_env_omits_oversized_selected_items() { + let mut app = app_with_items(&["a", "b"]); + app.item_list.selection.insert(crate::item::MatchedItem::new( + Arc::new("x".repeat(70 * 1024)), + crate::Rank::default(), + None, + &crate::item::RankBuilder::default(), + )); + let env = app.child_env(); + assert!(!env.iter().any(|(name, _)| name == "SKIM_SELECTED_ITEMS")); + assert!(app.child_env_unset().iter().any(|n| n == "FZF_SELECTED_ITEMS")); +} + +#[test] +fn child_env_join_selected_items_with_newlines() { + let mut app = app_with_items(&["a", "b", "c"]); + app.item_list.select_row(0); + app.item_list.select_row(2); + let env = app.child_env(); + let selected = env + .iter() + .find(|(name, _)| name == "SKIM_SELECTED_ITEMS") + .map(|(_, value)| value.as_str()); + assert_eq!(selected, Some("a\nc")); +} + +#[test] +fn child_env_uses_empty_and_zero_when_nothing_selected() { + let app = App::default(); + let env = app.child_env(); + let get = |key: &str| { + env.iter() + .find(|(name, _)| name == key) + .map(|(_, value)| value.as_str()) + }; + assert_eq!(get("SKIM_CURRENT_ITEM"), None); + assert_eq!(get("SKIM_SELECTED_ITEMS"), Some("")); + assert_eq!(get("SKIM_SELECT_COUNT"), Some("0")); + assert_eq!(get("SKIM_TOTAL_COUNT"), Some("0")); + assert_eq!(get("SKIM_MATCH_COUNT"), Some("0")); + assert_eq!(get("SKIM_POS"), Some("0")); +} + +#[test] +fn child_env_value_size_limit_counts_bytes_without_truncation() { + for (value, exported) in [ + ("x".repeat(MAX_ENV_VALUE_BYTES), true), + ("x".repeat(MAX_ENV_VALUE_BYTES + 1), false), + ("é".repeat(MAX_ENV_VALUE_BYTES / 2), true), + ("é".repeat(MAX_ENV_VALUE_BYTES / 2 + 1), false), + ] { + let mut app = app_with_items(&[&value]); + app.item_list.select_row(0); + app.input.value.clone_from(&value); + let (env, unset) = app.child_env_snapshot(); + for prefix in ["SKIM", "FZF"] { + for suffix in ["QUERY", "CURRENT_ITEM", "SELECTED_ITEMS"] { + let name = format!("{prefix}_{suffix}"); + let actual = env.iter().find(|(key, _)| key == &name).map(|(_, v)| v); + assert_eq!(actual, exported.then_some(&value), "{name}"); + assert_eq!(unset.contains(&name), !exported, "{name}"); + } + } + } +} + +#[test] +fn child_env_strips_ansi_from_current_and_selected_items() { + let mut app = app_with_items(&["\x1b[31mred\x1b[0m"]); + app.item_list.select_row(0); + let env = app.child_env(); + for prefix in ["SKIM", "FZF"] { + for suffix in ["CURRENT_ITEM", "SELECTED_ITEMS"] { + let name = format!("{prefix}_{suffix}"); + assert_eq!(env.iter().find(|(key, _)| key == &name).unwrap().1, "red"); + } + } +} + +#[test] +fn reload_actions_capture_selection_and_expand_before_clearing() { + for action in [ + Action::Reload(Some("echo {+}".to_string())), + Action::Reload(None), + Action::SetCmd("echo {+}".to_string()), + ] { + let mut app = app_with_items(&["alpha", "beta"]); + app.item_list.select_row(0); + app.item_list.select_row(1); + app.cmd = "echo default".to_string(); + let events = act(&mut app, action.clone()); + let [Event::Reload(cmd)] = events.as_slice() else { + panic!("expected reload, got {events:?}"); + }; + if !matches!(action, Action::Reload(None)) { + assert!(cmd.contains("alpha") && cmd.contains("beta"), "{cmd}"); + } + assert!(app.item_list.selection.is_empty()); + let (env, _) = app.pending_reload_env.pop_front().unwrap(); + for prefix in ["SKIM", "FZF"] { + let get = |suffix| { + &env.iter() + .find(|(key, _)| key == &format!("{prefix}_{suffix}")) + .unwrap() + .1 + }; + assert_eq!(get("SELECTED_ITEMS"), "alpha\nbeta"); + assert_eq!(get("SELECT_COUNT"), "2"); + assert_eq!(get("ACTION"), action.name()); + } + } +} + +#[test] +fn queued_reloads_keep_the_query_at_dispatch() { + let mut app = app_with_items(&["alpha"]); + app.options.interactive = true; + app.options.cmd = Some("echo {}".to_string()); + app.cmd = "echo {}".to_string(); + act(&mut app, Action::SetQuery("first".to_string())); + act(&mut app, Action::SetQuery("second".to_string())); + for expected in ["first", "second"] { + let (env, _) = app.pending_reload_env.pop_front().unwrap(); + assert_eq!(env.iter().find(|(key, _)| key == "SKIM_QUERY").unwrap().1, expected); + } + assert!(app.pending_reload_env.is_empty()); +} + +#[test] +fn child_env_omits_values_exec_cannot_carry() { + let mut app = app_with_items(&["a\0b"]); + app.item_list.select_row(0); + let env = app.child_env(); + let get = |key: &str| { + env.iter() + .find(|(name, _)| name == key) + .map(|(_, value)| value.as_str()) + }; + // The NUL-containing item text is skipped on both prefixes... + assert_eq!(get("SKIM_CURRENT_ITEM"), None); + assert_eq!(get("FZF_CURRENT_ITEM"), None); + assert_eq!(get("SKIM_SELECTED_ITEMS"), None); + // ...but the NUL-free variables are still exported. + assert_eq!(get("SKIM_QUERY"), Some("")); + assert_eq!(get("FZF_SELECT_COUNT"), Some("1")); +} + #[test] fn calculate_preview_offset_variants() { let app = App::default(); @@ -1927,6 +2315,57 @@ fn run_preview_positioned_text_neg_and_percent_offsets() { assert_eq!(app.preview.scroll_x, 47); } +#[cfg(unix)] +#[test] +fn all_command_preview_variants_receive_state_without_shell_expansion() { + struct EnvPreviewItem(String, bool); + impl SkimItem for EnvPreviewItem { + fn text(&self) -> std::borrow::Cow<'_, str> { + "item".into() + } + fn preview(&self, _: PreviewContext) -> ItemPreview { + if self.1 { + ItemPreview::CommandWithPos(self.0.clone(), crate::PreviewPosition::default()) + } else { + ItemPreview::Command(self.0.clone()) + } + } + } + + for variant in 0..3 { + let output = tempfile::NamedTempFile::new().unwrap(); + let cmd = format!( + "printf '%s|%s|%s' \"$SKIM_QUERY\" \"$FZF_QUERY\" \"$SKIM_CURRENT_ITEM\" > {}", + shlex::try_quote(output.path().to_str().unwrap()).unwrap(), + ); + let item: Arc = if variant == 0 { + Arc::new("item".to_string()) + } else { + Arc::new(EnvPreviewItem(cmd.clone(), variant == 2)) + }; + let mut app = app_with_one(item); + app.options.preview = Some(cmd); + app.input.value = "é ' \" $HOME; $(false)\nline".to_string(); + app.last_preview_spawn = past_instant(Duration::from_secs(1)); + let expected = format!("{0}|{0}|item", app.input.value); + let mut tui = test_tui(); + app.run_preview(&mut tui).unwrap(); + let started = Instant::now(); + let received = loop { + if std::fs::read_to_string(output.path()).unwrap() == expected { + break true; + } + if started.elapsed() >= Duration::from_secs(2) { + break false; + } + std::thread::sleep(Duration::from_millis(10)); + }; + app.preview.kill(); + app.preview.thread_handle.take().unwrap().join().unwrap(); + assert!(received, "preview variant {variant} did not receive the snapshot"); + } +} + /// A [`SkimItem`] whose preview runs a shell command. #[derive(Debug)] struct CommandPreviewItem; diff --git a/src/tui/backend.rs b/src/tui/backend.rs index abc056c52..8efa44a0f 100644 --- a/src/tui/backend.rs +++ b/src/tui/backend.rs @@ -381,7 +381,7 @@ where /// screen and raw mode, runs the command to completion, then restores skim's /// terminal state and restarts the reader. The child is given its own handle /// to the controlling terminal as stdin (see [`execute_child_stdin`]). - pub(crate) fn run_execute(&mut self, cmd: &str) -> Result<()> { + pub(crate) fn run_execute(&mut self, cmd: &str, env: &[(String, String)], unset: &[String]) -> Result<()> { use std::io::IsTerminal as _; let has_tty = std::io::stderr().is_terminal(); @@ -396,6 +396,7 @@ where } let mut command = crate::shell_cmd(cmd); + super::util::apply_child_env(&mut command, env, unset); command.stdin(execute_child_stdin()); let _ = command.spawn().and_then(|mut c| c.wait()); diff --git a/src/tui/event.rs b/src/tui/event.rs index 2b1bf14fd..e48dc7c66 100644 --- a/src/tui/event.rs +++ b/src/tui/event.rs @@ -44,8 +44,10 @@ pub enum Event { /// Carries the already-expanded command line. Handled by the TUI event /// loop (which has access to the [`Tui`](crate::tui::Tui)) rather than by /// `handle_action`, because running a foreground process requires - /// suspending skim's own input reader and toggling terminal modes. - RunExecute(String), + /// suspending skim's own input reader and toggling terminal modes. Also + /// carries the environment pairs to export and the names to unset, captured + /// when the action was dispatched. + RunExecute(String, Vec<(String, String)>, Vec), /// Redraw the screen Redraw, /// Reload with a new command diff --git a/src/tui/preview.rs b/src/tui/preview.rs index d057d1df6..0ba99d79c 100644 --- a/src/tui/preview.rs +++ b/src/tui/preview.rs @@ -591,7 +591,13 @@ impl Preview { } #[allow(clippy::too_many_lines)] - pub fn spawn(&mut self, tui: &mut Tui, cmd: &str) -> Result<()> + pub fn spawn( + &mut self, + tui: &mut Tui, + cmd: &str, + env: &[(String, String)], + unset: &[String], + ) -> Result<()> where B::Error: Send + Sync + 'static, { @@ -664,6 +670,12 @@ impl Preview { shell_cmd.env("ROWS", self.rows.to_string()); shell_cmd.env("COLUMNS", self.cols.to_string()); shell_cmd.env("PAGER", ""); + for name in unset { + shell_cmd.env_remove(name); + } + for (key, value) in env { + shell_cmd.env(key, value); + } shell_cmd.arg("-c"); if let Ok(cwd) = env::current_dir() { shell_cmd.cwd(cwd); @@ -758,6 +770,7 @@ impl Preview { } else { trace!("spawning preview cmd {cmd}"); let mut shell_cmd = crate::shell_cmd(cmd); + crate::tui::util::apply_child_env(&mut shell_cmd, env, unset); shell_cmd .env("ROWS", self.rows.to_string()) .env("COLUMNS", self.cols.to_string()) diff --git a/src/tui/preview_tests.rs b/src/tui/preview_tests.rs index a0413ce45..9adb002b1 100644 --- a/src/tui/preview_tests.rs +++ b/src/tui/preview_tests.rs @@ -108,7 +108,7 @@ fn plain_preview_streams_before_command_exits() { let mut tui = super::super::Tui::new_with_height_and_backend(TestBackend::new(20, 5), super::super::Size::Percent(100)) .unwrap(); - preview.spawn(&mut tui, "printf streamed; sleep 30").unwrap(); + preview.spawn(&mut tui, "printf streamed; sleep 30", &[], &[]).unwrap(); let started = Instant::now(); let streamed_in_time = loop { @@ -126,6 +126,89 @@ fn plain_preview_streams_before_command_exits() { assert!(streamed_in_time, "preview output did not stream"); } +#[cfg(unix)] +#[test] +fn plain_preview_inherits_extra_env() { + use std::time::{Duration, Instant}; + + use ratatui::backend::TestBackend; + + let mut preview = Preview::default(); + preview.pty = None; + let mut tui = + super::super::Tui::new_with_height_and_backend(TestBackend::new(40, 5), super::super::Size::Percent(100)) + .unwrap(); + let env = vec![ + ("SKIM_QUERY".to_string(), "hello".to_string()), + ("FZF_QUERY".to_string(), "hello".to_string()), + ]; + preview + .spawn(&mut tui, "printf '%s-%s' \"$SKIM_QUERY\" \"$FZF_QUERY\"", &env, &[]) + .unwrap(); + + let started = Instant::now(); + let got_env = loop { + if preview_contains(&preview, "hello-hello") { + break true; + } + if started.elapsed() >= Duration::from_secs(2) { + break false; + } + std::thread::sleep(Duration::from_millis(10)); + }; + + preview.kill(); + preview.thread_handle.take().unwrap().join().unwrap(); + assert!(got_env, "preview child did not inherit the exported env vars"); +} + +#[cfg(unix)] +#[test] +fn pty_preview_applies_environment_and_removes_inherited_values() { + use std::time::{Duration, Instant}; + + use ratatui::backend::TestBackend; + + assert!(std::env::var_os("HOME").is_some(), "test needs an inherited HOME"); + let output = tempfile::NamedTempFile::new().unwrap(); + let mut preview = Preview::default(); + preview.rows = 5; + preview.cols = 40; + preview.init_pty(); + assert!(preview.pty.is_some(), "test needs a PTY"); + let mut tui = + super::super::Tui::new_with_height_and_backend(TestBackend::new(40, 5), super::super::Size::Percent(100)) + .unwrap(); + let cmd = format!( + "printf '%s:%s:%s' \"$SKIM_QUERY\" \"$FZF_QUERY\" \"${{HOME-unset}}\" > {}", + shlex::try_quote(output.path().to_str().unwrap()).unwrap(), + ); + preview + .spawn( + &mut tui, + &cmd, + &[ + ("SKIM_QUERY".to_string(), "hello".to_string()), + ("FZF_QUERY".to_string(), "hello".to_string()), + ], + &["HOME".to_string(), "SKIM_QUERY".to_string()], + ) + .unwrap(); + let started = Instant::now(); + let got_env = loop { + if std::fs::read_to_string(output.path()).unwrap() == "hello:hello:unset" { + break true; + } + if started.elapsed() >= Duration::from_secs(2) { + break false; + } + std::thread::sleep(Duration::from_millis(10)); + }; + preview.kill(); + preview.thread_handle.take().unwrap().join().unwrap(); + assert!(got_env, "PTY preview did not apply the environment snapshot"); +} + #[cfg(unix)] #[test] fn stale_plain_preview_cannot_replace_newer_streamed_output() { @@ -139,10 +222,10 @@ fn stale_plain_preview_cannot_replace_newer_streamed_output() { super::super::Tui::new_with_height_and_backend(TestBackend::new(20, 5), super::super::Size::Percent(100)) .unwrap(); - preview.spawn(&mut tui, "printf stale; sleep 30").unwrap(); + preview.spawn(&mut tui, "printf stale; sleep 30", &[], &[]).unwrap(); let stale_cancelled = preview.plain_cancelled.as_ref().unwrap().clone(); let stale_thread = preview.thread_handle.take().unwrap(); - preview.spawn(&mut tui, "printf current; sleep 30").unwrap(); + preview.spawn(&mut tui, "printf current; sleep 30", &[], &[]).unwrap(); let started = Instant::now(); let current_streamed = loop { @@ -176,7 +259,7 @@ fn plain_preview_can_be_cancelled() { let mut tui = super::super::Tui::new_with_height_and_backend(TestBackend::new(20, 5), super::super::Size::Percent(100)) .unwrap(); - preview.spawn(&mut tui, "sleep 30").unwrap(); + preview.spawn(&mut tui, "sleep 30", &[], &[]).unwrap(); let started = Instant::now(); preview.kill(); diff --git a/src/tui/util.rs b/src/tui/util.rs index d61918a82..aecd9f215 100644 --- a/src/tui/util.rs +++ b/src/tui/util.rs @@ -432,6 +432,19 @@ pub(crate) fn cursor_pos_from_tty() -> io::Result<(u16, u16)> { crossterm::cursor::position().map(|(x, y)| (x + 1, y + 1)) } +/// Applies a child-process environment snapshot to `command`. +/// +/// Every name in `unset` is first removed from the inherited environment: a +/// skipped pair does not unset an inherited variable, so a skim started inside +/// fzf would otherwise pass the enclosing process's stale `FZF_*` values to its +/// children. The pairs in `env` are then set. +pub(crate) fn apply_child_env(command: &mut std::process::Command, env: &[(String, String)], unset: &[String]) { + for name in unset { + command.env_remove(name); + } + command.envs(env.iter().cloned()); +} + #[cfg(test)] #[path = "util_tests.rs"] mod tests; diff --git a/src/tui/util_tests.rs b/src/tui/util_tests.rs index 0e45559fe..7b5fd293b 100644 --- a/src/tui/util_tests.rs +++ b/src/tui/util_tests.rs @@ -412,3 +412,22 @@ fn test_style_span_and_line() { style_line(&mut line, red); assert!(line.spans.iter().all(|s| s.style.fg == Some(Color::Red))); } + +#[test] +fn apply_child_env_unsets_then_sets() { + // Inherited stale values are removed before the snapshot is applied, so an + // omitted variable is not silently passed through to the child. + let mut command = std::process::Command::new("unused"); + command.env("SKIM_TEST_INHERITED", "stale"); + apply_child_env( + &mut command, + &[("SKIM_TEST_SET".to_string(), "fresh".to_string())], + &["SKIM_TEST_INHERITED".to_string()], + ); + let env: Vec<_> = command.get_envs().collect(); + assert!(env.contains(&( + std::ffi::OsStr::new("SKIM_TEST_SET"), + Some(std::ffi::OsStr::new("fresh")), + ))); + assert!(env.contains(&(std::ffi::OsStr::new("SKIM_TEST_INHERITED"), None))); +} diff --git a/tests/execute.rs b/tests/execute.rs index fcbab7057..42a1f73a5 100644 --- a/tests/execute.rs +++ b/tests/execute.rs @@ -125,3 +125,31 @@ fn execute_interactive_child_keeps_receiving_keys_fullscreen() -> Result<()> { fn execute_interactive_child_keeps_receiving_keys_inline() -> Result<()> { run_interactive_execute("execute_interactive_inline", &["--height=40%"]) } + +/// `execute` child processes inherit the exported state env vars (issue #1195). +#[test] +fn execute_child_inherits_skim_env() -> Result<()> { + let mut tmux = ZellijController::new_named("execute_env")?; + let out = tmux.tempdir.path().join("env.txt"); + // The bind command runs through skim's `sh -c`, so `$SKIM_*`/`$FZF_*` are + // expanded by that shell, not by skim's placeholder logic. + let bind = format!( + "--bind='enter:execute(printf %s \"$SKIM_QUERY|$FZF_QUERY|$SKIM_CURRENT_ITEM|$SKIM_POS\" > {})'", + out.display() + ); + + tmux.start_sk(Some("printf 'aaa\\nbbb'"), &[bind.as_str()])?; + tmux.until(|l| l.iter().any(|s| s.trim_start().starts_with('>')))?; + + // Type a query so the env vars carry non-default values, then execute. + tmux.send_keys(&[Str("bb")])?; + tmux.send_keys(&[Enter])?; + + wait(|| match read_file(&out) { + Ok(s) if s == "bb|bb|bbb|1" => Ok(()), + Ok(s) => Err(std::io::Error::other(format!("unexpected env output: {s:?}"))), + Err(_) => Err(std::io::Error::other("env file not written yet")), + })?; + + Ok(()) +} diff --git a/tests/preview.rs b/tests/preview.rs index aaac7f244..2706e3fcc 100644 --- a/tests/preview.rs +++ b/tests/preview.rs @@ -58,7 +58,28 @@ insta_test!(preview_navigation, ["a", "b", "c"], &["--preview", &common::echo_co @snap; }); +#[cfg(unix)] +insta_test!( + preview_env_vars, + ["aaa", "bbb"], + &[ + "--preview", + "echo Q=$SKIM_QUERY FQ=$FZF_QUERY C=$SKIM_CURRENT_ITEM M=$SKIM_MATCH_COUNT S=$SKIM_SELECT_COUNT D=$SKIM_DIRECTION IS=$SKIM_INPUT_STATE P=$SKIM_PROMPT PT=$SKIM_POINTER W=$SKIM_WRAP N=$SKIM_NTH A=$SKIM_ACTION K=$SKIM_KEY T=$SKIM_PREVIEW_TOP L=$SKIM_PREVIEW_LEFT PL=$SKIM_PREVIEW_LINES PC=$SKIM_PREVIEW_COLUMNS", + "--preview-window", + "up:wrap", + "--wrap", + "--nth", + "1" + ], + { + @snap; + @type "bbb"; + @snap; + } +); + insta_test!(preview_plus, ["a", "b", "c"], &["--preview", &common::echo_command("{+}"), "-m"], { + @snap; @key Up; @snap; diff --git a/tests/snapshots/preview__preview_env_vars@001.snap b/tests/snapshots/preview__preview_env_vars@001.snap new file mode 100644 index 000000000..8693e2a4d --- /dev/null +++ b/tests/snapshots/preview__preview_env_vars@001.snap @@ -0,0 +1,29 @@ +--- +source: tests/preview.rs +description: "input: items [\"aaa\", \"bbb\"]\noptions: --preview echo Q=$SKIM_QUERY FQ=$FZF_QUERY C=$SKIM_CURRENT_ITEM M=$SKIM_MATCH_COUNT S=$SKIM_SELECT_COUNT D=$SKIM_DIRECTION IS=$SKIM_INPUT_STATE P=$SKIM_PROMPT PT=$SKIM_POINTER W=$SKIM_WRAP N=$SKIM_NTH A=$SKIM_ACTION K=$SKIM_KEY T=$SKIM_PREVIEW_TOP L=$SKIM_PREVIEW_LEFT PL=$SKIM_PREVIEW_LINES PC=$SKIM_PREVIEW_COLUMNS --preview-window up:wrap --wrap --nth 1" +--- +"Q= FQ= C=aaa M=2 S=0 D=up IS=enabled P=> PT=> W=char N=1 A= K= T=0 L=0 PL=12 " +"PC=80 " +" " +" " +" " +" " +" " +" " +" " +" " +" " +"────────────────────────────────────────────────────────────────────────────────" +" " +" " +" " +" " +" " +" " +" " +" " +" bbb " +"> aaa " +" 2/2 0/0" +"> " +cursor: (24, 3) diff --git a/tests/snapshots/preview__preview_env_vars@002.snap b/tests/snapshots/preview__preview_env_vars@002.snap new file mode 100644 index 000000000..69c97b351 --- /dev/null +++ b/tests/snapshots/preview__preview_env_vars@002.snap @@ -0,0 +1,29 @@ +--- +source: tests/preview.rs +description: "input: items [\"aaa\", \"bbb\"]\noptions: --preview echo Q=$SKIM_QUERY FQ=$FZF_QUERY C=$SKIM_CURRENT_ITEM M=$SKIM_MATCH_COUNT S=$SKIM_SELECT_COUNT D=$SKIM_DIRECTION IS=$SKIM_INPUT_STATE P=$SKIM_PROMPT PT=$SKIM_POINTER W=$SKIM_WRAP N=$SKIM_NTH A=$SKIM_ACTION K=$SKIM_KEY T=$SKIM_PREVIEW_TOP L=$SKIM_PREVIEW_LEFT PL=$SKIM_PREVIEW_LINES PC=$SKIM_PREVIEW_COLUMNS --preview-window up:wrap --wrap --nth 1\nafter:\n @type \"bbb\"" +--- +"Q=bbb FQ=bbb C=bbb M=1 S=0 D=up IS=enabled P=> PT=> W=char N=1 A=add-char K=b " +"T=0 L=0 PL=12 PC=80 " +" " +" " +" " +" " +" " +" " +" " +" " +" " +"────────────────────────────────────────────────────────────────────────────────" +" " +" " +" " +" " +" " +" " +" " +" " +" " +"> bbb " +" 1/2 0/0" +"> bbb " +cursor: (24, 6)