From 0e7a47521393fb7c3668f4e1b14f04ae92c82df9 Mon Sep 17 00:00:00 2001 From: Benjamin Brittain Date: Sat, 10 Feb 2024 21:55:02 -0500 Subject: [PATCH] Use `minus` as a builtin pager --- CHANGELOG.md | 4 ++ Cargo.lock | 19 ++++++++ Cargo.toml | 1 + blah2 | 0 cli/Cargo.toml | 1 + cli/src/config/windows.toml | 2 +- cli/src/ui.rs | 91 +++++++++++++++++++++++++++++++++++-- docs/config.md | 18 ++++++-- 8 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 blah2 diff --git a/CHANGELOG.md b/CHANGELOG.md index fcf54f08e0..26743dd8e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Templates now support logical operators: `||`, `&&`, `!` +* Built-in pager based on [minus](https://github.com/arijit79/minus/) + ### Fixed bugs * On Windows, symlinks in the repo are now materialized as regular files in the working copy (instead of resulting in a crash). +* On Windows, the pager will now be the built-in instead of disabled. + ## [0.14.0] - 2024-02-07 ### Deprecations diff --git a/Cargo.lock b/Cargo.lock index 4ec4d3601b..1fdf4fc861 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1620,6 +1620,7 @@ dependencies = [ "jj-lib", "libc", "maplit", + "minus", "once_cell", "pest", "pest_derive", @@ -1858,6 +1859,21 @@ dependencies = [ "adler", ] +[[package]] +name = "minus" +version = "5.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "14b5f31d6666667f707078608f25e7615c48c2243a06b66ca0fa6c4ecb96362d" +dependencies = [ + "crossbeam-channel", + "crossterm", + "once_cell", + "parking_lot", + "regex", + "textwrap", + "thiserror", +] + [[package]] name = "mio" version = "0.8.10" @@ -1938,6 +1954,9 @@ name = "once_cell" version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" +dependencies = [ + "parking_lot_core", +] [[package]] name = "oorandom" diff --git a/Cargo.toml b/Cargo.toml index 40934b16bd..7c726332a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,6 +58,7 @@ insta = { version = "1.34.0", features = ["filters"] } itertools = "0.12.1" libc = { version = "0.2.153" } maplit = "1.0.2" +minus = { version = "5.5.0", features = [ "dynamic_output", "search" ] } num_cpus = "1.16.0" once_cell = "1.19.0" ouroboros = "0.18.0" diff --git a/blah2 b/blah2 new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 470bb5efef..a9c0442034 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -52,6 +52,7 @@ indexmap = { workspace = true } itertools = { workspace = true } jj-lib = { workspace = true } maplit = { workspace = true } +minus = { workspace = true } once_cell = { workspace = true } pest = { workspace = true } pest_derive = { workspace = true } diff --git a/cli/src/config/windows.toml b/cli/src/config/windows.toml index 52991d5549..deca7a3816 100644 --- a/cli/src/config/windows.toml +++ b/cli/src/config/windows.toml @@ -1,3 +1,3 @@ [ui] -paginate = "never" +pager = ":builtin" editor = "Notepad" diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 0ecb4edb1b..e4c41a7677 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -14,15 +14,20 @@ use std::io::{IsTerminal as _, Stderr, StderrLock, Stdout, StdoutLock, Write}; use std::process::{Child, ChildStdin, Stdio}; +use std::rc::Rc; use std::str::FromStr; +use std::thread::JoinHandle; use std::{env, fmt, io, mem}; +use minus::Pager as MinusPager; use tracing::instrument; use crate::cli_util::CommandError; use crate::config::CommandNameAndArgs; use crate::formatter::{Formatter, FormatterFactory, LabeledWriter}; +const BUILTIN_PAGER_NAME: &str = ":builtin"; + enum UiOutput { Terminal { stdout: Stdout, @@ -32,9 +37,71 @@ enum UiOutput { child: Child, child_stdin: ChildStdin, }, + Builtin { + pager: BuiltinPager, + }, +} + +/// A builtin pager +#[derive(Clone)] +pub struct BuiltinPager { + pager: MinusPager, + dynamic_pager_thread: Rc>, +} + +impl std::io::Write for BuiltinPager { + fn flush(&mut self) -> io::Result<()> { + // no-op since this is being run in a dynamic pager mode. + Ok(()) + } + + fn write(&mut self, buf: &[u8]) -> io::Result { + self.pager + .push_str(std::str::from_utf8(buf).unwrap()) + .map_err(std::io::Error::other)?; + Ok(buf.len()) + } +} + +impl Default for BuiltinPager { + fn default() -> Self { + Self::new() + } +} + +impl BuiltinPager { + pub fn finalize(self) { + let dynamic_pager_thread = self.dynamic_pager_thread; + if let Some(thread) = Rc::into_inner(dynamic_pager_thread) { + thread.join().unwrap(); + } + } + + pub fn new() -> Self { + let pager = MinusPager::new(); + // Prefer to be cautious and only kill the pager instead of the whole process + // like minus does by default. + pager + .set_exit_strategy(minus::ExitStrategy::PagerQuit) + .expect("Able to set the exit strategy"); + let pager_handle = pager.clone(); + + BuiltinPager { + pager, + dynamic_pager_thread: Rc::new(std::thread::spawn(move || { + // This thread handles the actual paging. + minus::dynamic_paging(pager_handle).unwrap(); + })), + } + } } impl UiOutput { + fn new_builtin() -> UiOutput { + UiOutput::Builtin { + pager: BuiltinPager::new(), + } + } fn new_terminal() -> UiOutput { UiOutput::Terminal { stdout: io::stdout(), @@ -49,21 +116,22 @@ impl UiOutput { } } -#[derive(Debug)] pub enum UiStdout<'a> { Terminal(StdoutLock<'static>), Paged(&'a ChildStdin), + Builtin(BuiltinPager), } -#[derive(Debug)] pub enum UiStderr<'a> { Terminal(StderrLock<'static>), Paged(&'a ChildStdin), + Builtin(BuiltinPager), } macro_rules! for_outputs { ($ty:ident, $output:expr, $pat:pat => $expr:expr) => { match $output { + $ty::Builtin($pat) => $expr, $ty::Terminal($pat) => $expr, $ty::Paged($pat) => $expr, } @@ -217,6 +285,11 @@ impl Ui { match self.output { UiOutput::Terminal { .. } if io::stdout().is_terminal() => { + if self.pager_cmd == BUILTIN_PAGER_NAME.into() { + self.output = UiOutput::new_builtin(); + return; + } + match UiOutput::new_paged(&self.pager_cmd) { Ok(pager_output) => { self.output = pager_output; @@ -225,14 +298,15 @@ impl Ui { // The pager executable couldn't be found or couldn't be run writeln!( self.warning(), - "Failed to spawn pager '{name}': {e}", + "Failed to spawn pager '{name}': {e}. Consider using the `:builtin` \ + pager.", name = self.pager_cmd.split_name(), ) .ok(); } } } - UiOutput::Terminal { .. } | UiOutput::Paged { .. } => {} + UiOutput::Terminal { .. } | UiOutput::Builtin { .. } | UiOutput::Paged { .. } => {} } } @@ -252,6 +326,7 @@ impl Ui { match &self.output { UiOutput::Terminal { stdout, .. } => UiStdout::Terminal(stdout.lock()), UiOutput::Paged { child_stdin, .. } => UiStdout::Paged(child_stdin), + UiOutput::Builtin { pager } => UiStdout::Builtin(pager.clone()), } } @@ -268,6 +343,7 @@ impl Ui { match &self.output { UiOutput::Terminal { stderr, .. } => UiStderr::Terminal(stderr.lock()), UiOutput::Paged { child_stdin, .. } => UiStderr::Paged(child_stdin), + UiOutput::Builtin { pager } => UiStderr::Builtin(pager.clone()), } } @@ -281,6 +357,7 @@ impl Ui { match &self.output { UiOutput::Terminal { .. } => Ok(Stdio::inherit()), UiOutput::Paged { child_stdin, .. } => Ok(duplicate_child_stdin(child_stdin)?.into()), + UiOutput::Builtin { .. } => Ok(Stdio::inherit()), } } @@ -290,6 +367,7 @@ impl Ui { match &self.output { UiOutput::Terminal { stderr, .. } => self.progress_indicator && stderr.is_terminal(), UiOutput::Paged { .. } => false, + UiOutput::Builtin { .. } => false, } } @@ -314,6 +392,11 @@ impl Ui { /// Waits for the pager exits. #[instrument(skip_all)] pub fn finalize_pager(&mut self) { + if let UiOutput::Builtin { pager } = + mem::replace(&mut self.output, UiOutput::new_terminal()) + { + pager.finalize(); + } if let UiOutput::Paged { mut child, child_stdin, diff --git a/docs/config.md b/docs/config.md index b42b664e1b..39b580bb8b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -300,16 +300,26 @@ Can be customized by the `format_short_signature()` template alias. ## Pager -Windows users: Note that pagination is disabled by default on Windows for now -([#2040](https://github.com/martinvonz/jj/issues/2040)). - The default pager is can be set via `ui.pager` or the `PAGER` environment variable. The priority is as follows (environment variables are marked with a `$`): `ui.pager` > `$PAGER` -`less -FRX` is the default pager in the absence of any other setting. +`less -FRX` is the default pager in the absence of any other setting, except +on Windows where it is `:builtin`. + +The special value `:builtin` enables usage of the +[integrated pager](https://github.com/arijit79/minus/). It is likely if you +are using a standard Linux distro, your system has `$PAGER` set already +and that will be preferred over the built-in. To use the built-in: + +``` +jj config set --user ui.pager :builtin +``` + +It is possible the default will change to `:builtin` for all platforms in the +future. Additionally, paging behavior can be toggled via `ui.paginate` like so: