From c2b92f43c61faa1077c3e5326622adc7d260958e Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Sat, 11 Feb 2023 19:18:53 +0900
Subject: [PATCH] cli: do not report "broken pipe" if pager exits successfully

This partially reverts the change in d7f64c07e0aa "cli: handle last-minute
ui.write() error." In this commit, I tried to handle the case when the pager
process goes through env/sh, and exits immediately with "command not found".
However, I missed EPIPE could also occur when the pager was closed by user.

Apparently, hg is killed by SIGPIPE in that case (exits with 141), and chg
exits with 255. With this change, jj will silently exits with 3.
---
 src/cli_util.rs | 10 +++++++---
 src/ui.rs       | 20 ++++++++++++++------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/src/cli_util.rs b/src/cli_util.rs
index 2da61364d..b94634582 100644
--- a/src/cli_util.rs
+++ b/src/cli_util.rs
@@ -2085,9 +2085,13 @@ impl CliRunner {
                 exit_code
             }
             Err(err) => {
-                // The error is most likely a BrokenPipe. Close pager and write to stderr.
-                ui.finalize_pager();
-                writeln!(ui.error(), "Error: {err}").ok();
+                // The error is most likely a BrokenPipe. If the pager is closed by user
+                // (by e.g. pressing "q"), BrokenPipe shouldn't be considered a hard error.
+                // Otherwise, close pager and report the error to stderr.
+                let success_or_not_paged = ui.finalize_pager();
+                if !success_or_not_paged {
+                    writeln!(ui.error(), "Error: {err}").ok();
+                }
                 ExitCode::from(BROKEN_PIPE_EXIT_CODE)
             }
         }
diff --git a/src/ui.rs b/src/ui.rs
index cfffcf8ab..55aef69c7 100644
--- a/src/ui.rs
+++ b/src/ui.rs
@@ -242,19 +242,27 @@ impl Ui {
         }
     }
 
-    pub fn finalize_pager(&mut self) {
+    /// Waits for the pager exits. Returns true if the pager exits successfully
+    /// or the output isn't paged.
+    pub fn finalize_pager(&mut self) -> bool {
         if let UiOutput::Paged {
             mut child,
             child_stdin,
         } = mem::replace(&mut self.output, UiOutput::new_terminal())
         {
             drop(child_stdin);
-            if let Err(e) = child.wait() {
-                // It's possible (though unlikely) that this write fails, but
-                // this function gets called so late that there's not much we
-                // can do about it.
-                writeln!(self.error(), "Failed to wait on pager: {e}").ok();
+            match child.wait() {
+                Ok(status) => status.success(),
+                Err(e) => {
+                    // It's possible (though unlikely) that this write fails, but
+                    // this function gets called so late that there's not much we
+                    // can do about it.
+                    writeln!(self.error(), "Failed to wait on pager: {e}").ok();
+                    false
+                }
             }
+        } else {
+            true
         }
     }