From 2d3b1e090a6b484b699aae45ab25a4035ab08404 Mon Sep 17 00:00:00 2001 From: Jason Gedge Date: Fri, 24 Jul 2020 01:56:50 -0400 Subject: [PATCH] Remove piping of stderr. (#2247) In any other shell, stderr is inherited like normal, and only piped if you request it explicitly (e.g., `2>/dev/null`). In the case of a command like `fzf`, stderr is used for the interactive selection of files. By piping it, something like fzf | xargs echo does not work. By removing all stderr piping we eliminate this issue. We can return later with a way to deal with stderr piping when an actual use case arises. --- .../src/commands/classified/external.rs | 75 ------------------- 1 file changed, 75 deletions(-) diff --git a/crates/nu-cli/src/commands/classified/external.rs b/crates/nu-cli/src/commands/classified/external.rs index fedda6309f..a3251f734e 100644 --- a/crates/nu-cli/src/commands/classified/external.rs +++ b/crates/nu-cli/src/commands/classified/external.rs @@ -169,9 +169,6 @@ fn spawn( if !is_last { process.stdout(Stdio::piped()); trace!(target: "nu::run::external", "set up stdout pipe"); - - process.stderr(Stdio::piped()); - trace!(target: "nu::run::external", "set up stderr pipe"); } // open since we have some contents for stdin @@ -270,20 +267,6 @@ fn spawn( return Err(()); }; - let stderr = if let Some(stderr) = child.stderr.take() { - stderr - } else { - let _ = stdout_read_tx.send(Ok(Value { - value: UntaggedValue::Error(ShellError::labeled_error( - "Can't redirect the stderr for external command", - "can't redirect stderr", - &stdout_name_tag, - )), - tag: stdout_name_tag, - })); - return Err(()); - }; - let file = futures::io::AllowStdIo::new(stdout); let stream = FramedRead::new(file, MaybeTextCodec::default()); @@ -337,64 +320,6 @@ fn spawn( } } } - - let file = futures::io::AllowStdIo::new(stderr); - let err_stream = FramedRead::new(file, MaybeTextCodec::default()); - - for err_line in block_on_stream(err_stream) { - match err_line { - Ok(line) => match line { - StringOrBinary::String(s) => { - let result = stdout_read_tx.send(Ok(Value { - value: UntaggedValue::Error( - ShellError::untagged_runtime_error(s.clone()), - ), - tag: stdout_name_tag.clone(), - })); - - if result.is_err() { - break; - } - } - StringOrBinary::Binary(_) => { - let result = stdout_read_tx.send(Ok(Value { - value: UntaggedValue::Error( - ShellError::untagged_runtime_error( - "Binary in stderr output", - ), - ), - tag: stdout_name_tag.clone(), - })); - - if result.is_err() { - break; - } - } - }, - Err(e) => { - // If there's an exit status, it makes sense that we may error when - // trying to read from its stdout pipe (likely been closed). In that - // case, don't emit an error. - let should_error = match child.wait() { - Ok(exit_status) => !exit_status.success(), - Err(_) => true, - }; - - if should_error { - let _ = stdout_read_tx.send(Ok(Value { - value: UntaggedValue::Error(ShellError::labeled_error( - format!("Unable to read from stderr ({})", e), - "unable to read from stderr", - &stdout_name_tag, - )), - tag: stdout_name_tag.clone(), - })); - } - - return Ok(()); - } - } - } } // We can give an error when we see a non-zero exit code, but this is different