diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 004ff78f..40501480 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -273,7 +273,14 @@ jobs: tool: cargo-public-api@0.52 - run: rustup toolchain install nightly-x86_64-unknown-linux-gnu name: Install nightly toolchain - - run: cargo public-api diff latest --deny removed --deny changed + # Compare two repository revisions so cargo-public-api uses their committed + # lockfiles. A registry comparison resolves the released crate again and can + # fail when a new, broken transitive dependency is published. + - run: git fetch --force --tags + name: Fetch release tags + - run: | + latest_tag="$(git tag --list 'v*' --sort=-version:refname | head -n1)" + cargo public-api diff "${latest_tag}..HEAD" --deny removed --deny changed name: Run cargo-public-api diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6791fe0b..57de0c1f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -627,7 +627,8 @@ Interruption is cooperative: each chunk checks `interrupt.load(Relaxed)` before - A `tokio::sync::mpsc` channel (`event_tx` / `event_rx`) of capacity 1 M for events. - A `JoinHandle` for a background Tokio task that reads `crossterm::event::EventStream` and sends `Event` values. - A `CancellationToken` to stop the background task. -- A `is_fullscreen` flag that determines the `ratatui::Viewport`. +- An `is_fullscreen` flag that determines the `ratatui::Viewport`. +- The fixed viewport `Rect` for inline mode, which can move when the terminal scrolls. **Viewport selection** (`Tui::new_with_height_and_backend()`): @@ -636,7 +637,7 @@ Interruption is cooperative: each chunk checks `interrupt.load(Relaxed)` before - `Size::Percent(p)` → fixed viewport with `terminal_height * p / 100` rows. - `Size::Neg(lines)` → fixed viewport with `terminal_height - lines` rows, saturating at zero. -Any fixed viewport is anchored at the current cursor position; the terminal is scrolled if needed to make room. +Any fixed viewport is anchored at the current cursor position; the terminal is scrolled if needed to make room. After construction, `Tui::min_height()` can increase an inline viewport to `--min-height`. It limits the height to the terminal height and scrolls the terminal before it moves and resizes the viewport when there are too few rows below it. The default backend is `CrosstermBackend>`. Skim always draws to **stderr** so stdout remains clean for piped output. @@ -1314,24 +1315,24 @@ The global allocator is `mimalloc` (v3), chosen for its low-latency multi-thread | Call site | File | What it does | | --- | --- | --- | -| `Skim::run_with` | `src/skim.rs:70` | Top-level library entry point | -| `Skim::run_items` | `src/skim.rs:112` | Convenience wrapper for iterator inputs | -| `Skim::init_tui` | `src/skim.rs:136` | Initialize default crossterm TUI backend | -| `Skim::init` | `src/skim.rs:155` | Constructs all subsystems from options | -| `Skim::start` | `src/skim.rs:199` | Starts reader + initial matcher pass | -| `Skim::handle_reload` | `src/skim.rs:231` | Kills reader, clears pool, restarts | -| `Skim::init_tui_with` | `src/skim.rs:303` | Install a caller-provided TUI backend | -| `Skim::enter` | `src/skim.rs:390` | Enter terminal, resolve image picker, start listener/event pump | -| `Skim::should_enter` | `src/skim.rs:434` | Filter/select-1/exit-0/sync gate | -| `Skim::output` | `src/skim.rs:539` | Collect & return SkimOutput | -| `Skim::tick` | `src/skim.rs:620` | Single async event loop iteration | +| `Skim::run_with` | `src/skim.rs:72` | Top-level library entry point | +| `Skim::run_items` | `src/skim.rs:114` | Convenience wrapper for iterator inputs | +| `Skim::init_tui` | `src/skim.rs:138` | Initialize default crossterm TUI backend | +| `Skim::init` | `src/skim.rs:159` | Constructs all subsystems from options | +| `Skim::start` | `src/skim.rs:203` | Starts reader + initial matcher pass | +| `Skim::handle_reload` | `src/skim.rs:235` | Kills reader, clears pool, restarts | +| `Skim::init_tui_with` | `src/skim.rs:307` | Install a caller-provided TUI backend | +| `Skim::enter` | `src/skim.rs:394` | Enter terminal, resolve image picker, start listener/event pump | +| `Skim::should_enter` | `src/skim.rs:438` | Filter/select-1/exit-0/sync gate | +| `Skim::output` | `src/skim.rs:555` | Collect & return SkimOutput | +| `Skim::tick` | `src/skim.rs:636` | Single async event loop iteration | | `App::from_options` | `src/tui/app.rs:289` | Build all widgets from options | | `App::run_preview` | `src/tui/app.rs:503` | Expand cmd, debounce, call Preview::spawn | | `App::handle_event` | `src/tui/app.rs:628` | Dispatch all Event variants | | `App::handle_action` | `src/tui/app.rs:833` | Apply action follow-up bindings | | `App::dispatch_conditional` | `src/tui/app.rs:852` | Dispatch the selected conditional subaction chain without follow-up bindings | | `App::dispatch_action` | `src/tui/app.rs:875` | Dispatch one Action variant without follow-up bindings | -| `Tui::run_execute` | `src/tui/backend.rs:353` | Suspend reader, run `execute` child with its own tty stdin, restart reader | +| `Tui::run_execute` | `src/tui/backend.rs:363` | Suspend reader, run `execute` child with its own tty stdin, restart reader | | `App::restart_matcher` | `src/tui/app.rs:1352` | Kill old match pass, start new one | | `App::expand_cmd` | `src/tui/app.rs:1428` | Substitute `{}`, `{q}`, `{n}` etc. | | `App::handle_mouse` | `src/tui/app.rs:1496` | Handle mouse behavior and emit `double-click` | @@ -1345,11 +1346,12 @@ The global allocator is `mimalloc` (v3), chosen for its low-latency multi-thread | `spawn_io_reader` | `src/helper/item_reader.rs:378` | I/O reader thread: chunk reads + line splitting | | `spawn_reorder_thread` | `src/helper/item_reader.rs:483` | Reorder thread: ordered output + pipeline-done signal | | `Preview::spawn` | `src/tui/preview.rs:319` | Start image, PTY, or plain preview worker | -| `Tui::new_with_height_and_backend` | `src/tui/backend.rs:78` | Terminal init + viewport sizing | -| `Tui::enter` | `src/tui/backend.rs:127` | Enable raw mode + terminal setup | -| `Tui::start` | `src/tui/backend.rs:238` | Spawn crossterm EventStream task (fresh cancellation token each call) | -| `Tui::stop_and_join` | `src/tui/backend.rs:224` | Cancel event pump and block until `EventStream` is dropped (before `execute`) | -| `Tui::force_full_redraw` | `src/tui/backend.rs:205` | Reset ratatui diff buffers for a full repaint with no cursor query (after `execute`) | +| `Tui::new_with_height_and_backend` | `src/tui/backend.rs:81` | Terminal init + viewport sizing | +| `Tui::enter` | `src/tui/backend.rs:134` | Enable raw mode + terminal setup | +| `Tui::start` | `src/tui/backend.rs:235` | Spawn crossterm EventStream task (fresh cancellation token each call) | +| `Tui::stop_and_join` | `src/tui/backend.rs:221` | Cancel event pump and block until `EventStream` is dropped (before `execute`) | +| `Tui::force_full_redraw` | `src/tui/backend.rs:202` | Reset ratatui diff buffers for a full repaint with no cursor query (after `execute`) | +| `Tui::min_height` | `src/tui/backend.rs:400` | Grow an inline viewport and scroll the terminal when needed | | `popup::run_with` | `src/popup/mod.rs:86` | Delegate to multiplexer popup + parse output | | `popup::check_env` | `src/popup/mod.rs:72` | Guard: multiplexer present and not already in popup | | `check_and_run_popup` | `src/bin/main.rs:131` | Check popup conditions, dispatch to popup::run_with | diff --git a/Cargo.toml b/Cargo.toml index 3ef41905..5dc327df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,9 +4,7 @@ version = "5.6.6" authors = ["Loric ANDRE", "Zhang Jinzhou "] description = "Fuzzy Finder in rust!" documentation = "https://docs.rs/skim" -homepage = "https://github.com/skim-rs/skim" repository = "https://github.com/skim-rs/skim" -readme = "README.md" keywords = ["fuzzy", "menu", "util"] license = "MIT" edition = "2024" diff --git a/man/man1/sk.1 b/man/man1/sk.1 index b98c90b8..8bdfd471 100644 --- a/man/man1/sk.1 +++ b/man/man1/sk.1 @@ -346,10 +346,11 @@ Can either be a row count or a percentage A negative row count will use term hei Disable height (force full screen) .TP \fB\-\-min\-height\fR \fI\fR [default: 10] -Minimum height of skim\*(Aqs window +Minimum height of skim\*(Aqs window as a non\-negative row count -Useful when the height is set as a percentage -Ignored when \-\-height is not specified +Must be a non\-negative row count, not a percentage. +Useful when the height is set as a percentage. +Ignored when \-\-height is not specified. .TP \fB\-\-margin\fR \fI\fR [default: 0] Screen margin diff --git a/shell/completion.fish b/shell/completion.fish index 26594e61..0523158d 100644 --- a/shell/completion.fish +++ b/shell/completion.fish @@ -38,7 +38,7 @@ complete -c sk -l layout -d 'Set layout' -r -f -a "default\t'Display from the bo reverse\t'Display from the top of the screen' reverse-list\t'Display from the top of the screen, prompt at the bottom'" complete -c sk -l height -d 'Height of skim\'s window' -r -complete -c sk -l min-height -d 'Minimum height of skim\'s window' -r +complete -c sk -l min-height -d 'Minimum height of skim\'s window as a non-negative row count' -r complete -c sk -l margin -d 'Screen margin' -r complete -c sk -s p -l prompt -d 'Set prompt' -r complete -c sk -l cmd-prompt -d 'Set prompt in command mode' -r diff --git a/shell/completion.nu b/shell/completion.nu index 8f2a3992..2db1921d 100644 --- a/shell/completion.nu +++ b/shell/completion.nu @@ -79,7 +79,7 @@ module completions { --reverse # Shorthand for reverse layout --height: string # Height of skim's window --no-height # Disable height (force full screen) - --min-height: string # Minimum height of skim's window + --min-height: string # Minimum height of skim's window as a non-negative row count --margin: string # Screen margin --prompt(-p): string # Set prompt --cmd-prompt: string # Set prompt in command mode diff --git a/shell/completion.zsh b/shell/completion.zsh index eb57378f..602b3d55 100644 --- a/shell/completion.zsh +++ b/shell/completion.zsh @@ -49,7 +49,7 @@ history\:"History scheme\: will force index as the first tiebreak"))' \ reverse\:"Display from the top of the screen" reverse-list\:"Display from the top of the screen, prompt at the bottom"))' \ '--height=[Height of skim'\''s window]:HEIGHT:_default' \ -'--min-height=[Minimum height of skim'\''s window]:MIN_HEIGHT:_default' \ +'--min-height=[Minimum height of skim'\''s window as a non-negative row count]:MIN_HEIGHT:_default' \ '--margin=[Screen margin]:MARGIN:_default' \ '-p+[Set prompt]:PROMPT:_default' \ '--prompt=[Set prompt]:PROMPT:_default' \ diff --git a/src/options.rs b/src/options.rs index d49c4d65..73695b81 100644 --- a/src/options.rs +++ b/src/options.rs @@ -23,6 +23,17 @@ use crate::tui::{BorderType, PreviewCallback}; use crate::util::read_file_lines; use crate::{CaseMatching, FuzzyAlgorithm, Selector, Typos}; +const MIN_HEIGHT_PARSE_ERROR: &str = "min-height needs to be a non-negative integer"; + +pub(crate) fn parse_min_height(s: &str) -> Result { + s.parse().map_err(|_| MIN_HEIGHT_PARSE_ERROR.to_string()) +} + +#[cfg(feature = "cli")] +fn parse_min_height_value(s: &str) -> Result { + parse_min_height(s).map(|_| s.to_string()) +} + #[cfg(feature = "cli")] /// Custom value parser for delimiter that handles escape sequences fn parse_delimiter_value(s: &str) -> Result { @@ -502,13 +513,21 @@ pub struct SkimOptions { #[cfg_attr(feature = "cli", arg(long, help_heading = "Layout"))] pub no_height: bool, - /// Minimum height of skim's window + /// Minimum height of skim's window as a non-negative row count /// - /// Useful when the height is set as a percentage - /// Ignored when --height is not specified + /// Must be a non-negative row count, not a percentage. + /// Useful when the height is set as a percentage. + /// Ignored when --height is not specified. #[cfg_attr( feature = "cli", - arg(long, default_value = "10", help_heading = "Layout", verbatim_doc_comment) + arg( + long, + default_value = "10", + help_heading = "Layout", + allow_hyphen_values = true, + value_parser = parse_min_height_value, + verbatim_doc_comment + ) )] pub min_height: String, diff --git a/src/options_tests.rs b/src/options_tests.rs index 4a225b38..6e6b5002 100644 --- a/src/options_tests.rs +++ b/src/options_tests.rs @@ -22,6 +22,50 @@ fn merge( .expect("options should parse") } +#[test] +fn min_height_accepts_a_non_negative_row_count() { + assert_eq!(super::parse_min_height("60"), Ok(60)); + assert_eq!( + SkimOptionsBuilder::default() + .min_height("60") + .build() + .unwrap() + .min_height, + "60" + ); +} + +#[test] +fn min_height_rejects_non_integers() { + for value in ["-1", "30%", "many"] { + assert_eq!( + super::parse_min_height(value), + Err("min-height needs to be a non-negative integer".to_string()) + ); + } +} + +#[cfg(feature = "cli")] +#[test] +fn cli_min_height_uses_custom_error() { + for value in ["-1", "30%"] { + let error = SkimOptions::merge_args_and_parse( + "sk".to_string(), + None, + None, + ["--min-height".to_string(), value.to_string()], + None, + ) + .expect_err("invalid min-height must fail"); + + assert!( + error + .to_string() + .contains("min-height needs to be a non-negative integer") + ); + } +} + #[test] fn merge_uses_skim_default_command_when_no_cmd_flag() { // SKIM_DEFAULT_COMMAND fills `cmd` when neither --cmd nor a pipe is given. diff --git a/src/skim.rs b/src/skim.rs index 35b48a15..1fa9521a 100644 --- a/src/skim.rs +++ b/src/skim.rs @@ -140,6 +140,8 @@ impl Skim { if self.app.options.no_mouse { tui.disable_mouse(); } + let min_height = crate::options::parse_min_height(&self.app.options.min_height).map_err(eyre::Report::msg)?; + tui.min_height(min_height)?; self.tui = Some(tui); Ok(()) } diff --git a/src/tui/backend.rs b/src/tui/backend.rs index a6ef2825..1f865dd2 100644 --- a/src/tui/backend.rs +++ b/src/tui/backend.rs @@ -42,6 +42,8 @@ where pub cancellation_token: CancellationToken, /// Whether running in fullscreen mode pub is_fullscreen: bool, + /// The terminal's rect (drawing) area, set if the layout is inline + rect: Option, enable_mouse: bool, } @@ -87,6 +89,7 @@ where Size::Neg(lines) => Some(term_height.saturating_sub(lines)), }; + let rect: Option; let viewport = if let Some(mut height) = lines { // Until https://github.com/crossterm-rs/crossterm/issues/919 is fixed, we need to do it ourselves let cursor_pos = cursor_pos_from_tty()?; @@ -97,13 +100,15 @@ where crossterm::execute!(stderr(), crossterm::terminal::ScrollUp(to_scroll))?; y = y.saturating_sub(to_scroll); } - Viewport::Fixed(Rect::new( + rect = Some(Rect::new( 0, y, backend.size().expect("Failed to get terminal width").width - 1, height, - )) + )); + Viewport::Fixed(rect.unwrap()) } else { + rect = None; Viewport::Fullscreen }; @@ -111,6 +116,7 @@ where Ok(Self { terminal: ratatui::Terminal::with_options(backend, TerminalOptions { viewport })?, task: None, + rect, event_rx: event_channel.1, event_tx: event_channel.0, tick_rate: f64::from(TICK_RATE), @@ -381,6 +387,46 @@ where self.start(); restore_result } + + /// Set the minimum height of an inline viewport. + /// + /// Scrolls the terminal when there are not enough rows below the viewport's + /// current origin. + /// + /// # Errors + /// + /// Returns an error if the terminal size cannot be read, the terminal cannot + /// be scrolled, or the viewport cannot be resized. + pub fn min_height(&mut self, min_height: u16) -> Result<()> { + if self.is_fullscreen { + return Ok(()); + } + + let Some(current_rect) = self.rect else { + return Ok(()); + }; + let terminal_height = self.backend().size()?.height; + let (rect, to_scroll) = rect_with_min_height(current_rect, min_height, terminal_height); + if rect == current_rect { + return Ok(()); + } + + if to_scroll > 0 { + crossterm::execute!(stderr(), crossterm::terminal::ScrollUp(to_scroll))?; + } + debug!("min_height: resizing TUI to {rect:?}"); + self.resize(rect)?; + self.rect = Some(rect); + Ok(()) + } +} + +fn rect_with_min_height(mut rect: Rect, min_height: u16, terminal_height: u16) -> (Rect, u16) { + rect.height = rect.height.max(min_height).min(terminal_height); + let lowest_origin = terminal_height.saturating_sub(rect.height); + let to_scroll = rect.y.saturating_sub(lowest_origin); + rect.y = rect.y.saturating_sub(to_scroll); + (rect, to_scroll) } impl Deref for Tui @@ -476,6 +522,14 @@ mod tests { .expect("failed to build test TUI") } + fn inline_tui(rect: Rect) -> Tui { + let mut tui = fullscreen_tui(); + tui.is_fullscreen = false; + tui.rect = Some(rect); + tui.resize(rect).expect("failed to set initial viewport"); + tui + } + #[test] fn new_with_full_height_is_fullscreen() { let tui = fullscreen_tui(); @@ -507,4 +561,50 @@ mod tests { assert_eq!(area.width, 80); assert_eq!(area.height, 24); } + + #[test] + fn min_height_resizes_and_scrolls_inline_terminal() { + let mut tui = inline_tui(Rect::new(0, 20, 79, 4)); + + tui.min_height(10).expect("failed to apply minimum height"); + + assert_eq!(tui.get_frame().area(), Rect::new(0, 14, 79, 10)); + assert_eq!(tui.rect, Some(Rect::new(0, 14, 79, 10))); + } + + #[test] + fn min_height_keeps_origin_when_rows_are_available() { + let rect = Rect::new(0, 5, 79, 4); + let (rect, to_scroll) = rect_with_min_height(rect, 10, 24); + + assert_eq!(rect, Rect::new(0, 5, 79, 10)); + assert_eq!(to_scroll, 0); + } + + #[test] + fn min_height_scrolls_to_make_room() { + let rect = Rect::new(0, 20, 79, 4); + let (rect, to_scroll) = rect_with_min_height(rect, 10, 24); + + assert_eq!(rect, Rect::new(0, 14, 79, 10)); + assert_eq!(to_scroll, 6); + } + + #[test] + fn min_height_is_limited_to_terminal_height() { + let rect = Rect::new(0, 20, 79, 4); + let (rect, to_scroll) = rect_with_min_height(rect, 30, 24); + + assert_eq!(rect, Rect::new(0, 0, 79, 24)); + assert_eq!(to_scroll, 20); + } + + #[test] + fn existing_height_is_limited_to_terminal_height() { + let rect = Rect::new(0, 0, 79, 30); + let (rect, to_scroll) = rect_with_min_height(rect, 10, 24); + + assert_eq!(rect, Rect::new(0, 0, 79, 24)); + assert_eq!(to_scroll, 0); + } } diff --git a/tests/interactive.rs b/tests/interactive.rs index 698f9aa8..b55d3053 100644 --- a/tests/interactive.rs +++ b/tests/interactive.rs @@ -6,6 +6,10 @@ #[allow(dead_code)] #[macro_use] mod common; +use std::io::Cursor; + +use skim::prelude::*; + use common::zellij::Keys::*; sk_test!(sk_version_long, "", &["--version"], { @@ -38,3 +42,69 @@ sk_test!(issue_1120_height_mode_clears_on_exit, @cmd "seq 1 10", &["--height=50% @keys Key('\x1b'); @lines |l| (!l.iter().any(|line| line.starts_with(">"))); }); + +sk_test!(min_height_grows_inline_viewport, @cmd "for i in {1..20}; do echo min-height-item-$i; done", &["--height=20%", "--min-height=10"], { + @lines |l| (l.iter().map(|line| line.matches("min-height-item-").count()).sum::() >= 7); + @keys Escape; +}); + +#[test] +fn library_builder_min_height_child() -> Result<(), Box> { + if std::env::var_os("SKIM_MIN_HEIGHT_BUILDER_CHILD").is_none() { + return Ok(()); + } + + let options = SkimOptionsBuilder::default().height("20%").min_height("10").build()?; + let items = SkimItemReader::default().of_bufread(Cursor::new( + (1..=20) + .map(|i| format!("builder-min-height-item-{i}")) + .collect::>() + .join("\n"), + )); + Skim::run_with(options, Some(items))?; + Ok(()) +} + +#[test] +fn library_builder_min_height_resizes_and_scrolls() -> Result<(), Box> { + let zellij = common::zellij::ZellijController::new_named("builderminheight")?; + zellij.send_keys(&[Str("printf '\\n%.0s' {1..22}"), Enter])?; + zellij.until(|lines| lines.first().is_some_and(|line| line.starts_with("skim$")))?; + + let test_binary = std::env::current_exe()?.to_string_lossy().replace('\\', "/"); + let test_binary = format!("'{}'", test_binary.replace('\'', "'\\''")); + let command = + format!("SKIM_MIN_HEIGHT_BUILDER_CHILD=1 {test_binary} --exact library_builder_min_height_child --nocapture"); + zellij.send_keys(&[Str(&command), Enter])?; + zellij.until(|lines| { + lines + .iter() + .map(|line| line.matches("builder-min-height-item-").count()) + .sum::() + >= 7 + })?; + zellij.send_keys(&[Escape])?; + zellij.until(|lines| lines.iter().any(|line| line.contains("test result: ok")))?; + Ok(()) +} + +#[test] +fn min_height_scrolls_when_cursor_is_near_terminal_bottom() -> std::io::Result<()> { + let mut zellij = common::zellij::ZellijController::new_named("minheightscroll")?; + zellij.send_keys(&[Str("printf '\\n%.0s' {1..22}"), Enter])?; + zellij.until(|lines| lines.first().is_some_and(|line| line.starts_with("skim$")))?; + + zellij.start_sk( + Some("for i in {1..20}; do echo min-height-scroll-item-$i; done"), + &["--height=20%", "--min-height=10"], + )?; + zellij.until(|lines| { + lines + .iter() + .map(|line| line.matches("min-height-scroll-item-").count()) + .sum::() + >= 7 + })?; + zellij.send_keys(&[Escape])?; + Ok(()) +} diff --git a/tests/options.rs b/tests/options.rs index 1ba971e2..a02928c9 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -848,7 +848,7 @@ fn opt_multiple_flags_parse() { "-I {} -I XX", "--color base --color light", "--margin 30% --margin 0", - "--min-height 30% --min-height 10", + "--min-height 30 --min-height 10", "--preview 'ls {}' --preview 'cat {}'", "--preview-window up --preview-window down", "--multi -m",