From 1c814afd267ea5ab20e3b94974a9c6994fdcd5e8 Mon Sep 17 00:00:00 2001 From: itsjunetime Date: Mon, 22 Dec 2025 09:29:03 -0500 Subject: [PATCH 1/2] fix: Make dynamic paging work again and add doc testing to CI --- CHANGELOG.md | 54 +++++++++++++++++++++++------------------------ Justfile | 17 +++++++++------ src/core/init.rs | 37 ++++++++++++-------------------- src/core/mod.rs | 6 +++++- src/lib.rs | 22 ++++++++++++++++--- src/pager.rs | 28 ++++++++++++++++++++++++ src/screen/mod.rs | 37 +++++++++++++++----------------- 7 files changed, 120 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c80769..4fa78e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,7 +122,7 @@ This file documents all changes made to the project and is updated before each r ## v5.2.0 [2023-03-01] ### Added -* Added `AppendStyle` and `AppendProps` enums in the new `minus_core::utils::text` to control the append behaviour and +* Added `AppendStyle` and `AppendProps` enums in the new `minus_core::utils::text` to control the append behaviour and properties related to each append operation ### Changes @@ -160,7 +160,7 @@ This file documents all changes made to the project and is updated before each r ## v5.0.4 [2022-07-31] ### Added -* Added dependency on [crossbeam-utils](https://crates.io/crates/crossbeam-utils). +* Added dependency on [crossbeam-utils](https://crates.io/crates/crossbeam-utils). This allows us to use scoped threads feature provided by it. ### Changed @@ -188,17 +188,17 @@ This file documents all changes made to the project and is updated before each r - Line Numbers are displayed only on the first wrapped row of each line. This decreases the clutter on the line number column especially on text which span multiple lines. - + - Line Numbers are now padded by about 5 spaces. This makes the line numbers not get tightly packed with the left edge of the terminal. ### Fixed -- Fixed bug when appending complex sets of text, a wrong value of `unterminated` got calculated which +- Fixed bug when appending complex sets of text, a wrong value of `unterminated` got calculated which caused junk text to appended to the `PagerState::formatted_lines` and also to be displayed on the terminal. - Fixed mouse scroll wheel not scrolling through the screen. This occurred because a of a previous patch which removed the line that enabled the mouse events to be captured. - + * Fix panic when the search term gets changed This occurred due to the `search_idx` not being repopulated when a new search is activated. @@ -219,17 +219,17 @@ This file documents all changes made to the project and is updated before each r This is the unification of the previous `tokio_lib` and `async_std_lib` features. minus no longer depends on `tokio` or `async_std` directly and requires end-application to bring in these libs as dependency. **This makes minus completely runtime agnostic** - -* minus can now be called from a OS thread using - [`threads`](https://doc.rust-lang.org/std/thread/index.html). + +* minus can now be called from a OS thread using + [`threads`](https://doc.rust-lang.org/std/thread/index.html). See example in [README](./README.md#threads) - -* Applications should call `dynamic_paging` on s separate non-blocking thread like + +* Applications should call `dynamic_paging` on s separate non-blocking thread like [`tokio::task::spawn_blocking()`](https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html) or [`threads`](https://doc.rust-lang.org/std/thread/index.html). * Use channels for communication - + * This allows minus to exactly know when data is changed and do various optimizations on it's * Added [`crossbeam_channels`](https://crates.io/crates/crossbeam_channels) as dependency. @@ -237,17 +237,17 @@ This file documents all changes made to the project and is updated before each r * The `RUNMODE` static item tells minus whether it is running in static mode or asynchronous mode * Added `once_cell` as a dependency to store the above value in static scope. - + * Added feature to scroll through more than one line - * Prefixing any of the movement keys with a number will move the screen up or down to that many lines. + * Prefixing any of the movement keys with a number will move the screen up or down to that many lines. For example `10j` will take the view 10 lines down. - * Similarly jump to specific line by prefixing `G` with a number. For example `15G` will take you to the + * Similarly jump to specific line by prefixing `G` with a number. For example `15G` will take you to the 15th line of the data. - + * Searching through text with lots of ansi sequence inside it will no longer break the search Previously this case would cause the search matcher to not match it and move to the next one (#57) - + * Added a `PagerState` struct to store and share internal data. It is made public, along with some of its fields so that it can be used to implement `InputClassifier` trait for applications that want to modify the default keybindings @@ -263,18 +263,18 @@ default keybindings The `handle_input()` function cared about a lot of things and passing everything as a parameter was really tedious. This also caused a breaking change whenever a new parameter was added -* Changed function signature of `Pager::new` to `new() -> Pager`. It previously used to return a +* Changed function signature of `Pager::new` to `new() -> Pager`. It previously used to return a `Result`. -* Use threads even in static paging mode. Although mutating the `Pager`s data won't reflect any changes in +* Use threads even in static paging mode. Although mutating the `Pager`s data won't reflect any changes in static mode. - -* Replaced `tokio-no-overflow` example with `static-no-overflow` function. This is because the + +* Replaced `tokio-no-overflow` example with `static-no-overflow` function. This is because the `Pager::run_no_overflow` function is only available in `static_output`feature. - + * All implemented functions on `Pager` except `Pager::new` will return a `Result<(), MinusError>` because the communication with the pager may fail if the pager has quit early on. - + * Applications should spawn `dynamic_paging` by themselves. For example on tokio, this would be ```rust use tokio::{task::spawn_blocking, join} @@ -294,10 +294,10 @@ default keybindings * Removed `tokio`, `async-std` and `async-mutex` from dependencies. * Removed `Pager::finish` function. * Removed `Pager::end_data_stream` function. - - This was only required for running in dynamic mode with run no overflow on. With deprecation of this + + This was only required for running in dynamic mode with run no overflow on. With deprecation of this feature we no longer need this function - + * Removed `static_long` example. * Removed `PageAllError` from `static_pager` and `errors` modules. @@ -358,10 +358,10 @@ default keybindings ### Fixed * Prevent panic if invalid regex is given during search * Fix run\_no\_overflow for static pager (#43) - + Previously, this setting had no effect if paging static output, due to an if condition in `static_pager.rs` which did not consider the setting. This commit makes - this setting behave as expected. (@tomstoneham) + this setting behave as expected. (@tomstoneham) * The cursor is hidden as soon as the search query entry is complete. * Fix where color outputs get distorted after search matches diff --git a/Justfile b/Justfile index 5a1d2a0..510445e 100644 --- a/Justfile +++ b/Justfile @@ -1,6 +1,6 @@ _prechecks: -cargo hack 2> /dev/null - + if [ $? -eq 101 ]; then \ cargo install cargo-hack; \ fi @@ -18,14 +18,17 @@ tests: cargo test --all-features --no-run cargo test --all-features +docs: + cargo doc --no-deps --document-private-items --keep-going --all-features + examples: - cargo check --example=dyn_tokio --features=dynamic_output - cargo check --example=msg-tokio --features=dynamic_output - cargo check --example=static --features=static_output - cargo check --example=less-rs --features=dynamic_output,search + cargo clippy --example=dyn_tokio --features=dynamic_output + cargo clippy --example=msg-tokio --features=dynamic_output + cargo clippy --example=static --features=static_output + cargo clippy --example=less-rs --features=dynamic_output,search lint: _prechecks cargo hack --feature-powerset clippy --all-targets - -verify-all: check-fmt build tests examples lint + +verify-all: check-fmt build tests examples lint docs @echo "Ready to go" diff --git a/src/core/init.rs b/src/core/init.rs index b54d8d7..a4549e1 100644 --- a/src/core/init.rs +++ b/src/core/init.rs @@ -56,7 +56,7 @@ use super::{CommandQueue, RUNMODE, utils::display::draw_for_change}; /// /// * If the size of the data is less than the available number of rows in the terminal /// then it displays everything on the main stdout screen at once and quits. This -/// behaviour can be turned off if [`Pager::set_run_no_overflow(true)`] is called +/// behaviour can be turned off if [`Pager::set_run_no_overflow`](true) is called /// by the main application // Sorry... this behaviour would have been cool to have in async mode, just think about it!!! Many // implementations were proposed but none were perfect @@ -83,28 +83,26 @@ pub fn init_core(pager: &Pager, rm: RunMode) -> std::result::Result<(), MinusErr #[cfg(feature = "search")] let input_thread_running = Arc::new((Mutex::new(true), Condvar::new())); - #[allow(unused_mut)] - let mut ps = crate::state::PagerState::generate_initial_state(&pager.rx, &mut out)?; - { let mut runmode = super::RUNMODE.lock(); - assert!( - runmode.is_uninitialized(), + assert_eq!( + *runmode, + RunMode::Uninitialized, "Failed to set the RUNMODE. This is caused probably because another instance of minus is already running" ); *runmode = rm; - drop(runmode); } + #[allow(unused_mut)] + let mut ps = crate::state::PagerState::generate_initial_state(&pager.rx, &mut out)?; + // Static mode checks #[cfg(feature = "static_output")] if *RUNMODE.lock() == RunMode::Static { // If stdout is not a tty, write everything and quit if !out.is_tty() { write_raw_lines(&mut out, &[ps.screen.orig_text], None)?; - let mut rm = RUNMODE.lock(); - *rm = RunMode::Uninitialized; - drop(rm); + *RUNMODE.lock() = RunMode::Uninitialized; return Ok(()); } // If number of lines of text is less than available rows, write everything and quit @@ -112,9 +110,7 @@ pub fn init_core(pager: &Pager, rm: RunMode) -> std::result::Result<(), MinusErr if ps.screen.formatted_lines_count() <= ps.rows && !ps.run_no_overflow { write_raw_lines(&mut out, &ps.screen.formatted_lines, Some("\r"))?; ps.exit(); - let mut rm = RUNMODE.lock(); - *rm = RunMode::Uninitialized; - drop(rm); + *RUNMODE.lock() = RunMode::Uninitialized; return Ok(()); } } @@ -131,7 +127,7 @@ pub fn init_core(pager: &Pager, rm: RunMode) -> std::result::Result<(), MinusErr panic::set_hook(Box::new(move |pinfo| { is_exited2.store(true, std::sync::atomic::Ordering::SeqCst); // While silently ignoring error is considered a bad practice, we are forced to do it here - // as we cannot use the ? and panicking here will cause UB. + // as we cannot use the ? and panicking here will (probably?) cause an immediate abort drop(term::cleanup( stdout(), &crate::ExitStrategy::PagerQuit, @@ -169,9 +165,7 @@ pub fn init_core(pager: &Pager, rm: RunMode) -> std::result::Result<(), MinusErr if res.is_err() { is_exited3.store(true, std::sync::atomic::Ordering::SeqCst); - let mut rm = RUNMODE.lock(); - *rm = RunMode::Uninitialized; - drop(rm); + *RUNMODE.lock() = RunMode::Uninitialized; term::cleanup(out.as_ref(), &crate::ExitStrategy::PagerQuit, true)?; } res @@ -188,9 +182,7 @@ pub fn init_core(pager: &Pager, rm: RunMode) -> std::result::Result<(), MinusErr if res.is_err() { is_exited4.store(true, std::sync::atomic::Ordering::SeqCst); - let mut rm = RUNMODE.lock(); - *rm = RunMode::Uninitialized; - drop(rm); + *RUNMODE.lock() = RunMode::Uninitialized; term::cleanup(out_copy.as_ref(), &crate::ExitStrategy::PagerQuit, true)?; } res @@ -215,9 +207,8 @@ pub fn init_core(pager: &Pager, rm: RunMode) -> std::result::Result<(), MinusErr /// and redraw if it event requires it to do so. /// /// For example if all rows in a terminal aren't filled and a -/// [`AppendData`](super::events::Event::AppendData) event occurs, it is absolutely necessory -/// to update the screen immediately; while if all rows are filled, we can omit to redraw the -/// screen. +/// [`AppendData`](super::commands::Command::AppendData) event occurs, it is absolutely necessary to +/// update the screen immediately; while if all rows are filled, we can omit to redraw the screen. #[allow(clippy::too_many_lines)] fn start_reactor( rx: &Receiver, diff --git a/src/core/mod.rs b/src/core/mod.rs index e77e299..a92b2d3 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -5,6 +5,10 @@ pub mod ev_handler; #[cfg(any(feature = "dynamic_output", feature = "static_output"))] pub mod init; pub mod utils; + +// TODO: Global statics aren't great and this one, in particular, is making it hard to run tests +// (since most non-unit tests will end up setting this, which then needs to be unset for the next +// test). Figure out how to get rid of this. pub static RUNMODE: parking_lot::Mutex = parking_lot::const_mutex(RunMode::Uninitialized); use commands::Command; @@ -65,7 +69,7 @@ impl CommandQueue { } /// Define the modes in which minus can run -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum RunMode { #[cfg(feature = "static_output")] Static, diff --git a/src/lib.rs b/src/lib.rs index f890a49..d94d234 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,24 +32,29 @@ //! ## Threads //! //! ```rust,no_run -//! use minus::{dynamic_paging, MinusError, Pager}; //! use std::{ //! fmt::Write, //! thread::{spawn, sleep}, //! time::Duration //! }; //! +//! # #[cfg(feature = "dynamic_output")] +//! use minus::dynamic_paging; +//! use minus::{MinusError, Pager}; +//! //! fn main() -> Result<(), MinusError> { //! // Initialize the pager //! let mut pager = Pager::new(); //! // Run the pager in a separate thread //! let pager2 = pager.clone(); +//! # #[cfg(feature = "dynamic_output")] //! let pager_thread = spawn(move || dynamic_paging(pager2)); //! //! for i in 0..=100_u32 { //! writeln!(pager, "{}", i); //! sleep(Duration::from_millis(100)); //! } +//! # #[cfg(feature = "dynamic_output")] //! pager_thread.join().unwrap()?; //! Ok(()) //! } @@ -58,9 +63,13 @@ //! ## tokio //! //! ```rust,no_run -//! use minus::{dynamic_paging, MinusError, Pager}; //! use std::time::Duration; //! use std::fmt::Write; +//! +//! # #[cfg(feature = "dynamic_output")] +//! use minus::dynamic_paging; +//! use minus::{MinusError, Pager}; +//! //! use tokio::{join, task::spawn_blocking, time::sleep}; //! //! #[tokio::main] @@ -79,11 +88,14 @@ //! // spawn_blocking(dynamic_paging(...)) creates a separate thread managed by the tokio //! // runtime and runs the async_paging inside it //! let pager = pager.clone(); +//! # #[cfg(feature = "dynamic_output")] //! let (res1, res2) = join!(spawn_blocking(move || dynamic_paging(pager)), increment); //! // .unwrap() unwraps any error while creating the tokio task //! // The ? mark unpacks any error that might have occurred while the //! // pager is running +//! # #[cfg(feature = "dynamic_output")] //! res1.unwrap()?; +//! # #[cfg(feature = "dynamic_output")] //! res2?; //! Ok(()) //! } @@ -92,7 +104,10 @@ //! ## Static output //! ```rust,no_run //! use std::fmt::Write; -//! use minus::{MinusError, Pager, page_all}; +//! +//! # #[cfg(feature = "static_output")] +//! use minus::page_all; +//! use minus::{MinusError, Pager}; //! //! fn main() -> Result<(), MinusError> { //! // Initialize a default static configuration @@ -102,6 +117,7 @@ //! writeln!(output, "{}", i)?; //! } //! // Run the pager +//! # #[cfg(feature = "static_output")] //! minus::page_all(output)?; //! // Return Ok result //! Ok(()) diff --git a/src/pager.rs b/src/pager.rs index 6de05aa..6545c63 100644 --- a/src/pager.rs +++ b/src/pager.rs @@ -386,3 +386,31 @@ impl fmt::Write for Pager { self.push_str(s).map_err(|_| fmt::Error) } } + +#[cfg(test)] +mod tests { + + #[test] + #[cfg(feature = "dynamic_output")] + fn basic_dynamic_paging() { + use super::*; + use crate::{RunMode, input::InputEvent, minus_core::RUNMODE}; + + // Need to reset this since this test is run in the same process as other tests and they + // change the runmode, which causes this test to fail since everything assumes the runmode + // isn't already set. + *RUNMODE.lock() = RunMode::Uninitialized; + + let pager = Pager::new(); + pager.follow_output(true).unwrap(); + + let pager2 = pager.clone(); + let pager_thread = std::thread::spawn(move || crate::dynamic_pager::dynamic_paging(pager2)); + + pager.tx.send(Command::UserInput(InputEvent::Exit)).unwrap(); + + pager_thread.join().unwrap().unwrap(); + + *RUNMODE.lock() = RunMode::Uninitialized; + } +} diff --git a/src/screen/mod.rs b/src/screen/mod.rs index 9c8d779..cc1214f 100644 --- a/src/screen/mod.rs +++ b/src/screen/mod.rs @@ -39,8 +39,8 @@ pub struct Screen { pub(crate) line_count: usize, pub(crate) max_line_length: usize, /// Unterminated lines - /// Keeps track of the number of lines at the last of [PagerState::formatted_lines] which are - /// not terminated by a newline + /// Keeps track of the number of lines at the last of [Self::formatted_lines] which are not + /// terminated by a newline pub(crate) unterminated: usize, /// Whether to Line wrap lines /// @@ -241,27 +241,27 @@ where pub buffer: B, /// Contains the incoming text data pub text: TextBlock<'a>, - /// This is Some when the last line inside minus's present data is unterminated. It contains the last - /// line to be attached to the the incoming text + /// This is Some when the last line inside minus's present data is unterminated. It contains the + /// last line to be attached to the the incoming text pub attachment: Option>, /// Status of line numbers pub line_numbers: LineNumbers, - /// This is equal to the number of lines in [`PagerState::lines`](crate::state::PagerState::lines). This basically tells what line - /// number the upcoming line will hold. + /// This is equal to the number of lines in [`Screen::orig_text`]. This basically tells what + /// line number the upcoming line will hold. pub lines_count: usize, - /// This is equal to the number of lines in [`PagerState::formatted_lines`](crate::state::PagerState::lines). This is used to + /// This is equal to the number of lines in [`Screen::formatted_lines`]. This is used to /// calculate the search index of the rows of the line. pub formatted_lines_count: usize, /// Actual number of columns available for displaying pub cols: usize, - /// Number of lines that are previously unterminated. It is only relevant when there is `attachment` text otherwise - /// it should be 0. + /// Number of lines that are previously unterminated. It is only relevant when there is + /// `attachment` text otherwise it should be 0. pub prev_unterminated: usize, /// Search term if a search is active #[cfg(feature = "search")] pub search_term: Option<&'a regex::Regex>, - /// Value of [PagerState::line_wrapping] + /// Value of [`Screen::line_wrapping`] pub line_wrapping: bool, } @@ -283,8 +283,7 @@ pub(crate) struct FormatResult { /// If search is active, this contains the indices where search matches in the incoming text have been found #[cfg(feature = "search")] pub append_search_idx: BTreeSet, - /// Map of where first row of each line is placed inside in - /// [`PagerState::formatted_lines`](crate::state::PagerState::formatted_lines) + /// Map of where first row of each line is placed inside in [`Screen::formatted_lines`] pub lines_to_row_map: LinesRowMap, /// The length of longest line encountered in the formatted text block pub max_line_length: usize, @@ -489,17 +488,15 @@ where /// /// - `line`: The line to format /// - `line_numbers`: tells whether to format the line with line numbers. -/// - `len_line_number`: is the number of digits that number of lines in [`PagerState::lines`] occupy. -/// For example, this will be 2 if number of lines in [`PagerState::lines`] is 50 and 3 if -/// number of lines in [`PagerState::lines`] is 500. This is used for calculating the padding -/// of each displayed line. -/// - `idx`: is the position index where the line is placed in [`PagerState::lines`]. +/// - `len_line_number`: is the number of digits that number of lines in [`Screen::orig_text`] +/// occupy. For example, this will be 2 if number of lines in [`Screen::line_count`] is 50 and 3 +/// if the number of lines in [`Screen::line_count`] is 500. This is used for calculating the +/// padding of each displayed line. +/// - `idx`: is the position index where the line is placed in [`Screen::orig_text`]. /// - `formatted_idx`: is the position index where the line will be placed in the resulting -/// [`PagerState::formatted_lines`](crate::state::PagerState::formatted_lines) +/// [`Screen::formatted_lines`] /// - `cols`: Number of columns in the terminal /// - `search_term`: Contains the regex if a search is active -/// -/// [`PagerState::lines`]: crate::state::PagerState::lines #[allow(clippy::too_many_arguments)] #[allow(clippy::uninlined_format_args)] pub(crate) fn formatted_line<'a>( From 4ee96558fa3b4cb1a7dea5d71f744bf2985a8c6b Mon Sep 17 00:00:00 2001 From: itsjunetime Date: Mon, 22 Dec 2025 09:37:29 -0500 Subject: [PATCH 2/2] Actually add doc generation to CI and make clippy actually fail on warnings --- .github/workflows/ci.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72d0ee8..05aa8ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,17 @@ jobs: - name: Run documentation tests run: cargo test --doc --all-features + docs: + name: docs + env: + RUST_BACKTRACE: 1 + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run docs generation + run: cargo doc --no-deps --document-private-items --keep-going --all-features + lint: name: lint runs-on: ubuntu-latest @@ -100,12 +111,8 @@ jobs: - uses: actions-rs/cargo@v1 with: command: clippy - args: --features=dynamic_output,search --tests --examples - - uses: actions-rs/cargo@v1 - with: - command: clippy - args: --features=dynamic_output,search --tests --examples + args: --features=dynamic_output,search --tests --examples -- -D warnings - uses: actions-rs/cargo@v1 with: command: clippy - args: --features=static_output,search --tests --examples + args: --features=static_output,search --tests --examples -- -D warnings