From 2301662260cf0ed9373ba606df875b040a7636e4 Mon Sep 17 00:00:00 2001
From: Cyrill Schenkel <cyrill.schenkel@gmail.com>
Date: Tue, 5 Mar 2024 13:08:28 +0100
Subject: [PATCH] Prevent `shell_keep_pipe` from stopping on nonzero exit
 status code

The `shell_impl` and `shell_impl_async` functions no longer return
`success` because it was always `true`. If the command didn't succeed
both functions would return an `Err`.

This was also the reason, why `shell_keep_pipe` didn't work. It relied
upon the value of `success` and aborted in case of an `Err`.
It now removes any selection for which `shell_impl` returns `Err`.

If the command always fails, the selections are preserved and an error
message is displayed in the status bar.
---
 helix-term/src/commands.rs       | 30 +++++++++---------------------
 helix-term/src/commands/typed.rs |  8 ++------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index 0b2ea0b8a52e..32567cfd58ec 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -5372,16 +5372,9 @@ fn shell_keep_pipe(cx: &mut Context) {
 
             for (i, range) in selection.ranges().iter().enumerate() {
                 let fragment = range.slice(text);
-                let (_output, success) = match shell_impl(shell, input, Some(fragment.into())) {
-                    Ok(result) => result,
-                    Err(err) => {
-                        cx.editor.set_error(err.to_string());
-                        return;
-                    }
-                };
-
-                // if the process exits successfully, keep the selection
-                if success {
+                if let Err(err) = shell_impl(shell, input, Some(fragment.into())) {
+                    log::debug!("Shell command failed: {}", err);
+                } else {
                     ranges.push(*range);
                     if i >= old_index && index.is_none() {
                         index = Some(ranges.len() - 1);
@@ -5400,7 +5393,7 @@ fn shell_keep_pipe(cx: &mut Context) {
     );
 }
 
-fn shell_impl(shell: &[String], cmd: &str, input: Option<Rope>) -> anyhow::Result<(Tendril, bool)> {
+fn shell_impl(shell: &[String], cmd: &str, input: Option<Rope>) -> anyhow::Result<Tendril> {
     tokio::task::block_in_place(|| helix_lsp::block_on(shell_impl_async(shell, cmd, input)))
 }
 
@@ -5408,7 +5401,7 @@ async fn shell_impl_async(
     shell: &[String],
     cmd: &str,
     input: Option<Rope>,
-) -> anyhow::Result<(Tendril, bool)> {
+) -> anyhow::Result<Tendril> {
     use std::process::Stdio;
     use tokio::process::Command;
     ensure!(!shell.is_empty(), "No shell set");
@@ -5471,7 +5464,7 @@ async fn shell_impl_async(
     let str = std::str::from_utf8(&output.stdout)
         .map_err(|_| anyhow!("Process did not output valid UTF-8"))?;
     let tendril = Tendril::from(str);
-    Ok((tendril, output.status.success()))
+    Ok(tendril)
 }
 
 fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) {
@@ -5492,14 +5485,14 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) {
     let mut shell_output: Option<Tendril> = None;
     let mut offset = 0isize;
     for range in selection.ranges() {
-        let (output, success) = if let Some(output) = shell_output.as_ref() {
-            (output.clone(), true)
+        let output = if let Some(output) = shell_output.as_ref() {
+            output.clone()
         } else {
             let fragment = range.slice(text);
             match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
                 Ok(result) => {
                     if !pipe {
-                        shell_output = Some(result.0.clone());
+                        shell_output = Some(result.clone());
                     }
                     result
                 }
@@ -5510,11 +5503,6 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) {
             }
         };
 
-        if !success {
-            cx.editor.set_error("Command failed");
-            return;
-        }
-
         let output_len = output.chars().count();
 
         let (from, to, deleted_len) = match behavior {
diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs
index 3d7ea3fc8b36..1a57701a909f 100644
--- a/helix-term/src/commands/typed.rs
+++ b/helix-term/src/commands/typed.rs
@@ -2257,7 +2257,7 @@ fn run_shell_command(
     let args = args.join(" ");
 
     let callback = async move {
-        let (output, success) = shell_impl_async(&shell, &args, None).await?;
+        let output = shell_impl_async(&shell, &args, None).await?;
         let call: job::Callback = Callback::EditorCompositor(Box::new(
             move |editor: &mut Editor, compositor: &mut Compositor| {
                 if !output.is_empty() {
@@ -2270,11 +2270,7 @@ fn run_shell_command(
                     ));
                     compositor.replace_or_push("shell", popup);
                 }
-                if success {
-                    editor.set_status("Command succeeded");
-                } else {
-                    editor.set_error("Command failed");
-                }
+                editor.set_status("Command succeeded");
             },
         ));
         Ok(call)