Use minus as a builtin pager

jj-vcs · Feb 11, 2024 · 0e7a475 · 0e7a475
1 parent 785f2f5
commit 0e7a475
Show file tree

Hide file tree

Showing 8 changed files with 127 additions and 9 deletions.
diff --git 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
diff --git 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
diff --git 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
@@ -1,3 +1,3 @@
 [ui]
-paginate = "never"
+pager = ":builtin"
 editor = "Notepad"
diff --git 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<JoinHandle<()>>,
+}
+
+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<usize> {
+        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
@@ -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: