Fix external extra (#4777)

* Fix empty table from externals * Fix empty table from externals
nushell · Mar 8, 2022 · 299fea8 · 299fea8
1 parent 35ff107
commit 299fea8
Show file tree

Hide file tree

Showing 18 changed files with 138 additions and 41 deletions.
diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs
@@ -18,7 +18,11 @@ pub fn print_pipeline_data(
 
  let stdout = std::io::stdout();
 
- if let PipelineData::ExternalStream { stdout: stream, .. } = input {
+ if let PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } = input
+ {
  for s in stream {
  let _ = stdout.lock().write_all(s?.as_binary()?);
  }

diff --git a/crates/nu-command/src/conversions/into/binary.rs b/crates/nu-command/src/conversions/into/binary.rs
@@ -99,7 +99,15 @@ fn into_binary(
  let column_paths: Vec<CellPath> = call.rest(engine_state, stack, 0)?;
 
  match input {
- PipelineData::ExternalStream { stdout: stream, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => Ok(Value::Binary {
+ val: vec![],
+ span: head,
+ }
+ .into_pipeline_data()),
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => {
  // TODO: in the future, we may want this to stream out, converting each to bytes
  let output = stream.into_bytes()?;
  Ok(Value::Binary {

diff --git a/crates/nu-command/src/conversions/into/string.rs b/crates/nu-command/src/conversions/into/string.rs
@@ -150,7 +150,15 @@ fn string_helper(
  }
 
  match input {
- PipelineData::ExternalStream { stdout: stream, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => Ok(Value::String {
+ val: String::new(),
+ span: head,
+ }
+ .into_pipeline_data()),
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => {
  // TODO: in the future, we may want this to stream out, converting each to bytes
  let output = stream.into_string()?;
  Ok(Value::String {

diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs
@@ -121,11 +121,11 @@ impl Command for Open {
  let buf_reader = BufReader::new(file);
 
  let output = PipelineData::ExternalStream {
- stdout: RawStream::new(
+ stdout: Some(RawStream::new(
  Box::new(BufferedReader { input: buf_reader }),
  ctrlc,
  call_span,
- ),
+ )),
  stderr: None,
  exit_code: None,
  span: call_span,

diff --git a/crates/nu-command/src/filters/each.rs b/crates/nu-command/src/filters/each.rs
@@ -169,7 +169,11 @@ impl Command for Each {
  }
  })
  .into_pipeline_data(ctrlc)),
- PipelineData::ExternalStream { stdout: stream, .. } => Ok(stream
+ PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::new(call.head)),
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => Ok(stream
  .into_iter()
  .enumerate()
  .map(move |(idx, x)| {

diff --git a/crates/nu-command/src/filters/par_each.rs b/crates/nu-command/src/filters/par_each.rs
@@ -213,7 +213,11 @@ impl Command for ParEach {
  .into_iter()
  .flatten()
  .into_pipeline_data(ctrlc)),
- PipelineData::ExternalStream { stdout: stream, .. } => Ok(stream
+ PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::new(call.head)),
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => Ok(stream
  .enumerate()
  .par_bridge()
  .map(move |(idx, x)| {

diff --git a/crates/nu-command/src/filters/skip/skip_.rs b/crates/nu-command/src/filters/skip/skip_.rs
@@ -77,7 +77,7 @@ impl Command for Skip {
 
  match input {
  PipelineData::ExternalStream {
- stdout: stream,
+ stdout: Some(stream),
  span: bytes_span,
  metadata,
  ..

diff --git a/crates/nu-command/src/network/fetch.rs b/crates/nu-command/src/network/fetch.rs
@@ -357,13 +357,13 @@ fn response_to_buffer(
  let buffered_input = BufReader::new(response);
 
  PipelineData::ExternalStream {
- stdout: RawStream::new(
+ stdout: Some(RawStream::new(
  Box::new(BufferedReader {
  input: buffered_input,
  }),
  engine_state.ctrlc.clone(),
  span,
- ),
+ )),
  stderr: None,
  exit_code: None,
  span,

diff --git a/crates/nu-command/src/network/post.rs b/crates/nu-command/src/network/post.rs
@@ -418,13 +418,13 @@ fn response_to_buffer(
  let buffered_input = BufReader::new(response);
 
  PipelineData::ExternalStream {
- stdout: RawStream::new(
+ stdout: Some(RawStream::new(
  Box::new(BufferedReader {
  input: buffered_input,
  }),
  engine_state.ctrlc.clone(),
  span,
- ),
+ )),
  stderr: None,
  exit_code: None,
  span,

diff --git a/crates/nu-command/src/strings/decode.rs b/crates/nu-command/src/strings/decode.rs
@@ -44,7 +44,11 @@ impl Command for Decode {
  let encoding: Spanned<String> = call.req(engine_state, stack, 0)?;
 
  match input {
- PipelineData::ExternalStream { stdout: stream, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::new(call.head)),
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => {
  let bytes: Vec<u8> = stream.into_bytes()?.item;
 
  let encoding = match Encoding::for_label(encoding.item.as_bytes()) {

diff --git a/crates/nu-command/src/system/complete.rs b/crates/nu-command/src/system/complete.rs
@@ -34,21 +34,24 @@ impl Command for Complete {
  exit_code,
  ..
  } => {
- let mut cols = vec!["stdout".to_string()];
+ let mut cols = vec![];
  let mut vals = vec![];
 
- let stdout = stdout.into_bytes()?;
- if let Ok(st) = String::from_utf8(stdout.item.clone()) {
- vals.push(Value::String {
- val: st,
- span: stdout.span,
- })
- } else {
- vals.push(Value::Binary {
- val: stdout.item,
- span: stdout.span,
- })
- };
+ if let Some(stdout) = stdout {
+ cols.push("stdout".to_string());
+ let stdout = stdout.into_bytes()?;
+ if let Ok(st) = String::from_utf8(stdout.item.clone()) {
+ vals.push(Value::String {
+ val: st,
+ span: stdout.span,
+ })
+ } else {
+ vals.push(Value::Binary {
+ val: stdout.item,
+ span: stdout.span,
+ })
+ }
+ }
 
  if let Some(stderr) = stderr {
  cols.push("stderr".to_string());

diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs
@@ -307,7 +307,15 @@ impl ExternalCommand {
  let exit_code_receiver = ValueReceiver::new(exit_code_rx);
 
  Ok(PipelineData::ExternalStream {
- stdout: RawStream::new(Box::new(stdout_receiver), output_ctrlc.clone(), head),
+ stdout: if redirect_stdout {
+ Some(RawStream::new(
+ Box::new(stdout_receiver),
+ output_ctrlc.clone(),
+ head,
+ ))
+ } else {
+ None
+ },
  stderr: Some(RawStream::new(
  Box::new(stderr_receiver),
  output_ctrlc.clone(),

diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs
@@ -67,7 +67,7 @@ impl Command for Table {
  PipelineData::ExternalStream { .. } => Ok(input),
  PipelineData::Value(Value::Binary { val, .. }, ..) => {
  Ok(PipelineData::ExternalStream {
- stdout: RawStream::new(
+ stdout: Some(RawStream::new(
  Box::new(
  vec![Ok(format!("{}\n", nu_pretty_hex::pretty_hex(&val))
  .as_bytes()
@@ -76,7 +76,7 @@ impl Command for Table {
  ),
  ctrlc,
  head,
- ),
+ )),
  stderr: None,
  exit_code: None,
  span: head,
@@ -269,7 +269,7 @@ fn handle_row_stream(
  let head = call.head;
 
  Ok(PipelineData::ExternalStream {
- stdout: RawStream::new(
+ stdout: Some(RawStream::new(
  Box::new(PagingTableCreator {
  row_offset,
  config,
@@ -279,7 +279,7 @@ fn handle_row_stream(
  }),
  ctrlc,
  head,
- ),
+ )),
  stderr: None,
  exit_code: None,
  span: head,

diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs
@@ -47,6 +47,7 @@ mod reverse;
 mod rm;
 mod roll;
 mod rotate;
+mod run_external;
 mod save;
 mod select;
 mod semicolon;

diff --git a/crates/nu-command/tests/commands/run_external.rs b/crates/nu-command/tests/commands/run_external.rs
@@ -0,0 +1,15 @@
+use nu_test_support::{nu, pipeline};
+
+#[test]
+fn better_empty_redirection() {
+ let actual = nu!(
+ cwd: "tests/fixtures/formats", pipeline(
+ r#"
+ ls | each { |it| nu --testbin cococo $it.name }
+ "#
+ ));
+
+ eprintln!("out: {}", actual.out);
+
+ assert!(!actual.out.contains('2'));
+}
diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs
@@ -36,7 +36,7 @@ pub enum PipelineData {
  Value(Value, Option<PipelineMetadata>),
  ListStream(ListStream, Option<PipelineMetadata>),
  ExternalStream {
- stdout: RawStream,
+ stdout: Option<RawStream>,
  stderr: Option<RawStream>,
  exit_code: Option<ListStream>,
  span: Span,
@@ -93,7 +93,11 @@ impl PipelineData {
  vals: s.collect(),
  span, // FIXME?
  },
- PipelineData::ExternalStream { stdout: mut s, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => Value::Nothing { span },
+ PipelineData::ExternalStream {
+ stdout: Some(mut s),
+ ..
+ } => {
  let mut items = vec![];
 
  for val in &mut s {
@@ -157,7 +161,10 @@ impl PipelineData {
  match self {
  PipelineData::Value(v, ..) => Ok(v.into_string(separator, config)),
  PipelineData::ListStream(s, ..) => Ok(s.into_string(separator, config)),
- PipelineData::ExternalStream { stdout: s, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => Ok(String::new()),
+ PipelineData::ExternalStream {
+ stdout: Some(s), ..
+ } => {
  let mut items = vec![];
 
  for val in s {
@@ -236,7 +243,13 @@ impl PipelineData {
  Ok(vals.into_iter().map(f).into_pipeline_data(ctrlc))
  }
  PipelineData::ListStream(stream, ..) => Ok(stream.map(f).into_pipeline_data(ctrlc)),
- PipelineData::ExternalStream { stdout: stream, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => {
+ Ok(PipelineData::new(Span { start: 0, end: 0 }))
+ }
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => {
  let collected = stream.into_bytes()?;
 
  if let Ok(st) = String::from_utf8(collected.clone().item) {
@@ -283,7 +296,13 @@ impl PipelineData {
  PipelineData::ListStream(stream, ..) => {
  Ok(stream.flat_map(f).into_pipeline_data(ctrlc))
  }
- PipelineData::ExternalStream { stdout: stream, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => {
+ Ok(PipelineData::new(Span { start: 0, end: 0 }))
+ }
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => {
  let collected = stream.into_bytes()?;
 
  if let Ok(st) = String::from_utf8(collected.clone().item) {
@@ -324,7 +343,13 @@ impl PipelineData {
  Ok(vals.into_iter().filter(f).into_pipeline_data(ctrlc))
  }
  PipelineData::ListStream(stream, ..) => Ok(stream.filter(f).into_pipeline_data(ctrlc)),
- PipelineData::ExternalStream { stdout: stream, .. } => {
+ PipelineData::ExternalStream { stdout: None, .. } => {
+ Ok(PipelineData::new(Span { start: 0, end: 0 }))
+ }
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => {
  let collected = stream.into_bytes()?;
 
  if let Ok(st) = String::from_utf8(collected.clone().item) {
@@ -414,7 +439,11 @@ impl Iterator for PipelineIterator {
  PipelineData::Value(Value::Nothing { .. }, ..) => None,
  PipelineData::Value(v, ..) => Some(std::mem::take(v)),
  PipelineData::ListStream(stream, ..) => stream.next(),
- PipelineData::ExternalStream { stdout: stream, .. } => stream.next().map(|x| match x {
+ PipelineData::ExternalStream { stdout: None, .. } => None,
+ PipelineData::ExternalStream {
+ stdout: Some(stream),
+ ..
+ } => stream.next().map(|x| match x {
  Ok(x) => x,
  Err(err) => Value::Error { error: err },
  }),

diff --git a/src/eval_file.rs b/src/eval_file.rs
@@ -100,7 +100,12 @@ pub fn print_table_or_error(
  );
 
  match table {
- Ok(table) => {
+ Ok(mut table) => {
+ let exit_code = match &mut table {
+ PipelineData::ExternalStream { exit_code, .. } => exit_code.take(),
+ _ => None,
+ };
+
  for item in table {
  let stdout = std::io::stdout();
 
@@ -120,6 +125,10 @@ pub fn print_table_or_error(
  Err(err) => eprintln!("{}", err),
  };
  }
+
+ if let Some(exit_code) = exit_code {
+ let _: Vec<_> = exit_code.into_iter().collect();
+ }
  }
  Err(error) => {
  let working_set = StateWorkingSet::new(engine_state);

diff --git a/src/main.rs b/src/main.rs
@@ -172,11 +172,11 @@ fn main() -> Result<()> {
  let buf_reader = BufReader::new(stdin);
 
  PipelineData::ExternalStream {
- stdout: RawStream::new(
+ stdout: Some(RawStream::new(
  Box::new(BufferedReader::new(buf_reader)),
  Some(ctrlc),
  redirect_stdin.span,
- ),
+ )),
  stderr: None,
  exit_code: None,
  span: redirect_stdin.span,