From 7c2400f3e5a7a849982c02c547b9e40eabe542ab Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 18 Oct 2022 11:07:35 -0700 Subject: [PATCH] ui: add pager Teach Ui's writing functions to write to a pager without touching the process's file descriptors. This is done by introducing UiOutput::Paged, which spawns a pager that Ui's functions can write to. The pager program can be chosen via `ui.pager`. (defaults to Defaults to $PAGER, and 'less' if that is unset (falling back to 'less' also makes the tests pass). Currently, commands are paginated if: - they have "long" output (as defined by jj developers) - jj is invoked in a terminal The next commit will allow pagination to be turned off via a CLI option. More complex pagination toggling (e.g. showing a pager even if the output doesn't look like a terminal, using a pager for shorter ouput) is left for a future PR. --- CHANGELOG.md | 2 + docs/config.md | 11 +++++ examples/custom-backend/main.rs | 1 + examples/custom-command/main.rs | 1 + src/commands.rs | 6 +++ src/config.rs | 3 ++ src/main.rs | 1 + src/ui.rs | 74 ++++++++++++++++++++++++++++++++- 8 files changed, 97 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d34ac158a..b34f091a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### New features +* Commands with long output are paginated. + * The new `jj git remote rename` command allows git remotes to be renamed in-place. diff --git a/docs/config.md b/docs/config.md index e1a890796..c13280048 100644 --- a/docs/config.md +++ b/docs/config.md @@ -43,6 +43,17 @@ This setting overrides the `NO_COLOR` environment variable (if set). ui.color = "never" # Turn off color +### Paginating output + +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` is the default pager in the absence of any other setting. + + ### Editor The default editor is set via `ui.editor`, diff --git a/examples/custom-backend/main.rs b/examples/custom-backend/main.rs index 41a4bdfd5..03959d06f 100644 --- a/examples/custom-backend/main.rs +++ b/examples/custom-backend/main.rs @@ -65,6 +65,7 @@ fn main() { let (mut ui, result) = create_ui(); let result = result.and_then(|()| run(&mut ui)); let exit_code = handle_command_result(&mut ui, result); + ui.finalize_writes(); std::process::exit(exit_code); } diff --git a/examples/custom-command/main.rs b/examples/custom-command/main.rs index 5047117e1..0606a3796 100644 --- a/examples/custom-command/main.rs +++ b/examples/custom-command/main.rs @@ -63,5 +63,6 @@ fn main() { let (mut ui, result) = create_ui(); let result = result.and_then(|()| run(&mut ui)); let exit_code = handle_command_result(&mut ui, result); + ui.finalize_writes(); std::process::exit(exit_code); } diff --git a/src/commands.rs b/src/commands.rs index 34c62e6a8..93b024eab 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1332,6 +1332,7 @@ fn show_color_words_diff_line( } fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(), CommandError> { + ui.request_pager(); let workspace_command = command.workspace_helper(ui)?; let from_tree; let to_tree; @@ -1359,6 +1360,7 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(), } fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(), CommandError> { + ui.request_pager(); let workspace_command = command.workspace_helper(ui)?; let commit = workspace_command.resolve_single_rev(&args.revision)?; let parents = commit.parents(); @@ -2031,6 +2033,7 @@ fn log_template(settings: &UserSettings) -> String { } fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), CommandError> { + ui.request_pager(); let workspace_command = command.workspace_helper(ui)?; let default_revset = ui.settings().default_revset(); @@ -2193,6 +2196,7 @@ fn show_patch( } fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result<(), CommandError> { + ui.request_pager(); let workspace_command = command.workspace_helper(ui)?; let start_commit = workspace_command.resolve_single_rev(&args.revision)?; @@ -2288,6 +2292,7 @@ fn cmd_interdiff( command: &CommandHelper, args: &InterdiffArgs, ) -> Result<(), CommandError> { + ui.request_pager(); let workspace_command = command.workspace_helper(ui)?; let from = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?; let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; @@ -3604,6 +3609,7 @@ fn cmd_op_log( command: &CommandHelper, _args: &OperationLogArgs, ) -> Result<(), CommandError> { + ui.request_pager(); let workspace_command = command.workspace_helper(ui)?; let repo = workspace_command.repo(); let head_op = repo.operation().clone(); diff --git a/src/config.rs b/src/config.rs index 90b6864a7..303736dcc 100644 --- a/src/config.rs +++ b/src/config.rs @@ -58,6 +58,9 @@ fn env_base() -> config::Config { // should override $NO_COLOR." https://no-color.org/ builder = builder.set_override("ui.color", "never").unwrap(); } + if let Ok(value) = env::var("PAGER") { + builder = builder.set_override("ui.pager", value).unwrap(); + } if let Ok(value) = env::var("VISUAL") { builder = builder.set_override("ui.editor", value).unwrap(); } else if let Ok(value) = env::var("EDITOR") { diff --git a/src/main.rs b/src/main.rs index 93402487b..0077ddff0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -59,5 +59,6 @@ fn main() { let (mut ui, result) = create_ui(); let result = result.and_then(|()| run(&mut ui, reload_log_filter)); let exit_code = handle_command_result(&mut ui, result); + ui.finalize_writes(); std::process::exit(exit_code); } diff --git a/src/ui.rs b/src/ui.rs index bc58f4c4e..1b6b3e02a 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -14,8 +14,9 @@ use std::io::{Stderr, Stdout, Write}; use std::path::{Path, PathBuf}; +use std::process::{Child, ChildStdin, Command, Stdio}; use std::str::FromStr; -use std::{fmt, io}; +use std::{fmt, io, mem}; use crossterm::tty::IsTty; use jujutsu_lib::settings::UserSettings; @@ -92,6 +93,13 @@ fn use_color(choice: ColorChoice) -> bool { } } +fn pager_setting(settings: &UserSettings) -> String { + settings + .config() + .get_string("ui.pager") + .unwrap_or_else(|_| "less".to_string()) +} + impl Ui { pub fn for_terminal(settings: UserSettings) -> Ui { let cwd = std::env::current_dir().unwrap(); @@ -116,6 +124,18 @@ impl Ui { } } + /// Switches the output to use the pager, if allowed. + pub fn request_pager(&mut self) { + match self.output { + UiOutput::Paged { .. } => {} + UiOutput::Terminal { .. } => { + if io::stdout().is_tty() { + self.output = UiOutput::new_paged_else_terminal(&self.settings); + } + } + } + } + pub fn color(&self) -> bool { self.color } @@ -148,6 +168,7 @@ impl Ui { pub fn stdout_formatter<'a>(&'a self) -> Box { match &self.output { UiOutput::Terminal { stdout, .. } => self.new_formatter(stdout.lock()), + UiOutput::Paged { child_stdin, .. } => self.new_formatter(child_stdin), } } @@ -155,6 +176,7 @@ impl Ui { pub fn stderr_formatter<'a>(&'a self) -> Box { match &self.output { UiOutput::Terminal { stderr, .. } => self.new_formatter(stderr.lock()), + UiOutput::Paged { child_stdin, .. } => self.new_formatter(child_stdin), } } @@ -168,6 +190,7 @@ impl Ui { let data = text.as_bytes(); match &mut self.output { UiOutput::Terminal { stdout, .. } => stdout.write_all(data), + UiOutput::Paged { child_stdin, .. } => child_stdin.write_all(data), } } @@ -175,12 +198,14 @@ impl Ui { let data = text.as_bytes(); match &mut self.output { UiOutput::Terminal { stderr, .. } => stderr.write_all(data), + UiOutput::Paged { child_stdin, .. } => child_stdin.write_all(data), } } pub fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { match &mut self.output { UiOutput::Terminal { stdout, .. } => stdout.write_fmt(fmt), + UiOutput::Paged { child_stdin, .. } => child_stdin.write_fmt(fmt), } } @@ -211,6 +236,24 @@ impl Ui { pub fn flush(&mut self) -> io::Result<()> { match &mut self.output { UiOutput::Terminal { stdout, .. } => stdout.flush(), + UiOutput::Paged { child_stdin, .. } => child_stdin.flush(), + } + } + + pub fn finalize_writes(&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. + self.write_error(&format!("Failed to wait on pager {}", e)) + .ok(); + } } } @@ -249,13 +292,23 @@ impl Ui { text, output: match self.output { UiOutput::Terminal { .. } => io::stdout(), + // TODO we don't actually need to write in this case, so it + // might be better to no-op + UiOutput::Paged { .. } => io::stdout(), }, } } } enum UiOutput { - Terminal { stdout: Stdout, stderr: Stderr }, + Terminal { + stdout: Stdout, + stderr: Stderr, + }, + Paged { + child: Child, + child_stdin: ChildStdin, + }, } impl UiOutput { @@ -265,6 +318,23 @@ impl UiOutput { stderr: io::stderr(), } } + + fn new_paged_else_terminal(settings: &UserSettings) -> UiOutput { + let pager_cmd = pager_setting(settings); + let child_result = Command::new(pager_cmd).stdin(Stdio::piped()).spawn(); + match child_result { + Ok(mut child) => { + let child_stdin = child.stdin.take().unwrap(); + UiOutput::Paged { child, child_stdin } + } + Err(e) => { + io::stderr() + .write_fmt(format_args!("Failed to spawn pager: {}", e)) + .ok(); + UiOutput::new_terminal() + } + } + } } pub struct OutputGuard {