Skip to content

Commit

Permalink
Use minus as a builtin pager
Browse files Browse the repository at this point in the history
  • Loading branch information
benbrittain committed Feb 11, 2024
1 parent 785f2f5 commit 7d77274
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 18 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Empty file added blah2
Empty file.
1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"pager": {
"type": "string",
"description": "Pager to use for displaying command output",
"default": "less -FRX"
"default": ":builtin"
},
"diff": {
"type": "object",
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config/misc.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config/windows.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[ui]
paginate = "never"
pager = ":builtin"
editor = "Notepad"
83 changes: 80 additions & 3 deletions cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
Expand All @@ -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,
}
Expand Down Expand Up @@ -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) => {
Expand All @@ -232,7 +300,7 @@ impl Ui {
}
}
}
UiOutput::Terminal { .. } | UiOutput::Paged { .. } => {}
UiOutput::Terminal { .. } | UiOutput::Builtin { .. } | UiOutput::Paged { .. } => {}
}
}

Expand All @@ -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()),
}
}

Expand All @@ -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()),
}
}

Expand All @@ -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()),
}
}

Expand All @@ -290,6 +361,7 @@ impl Ui {
match &self.output {
UiOutput::Terminal { stderr, .. } => self.progress_indicator && stderr.is_terminal(),
UiOutput::Paged { .. } => false,
UiOutput::Builtin { .. } => false,
}
}

Expand All @@ -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,
Expand Down
12 changes: 8 additions & 4 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Check failure on line 312 in docs/config.md

View workflow job for this annotation

GitHub Actions / Codespell

prefered ==> preferred

```
jj config set --user ui.pager :builtin
```

Additionally, paging behavior can be toggled via `ui.paginate` like so:

Expand Down
Empty file added test
Empty file.

0 comments on commit 7d77274

Please sign in to comment.