From 7d7727495782b7f4ccd5a6729acff7bf05bdcbe9 Mon Sep 17 00:00:00 2001
From: Benjamin Brittain <ben@brittain.org>
Date: Sat, 10 Feb 2024 21:55:02 -0500
Subject: [PATCH] Use `minus` as a builtin pager

---
 CHANGELOG.md                |  2 +
 Cargo.lock                  | 18 ++++++++
 Cargo.toml                  |  2 +
 blah2                       |  0
 cli/Cargo.toml              |  1 +
 cli/src/config-schema.json  |  2 +-
 cli/src/config/misc.toml    |  2 +-
 cli/src/config/windows.toml |  2 +-
 cli/src/ui.rs               | 83 +++++++++++++++++++++++++++++++++++--
 docs/config.md              | 12 ++++--
 test                        |  0
 11 files changed, 114 insertions(+), 10 deletions(-)
 create mode 100644 blah2
 create mode 100644 test

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fcf54f08e0e..d3352993f9f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,11 +16,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### New features
 
 * Templates now support logical operators: `||`, `&&`, `!`
+* Builtin 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 builtin instead of disabled.
 
 ## [0.14.0] - 2024-02-07
 
diff --git a/Cargo.lock b/Cargo.lock
index 4ec4d3601b7..a1d489fd174 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,20 @@ dependencies = [
  "adler",
 ]
 
+[[package]]
+name = "minus"
+version = "5.5.1"
+source = "git+https://github.com/benbrittain/minus?branch=persist-alternate#28158b69f3004cd634f8b31dc404223bd76411bc"
+dependencies = [
+ "crossbeam-channel",
+ "crossterm",
+ "once_cell",
+ "parking_lot",
+ "regex",
+ "textwrap",
+ "thiserror",
+]
+
 [[package]]
 name = "mio"
 version = "0.8.10"
@@ -1938,6 +1953,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 40934b16bd3..c06ef2e2581 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -58,6 +58,8 @@ insta = { version = "1.34.0", features = ["filters"] }
 itertools = "0.12.1"
 libc = { version = "0.2.153" }
 maplit = "1.0.2"
+# TODO use upstream
+minus = { git = "https://github.com/benbrittain/minus", branch = "persist-alternate", 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 00000000000..e69de29bb2d
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index 470bb5efefd..a9c04420346 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-schema.json b/cli/src/config-schema.json
index 08b84189f89..e6bdd5c8ce1 100644
--- a/cli/src/config-schema.json
+++ b/cli/src/config-schema.json
@@ -72,7 +72,7 @@
                 "pager": {
                     "type": "string",
                     "description": "Pager to use for displaying command output",
-                    "default": "less -FRX"
+                    "default": ":builtin"
                 },
                 "diff": {
                     "type": "object",
diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml
index d4684ee2021..103d24bfe38 100644
--- a/cli/src/config/misc.toml
+++ b/cli/src/config/misc.toml
@@ -6,7 +6,7 @@ tree-level-conflicts = true
 
 [ui]
 paginate = "auto"
-pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } }
+pager = ":builtin"
 log-word-wrap = false
 
 [snapshot]
diff --git a/cli/src/config/windows.toml b/cli/src/config/windows.toml
index 52991d55491..deca7a3816d 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 0ecb4edb1b3..99cd763921f 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,68 @@ 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(|e| std::io::Error::other(e))?;
+        Ok(buf.len())
+    }
+}
+
+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();
+        // Keep the alternate screen around to emulate the $LESS `-X` flag.
+        pager
+            .persist_alternate_screen(true).expect("Able to set screen persistence");
+        // 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 +113,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,
         }
@@ -216,6 +281,9 @@ impl Ui {
         }
 
         match self.output {
+            UiOutput::Terminal { .. } if self.pager_cmd == BUILTIN_PAGER_NAME.into() => {
+                self.output = UiOutput::new_builtin();
+            }
             UiOutput::Terminal { .. } if io::stdout().is_terminal() => {
                 match UiOutput::new_paged(&self.pager_cmd) {
                     Ok(pager_output) => {
@@ -232,7 +300,7 @@ impl Ui {
                     }
                 }
             }
-            UiOutput::Terminal { .. } | UiOutput::Paged { .. } => {}
+            UiOutput::Terminal { .. } | UiOutput::Builtin { .. } | UiOutput::Paged { .. } => {}
         }
     }
 
@@ -252,6 +320,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 +337,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 +351,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 +361,7 @@ impl Ui {
         match &self.output {
             UiOutput::Terminal { stderr, .. } => self.progress_indicator && stderr.is_terminal(),
             UiOutput::Paged { .. } => false,
+            UiOutput::Builtin { .. } => false,
         }
     }
 
@@ -314,6 +386,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 b42b664e1bf..3d37db1b194 100644
--- a/docs/config.md
+++ b/docs/config.md
@@ -300,16 +300,20 @@ 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.
+In the absence of any setting, the default is the special value `:builtin`,
+which uses an [integrated pager](https://github.com/arijit79/minus/). It is likely
+that if you are using a standard linux distro, your system has `$PAGER` set already
+and that will be prefered over the built-in. To use the built-in:
+
+```
+jj config set --user ui.pager :builtin
+```
 
 Additionally, paging behavior can be toggled via `ui.paginate` like so:
 
diff --git a/test b/test
new file mode 100644
index 00000000000..e69de29bb2d