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 12, 2024
1 parent af8eb3f commit d3699c2
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 20 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj next/prev` now infer `--edit` when you're already editing a non-head
commit (a commit with children).

* 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
Expand Down
19 changes: 19 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
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/windows.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[ui]
paginate = "never"
pager = ":builtin"
editor = "Notepad"
109 changes: 94 additions & 15 deletions cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
use std::io::{IsTerminal as _, Stderr, StderrLock, Stdout, StdoutLock, Write};
use std::process::{Child, ChildStdin, Stdio};
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 +36,67 @@ enum UiOutput {
child: Child,
child_stdin: ChildStdin,
},
BuiltinPaged {
pager: BuiltinPager,
},
}

/// A builtin pager
pub struct BuiltinPager {
pager: MinusPager,
dynamic_pager_thread: 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> {
let string = std::str::from_utf8(buf).map_err(std::io::Error::other)?;
self.pager.push_str(string).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;
dynamic_pager_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: std::thread::spawn(move || {
// This thread handles the actual paging.
minus::dynamic_paging(pager_handle).unwrap();
}),
}
}
}

impl UiOutput {
fn new_builtin() -> UiOutput {
UiOutput::BuiltinPaged {
pager: BuiltinPager::new(),
}
}
fn new_terminal() -> UiOutput {
UiOutput::Terminal {
stdout: io::stdout(),
Expand All @@ -49,23 +111,24 @@ impl UiOutput {
}
}

#[derive(Debug)]
pub enum UiStdout<'a> {
Terminal(StdoutLock<'static>),
Paged(&'a ChildStdin),
Builtin(&'a BuiltinPager),
}

#[derive(Debug)]
pub enum UiStderr<'a> {
Terminal(StderrLock<'static>),
Paged(&'a ChildStdin),
Builtin(&'a BuiltinPager),
}

macro_rules! for_outputs {
($ty:ident, $output:expr, $pat:pat => $expr:expr) => {
match $output {
$ty::Terminal($pat) => $expr,
$ty::Paged($pat) => $expr,
$ty::Builtin($pat) => $expr,
}
};
}
Expand Down Expand Up @@ -217,6 +280,11 @@ impl Ui {

match self.output {
UiOutput::Terminal { .. } if io::stdout().is_terminal() => {
if self.pager_cmd == CommandNameAndArgs::String(BUILTIN_PAGER_NAME.into()) {
self.output = UiOutput::new_builtin();
return;
}

match UiOutput::new_paged(&self.pager_cmd) {
Ok(pager_output) => {
self.output = pager_output;
Expand All @@ -225,14 +293,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::BuiltinPaged { .. } | UiOutput::Paged { .. } => {}
}
}

Expand All @@ -252,6 +321,7 @@ impl Ui {
match &self.output {
UiOutput::Terminal { stdout, .. } => UiStdout::Terminal(stdout.lock()),
UiOutput::Paged { child_stdin, .. } => UiStdout::Paged(child_stdin),
UiOutput::BuiltinPaged { pager } => UiStdout::Builtin(pager),
}
}

Expand All @@ -268,6 +338,7 @@ impl Ui {
match &self.output {
UiOutput::Terminal { stderr, .. } => UiStderr::Terminal(stderr.lock()),
UiOutput::Paged { child_stdin, .. } => UiStderr::Paged(child_stdin),
UiOutput::BuiltinPaged { pager } => UiStderr::Builtin(pager),
}
}

Expand All @@ -281,6 +352,8 @@ impl Ui {
match &self.output {
UiOutput::Terminal { .. } => Ok(Stdio::inherit()),
UiOutput::Paged { child_stdin, .. } => Ok(duplicate_child_stdin(child_stdin)?.into()),
// Stderr does not get redirected through the built-in pager.
UiOutput::BuiltinPaged { .. } => Ok(Stdio::inherit()),
}
}

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

Expand All @@ -314,18 +388,23 @@ impl Ui {
/// Waits for the pager exits.
#[instrument(skip_all)]
pub fn finalize_pager(&mut self) {
if let UiOutput::Paged {
mut child,
child_stdin,
} = mem::replace(&mut self.output, UiOutput::new_terminal())
{
drop(child_stdin);
if let Err(e) = child.wait() {
// It's possible (though unlikely) that this write fails, but
// this function gets called so late that there's not much we
// can do about it.
writeln!(self.error(), "Failed to wait on pager: {e}").ok();
match mem::replace(&mut self.output, UiOutput::new_terminal()) {
UiOutput::Paged {
mut child,
child_stdin,
} => {
drop(child_stdin);
if let Err(e) = child.wait() {
// It's possible (though unlikely) that this write fails, but
// this function gets called so late that there's not much we
// can do about it.
writeln!(self.error(), "Failed to wait on pager: {e}").ok();
}
}
UiOutput::BuiltinPaged { pager } => {
pager.finalize();
}
_ => { /* no-op */ }
}
}

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

Expand Down

0 comments on commit d3699c2

Please sign in to comment.