Public/nushell
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove piping of stderr. (#2247)

Browse Source 
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.
This commit is contained in:
Jason Gedge 2020-07-24 01:56:50 -04:00 committed by GitHub
parent ed0c1038e3
commit 2d3b1e090a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 0 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
crates/nu-cli/src/commands/classified/external.rs
View File

@ -169,9 +169,6 @@ fn spawn(
if !is_last { if !is_last {
process.stdout(Stdio::piped()); process.stdout(Stdio::piped());
trace!(target: "nu::run::external", "set up stdout pipe"); 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 // open since we have some contents for stdin
@ -270,20 +267,6 @@ fn spawn(
return Err(()); 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 file = futures::io::AllowStdIo::new(stdout);
let stream = FramedRead::new(file, MaybeTextCodec::default()); 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 // We can give an error when we see a non-zero exit code, but this is different