From 37ccfd5acc423c6df27923e1697321c412524203 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Fri, 2 Aug 2024 01:05:30 -0700 Subject: [PATCH] built-in pager: write a message to the user if pager failed to start The message is printed at the end, any text sent to the pager before then is lost. See https://github.com/martinvonz/jj/pull/4197#discussion_r1701799135 for a discussion about why that seems OK. --- cli/Cargo.toml | 2 +- cli/src/ui.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index ffb41e2997..a1d0523b94 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -69,6 +69,7 @@ git2 = { workspace = true } gix = { workspace = true } hex = { workspace = true } indexmap = { workspace = true } +indoc = { workspace = true } itertools = { workspace = true } jj-lib = { workspace = true } maplit = { workspace = true } @@ -102,7 +103,6 @@ anyhow = { workspace = true } assert_cmd = { workspace = true } assert_matches = { workspace = true } async-trait = { workspace = true } -indoc = { workspace = true } insta = { workspace = true } test-case = { workspace = true } testutils = { workspace = true } diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 3adbbad45e..a8e296f101 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -18,7 +18,8 @@ use std::str::FromStr; use std::thread::JoinHandle; use std::{env, fmt, io, mem}; -use minus::Pager as MinusPager; +use indoc::indoc; +use minus::{MinusError, Pager as MinusPager}; use tracing::instrument; use crate::command_error::{config_error_with_message, CommandError}; @@ -46,7 +47,7 @@ enum UiOutput { /// A builtin pager pub struct BuiltinPager { pager: MinusPager, - dynamic_pager_thread: JoinHandle<()>, + dynamic_pager_thread: JoinHandle>, } impl Write for &BuiltinPager { @@ -69,9 +70,9 @@ impl Default for BuiltinPager { } impl BuiltinPager { - pub fn finalize(self) { + pub fn finalize(self) -> Result<(), MinusError> { let dynamic_pager_thread = self.dynamic_pager_thread; - dynamic_pager_thread.join().unwrap(); + dynamic_pager_thread.join().unwrap() } pub fn new() -> Self { @@ -87,7 +88,7 @@ impl BuiltinPager { pager, dynamic_pager_thread: std::thread::spawn(move || { // This thread handles the actual paging. - minus::dynamic_paging(pager_handle).unwrap(); + minus::dynamic_paging(pager_handle) }), } } @@ -130,7 +131,44 @@ impl UiOutput { } } UiOutput::BuiltinPaged { pager } => { - pager.finalize(); + if let Err(minus_error) = pager.finalize() { + writeln!( + ui.warning_default(), + "Built-in pager failed to start: {minus_error}", + ) + .ok(); + writeln!(ui.warning_default(), "The output of this command is lost.").ok(); + if matches!( + minus_error, + MinusError::Setup(minus::error::SetupError::InvalidTerminal) + ) { + if cfg!(windows) { + writeln!( + ui.hint_default(), + indoc! {r#" + jj's builtin pager is likely incompatible with this terminal + + This is known to happen with `mintty`, the default Git Bash terminal on Windows. + + Possible workarounds: + - Use `jj --no-pager` + - Configure a different pager, see https://martinvonz.github.io/jj/latest/windows/#pagination for Git Bash on Windows + - Use a different terminal (e.g. Windows Terminal or the Command Prompt) + - Use `winpty jj ...`; `winpty` comes with Git Bash or Cygwin."# + } + ).ok(); + } else { + writeln!( + ui.hint_default(), + "Try `jj --no-pager`. You can also try `TERM=xterm` or setting up \ + a different pager." + ) + .ok(); + } + } + // We do not cause a panic or another error so that the exit + // code of the command is preserved. + } } } }