Skip to content

Commit

Permalink
built-in pager: write a message to the user before panicking if pager…
Browse files Browse the repository at this point in the history
… doesn't start

This will hopefully improve the experience of users who encounter #4182.

Unfortunately, this is the most I can think of doing in the short term
to address the problem. See the linked issue for more details.
  • Loading branch information
ilyagr committed Aug 3, 2024
1 parent 5b22594 commit cf14489
Show file tree
Hide file tree
Showing 2 changed files with 41 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
46 changes: 40 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 @@ -482,7 +483,40 @@ impl Ui {
}
}
UiOutput::BuiltinPaged { pager } => {
pager.finalize();
if let Err(minus_error) = pager.finalize() {
writeln!(
self.error_with_heading("Error"),
"Built-in pager failed to start: {minus_error}",
)
.ok();
writeln!(
self.error_no_heading(),
"The output of this command is lost."
)
.ok();
if let minus::error::MinusError::Setup(
minus::error::SetupError::InvalidTerminal,
) = minus_error
{
writeln!(
self.hint_with_heading("Hint"),
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`
- Use a different terminal (e.g. Windows Terminal or the Command Prompt)
- Configure a different pager, see https://martinvonz.github.io/jj/latest/windows/#pagination for Git Bash on Windows
- 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.
}
}
_ => { /* no-op */ }
}
Expand Down

0 comments on commit cf14489

Please sign in to comment.