Merge pull request #153 from itsjunetime/fix_dynamic_paging

fix: Make dynamic paging work again and add doc testing to CI

Author: itsjunetime

.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

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,35 +219,35 @@ 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.
 
 * Store the current run mode as static value
 
   * 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<Pager, TermError>`.
 
-* 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

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"

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,38 +83,34 @@ 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
         // unless run_no_overflow is set to true
         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<Command>,

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<RunMode> = 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,