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
jj-vcs#4197 (comment)
for a discussion about why that seems OK.
  • Loading branch information
ilyagr committed Aug 3, 2024
1 parent 9ec6175 commit 87b1c5c
Show file tree
Hide file tree
Showing 2 changed files with 37 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
42 changes: 36 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,36 @@ 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)
) {
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.
See also https://github.com/martinvonz/jj/issues/4182.
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 ...` on Windows."#
}
).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 87b1c5c

Please sign in to comment.