Skip to content

Commit

Permalink
Prevent shell_keep_pipe from stopping on nonzero exit status code (h…
Browse files Browse the repository at this point in the history
…elix-editor#9817)

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.
  • Loading branch information
sirius94 authored and Schuyler Mortimer committed Jul 10, 2024
1 parent 9088ff5 commit cf51568
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 27 deletions.
30 changes: 9 additions & 21 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5440,16 +5440,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);
Expand All @@ -5468,15 +5461,15 @@ 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)))
}

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");
Expand Down Expand Up @@ -5539,7 +5532,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) {
Expand All @@ -5560,14 +5553,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
}
Expand All @@ -5578,11 +5571,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 {
Expand Down
8 changes: 2 additions & 6 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2261,7 +2261,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() {
Expand All @@ -2274,11 +2274,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)
Expand Down

0 comments on commit cf51568

Please sign in to comment.