From 6cb2cf6329dc9f7f7b15288b7bd42b68391d500b Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Fri, 2 Aug 2024 01:05:30 -0700
Subject: [PATCH] built-in pager: write a message to the user before panicking
 if pager 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.
---
 cli/Cargo.toml |  2 +-
 cli/src/ui.rs  | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index ffb41e2997e..a1d0523b94e 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 21945db687a..649536d2d64 100644
--- a/cli/src/ui.rs
+++ b/cli/src/ui.rs
@@ -18,6 +18,7 @@ use std::str::FromStr;
 use std::thread::JoinHandle;
 use std::{env, fmt, io, mem};
 
+use indoc::indoc;
 use minus::Pager as MinusPager;
 use tracing::instrument;
 
@@ -87,7 +88,40 @@ impl BuiltinPager {
             pager,
             dynamic_pager_thread: std::thread::spawn(move || {
                 // This thread handles the actual paging.
-                minus::dynamic_paging(pager_handle).unwrap();
+
+                // TODO(#4182): handle error better. This is a bit tricky.
+                // Currently, the panic tends to happen early and abort any
+                // mutating operations. The only way I can think of easily
+                // preserving the error, it will only be detected after the jj
+                // operation completes. So, jj could do some mutations, but all
+                // user output will be lost at that point. This seemed
+                // problematic to me, so while a panic is not great, it seems
+                // like a better alternative.
+                minus::dynamic_paging(pager_handle)
+                    .inspect_err(|err| {
+                        eprintln!("\n");
+                        if let minus::error::MinusError::Setup(
+                            minus::error::SetupError::InvalidTerminal,
+                        ) = err
+                        {
+                            eprintln!(indoc! {r#"
+                                jj ERROR: jj's builtin pager is 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."#
+                                })
+                        } else {
+                            eprintln!("jj ERROR: built-in pager failed to start, will panic.")
+                        }
+                        eprintln!();
+                    })
+                    .unwrap();
             }),
         }
     }