From fdf87e49566daadbd5fdac9ea58b3ee139b48dac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Apr 2026 18:06:55 +0000 Subject: [PATCH] remove AliveGuard from dynamic paging quit-if-one-screen Agent-Logs-Url: https://github.com/forkeith/minus/sessions/6ad07131-4482-4d7e-83e6-fd6ab6ec33a4 Co-authored-by: keith-hall <11882719+keith-hall@users.noreply.github.com> --- src/dynamic_pager.rs | 19 +--------- src/pager.rs | 87 ++++---------------------------------------- 2 files changed, 8 insertions(+), 98 deletions(-) diff --git a/src/dynamic_pager.rs b/src/dynamic_pager.rs index a83fe7c..1b4d3f7 100644 --- a/src/dynamic_pager.rs +++ b/src/dynamic_pager.rs @@ -16,22 +16,5 @@ use crate::minus_core::init; #[cfg_attr(docsrs, doc(cfg(feature = "dynamic_output")))] #[allow(clippy::needless_pass_by_value)] pub fn dynamic_paging(pager: Pager) -> Result<(), MinusError> { - use crate::pager::AliveGuard; - use std::sync::Arc; - // Build a new Pager whose `alive` Arc is independent of the one held by - // application-side clones. When this local Pager drops (after `init_core` - // returns) only the independent Arc is decremented, which is harmless. - // - // Dropping `pager` here decrements the application-side Arc so that the - // correct reference count is maintained: only the application-side handles - // should keep that Arc alive. - let pager_for_init = Pager { - tx: pager.tx.clone(), - rx: pager.rx.clone(), - // New, independent Arc that fires a CheckQuitIfOneScreen into an already- - // closed channel once init_core returns – the send error is silently ignored. - alive: Arc::new(AliveGuard::new(pager.tx.clone())), - }; - drop(pager); // decrement the application-side Arc (count N+1 → N) - init::init_core(&pager_for_init, crate::RunMode::Dynamic) + init::init_core(&pager, crate::RunMode::Dynamic) } diff --git a/src/pager.rs b/src/pager.rs index 081eeb2..7dcf328 100644 --- a/src/pager.rs +++ b/src/pager.rs @@ -3,39 +3,10 @@ use crate::{ExitStrategy, LineNumbers, error::MinusError, input, minus_core::commands::Command}; use crossbeam_channel::{Receiver, Sender}; use std::fmt; -#[cfg(feature = "dynamic_output")] -use std::sync::Arc; #[cfg(feature = "search")] use crate::search::SearchOpts; -/// Guard that detects when all application-side [`Pager`] instances have been dropped. -/// -/// When the last [`Pager`] clone held by the application is dropped, the `Arc` wrapping -/// this guard reaches a reference count of zero and its `Drop` implementation sends a -/// [`Command::CheckQuitIfOneScreen`] to the reactor. The reactor then checks whether the -/// [`quit_if_one_screen`](Pager::set_quit_if_one_screen) option is enabled and, if so, -/// whether all content fits on one screen. -#[cfg(feature = "dynamic_output")] -pub struct AliveGuard { - tx: Sender, -} - -#[cfg(feature = "dynamic_output")] -impl AliveGuard { - pub(crate) const fn new(tx: Sender) -> Self { - Self { tx } - } -} - -#[cfg(feature = "dynamic_output")] -impl Drop for AliveGuard { - fn drop(&mut self) { - // Ignore errors: the reactor may have already exited. - let _ = self.tx.send(Command::CheckQuitIfOneScreen); - } -} - /// A communication bridge between the main application and the pager. /// /// The [Pager] type which is a bridge between your application and running @@ -66,10 +37,6 @@ impl Drop for AliveGuard { pub struct Pager { pub(crate) tx: Sender, pub(crate) rx: Receiver, - /// Shared guard that fires [`Command::CheckQuitIfOneScreen`] when all - /// application-side [`Pager`] clones are dropped. - #[cfg(feature = "dynamic_output")] - pub(crate) alive: Arc, } impl Clone for Pager { @@ -77,8 +44,6 @@ impl Clone for Pager { Self { tx: self.tx.clone(), rx: self.rx.clone(), - #[cfg(feature = "dynamic_output")] - alive: Arc::clone(&self.alive), } } } @@ -93,12 +58,7 @@ impl Pager { #[must_use] pub fn new() -> Self { let (tx, rx) = crossbeam_channel::unbounded(); - Self { - #[cfg(feature = "dynamic_output")] - alive: Arc::new(AliveGuard::new(tx.clone())), - tx, - rx, - } + Self { tx, rx } } /// Set the output text to this `t` @@ -268,17 +228,10 @@ impl Pager { /// Automatically quit when all content fits on one screen in dynamic paging mode. /// /// When this is set to `true`, minus will automatically exit the pager and preserve - /// the output on the terminal screen (similar to `less -F`) once the end of output - /// is signalled **and** the content fits within the available rows. If the content - /// does not fit the pager remains open until the user quits manually. - /// - /// The end of output can be signalled in two ways: - /// - /// - **Explicitly** — call [`Pager::end_of_output`] when you have finished sending - /// data. This is the preferred approach when the caller needs to keep the [`Pager`] - /// handle alive after signalling (e.g. to receive later notifications). - /// - **Implicitly** — simply drop all application-side [`Pager`] clones. When the - /// last clone is dropped the signal is sent automatically. + /// the output on the terminal screen (similar to `less -F`) once + /// [`end_of_output`](Pager::end_of_output) is called **and** the content fits within + /// the available rows. If the content does not fit the pager remains open until the + /// user quits manually. /// /// The content is always preserved on the terminal after an automatic quit, /// regardless of the configured [`ExitStrategy`](crate::ExitStrategy). @@ -289,7 +242,7 @@ impl Pager { /// Returns [`Err(MinusError::Communication)`](MinusError::Communication) if the /// configuration message could not be delivered to the pager. /// - /// # Example — explicit signal + /// # Example /// ```no_run /// # #[cfg(feature = "dynamic_output")] /// # { @@ -303,35 +256,13 @@ impl Pager { /// /// pager.push_str("Hello\nWorld\n").unwrap(); /// - /// // Explicitly signal that no more data will be sent. - /// // The pager handle is still alive after this call. + /// // Signal that no more data will be sent. /// pager.end_of_output().unwrap(); /// /// // If the two lines fit on one screen the pager has already exited. /// t.join().unwrap().unwrap(); /// # } /// ``` - /// - /// # Example — implicit signal (drop-based) - /// ```no_run - /// # #[cfg(feature = "dynamic_output")] - /// # { - /// use minus::{Pager, dynamic_paging}; - /// - /// let pager = Pager::new(); - /// pager.set_quit_if_one_screen(true).unwrap(); - /// - /// let pager2 = pager.clone(); - /// let t = std::thread::spawn(move || dynamic_paging(pager2)); - /// - /// pager.push_str("Hello\nWorld\n").unwrap(); - /// - /// // Dropping the last application-side clone signals end-of-output. - /// drop(pager); - /// - /// t.join().unwrap().unwrap(); - /// # } - /// ``` #[cfg(feature = "dynamic_output")] #[cfg_attr(docsrs, doc(cfg(feature = "dynamic_output")))] pub fn set_quit_if_one_screen(&self, val: bool) -> Result<(), MinusError> { @@ -345,10 +276,6 @@ impl Pager { /// exits the pager automatically while preserving the content on the terminal. /// If the content does not fit the pager remains open and the user can quit manually. /// - /// This is the explicit counterpart to the implicit drop-based signal: it lets you - /// mark the end of output while still holding the [`Pager`] handle (e.g. when the - /// caller needs to keep the handle for other purposes). - /// /// Calling this method when `set_quit_if_one_screen` is `false` is a no-op. /// /// # Errors