Skip to content

Commit

Permalink
built-in pager: write a message to the user if pager failed to start
Browse files Browse the repository at this point in the history
The message is printed at the end, any text sent to the pager before
then is lost. See
#4197 (comment)
for a discussion about why that seems OK.
  • Loading branch information
ilyagr committed Aug 4, 2024
1 parent 9ec6175 commit 37ccfd5
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 7 deletions.
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down
50 changes: 44 additions & 6 deletions cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -46,7 +47,7 @@ enum UiOutput {
/// A builtin pager
pub struct BuiltinPager {
pager: MinusPager,
dynamic_pager_thread: JoinHandle<()>,
dynamic_pager_thread: JoinHandle<Result<(), MinusError>>,
}

impl Write for &BuiltinPager {
Expand All @@ -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 {
Expand All @@ -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)
}),
}
}
Expand Down Expand Up @@ -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.
}
}
}
}
Expand Down

0 comments on commit 37ccfd5

Please sign in to comment.