From 299fea8538361ff841decbf7d1f7e8ff8dfc7ef9 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Mon, 7 Mar 2022 20:17:33 -0500 Subject: [PATCH] Fix external extra (#4777) * Fix empty table from externals * Fix empty table from externals --- crates/nu-cli/src/util.rs | 6 ++- .../nu-command/src/conversions/into/binary.rs | 10 ++++- .../nu-command/src/conversions/into/string.rs | 10 ++++- crates/nu-command/src/filesystem/open.rs | 4 +- crates/nu-command/src/filters/each.rs | 6 ++- crates/nu-command/src/filters/par_each.rs | 6 ++- crates/nu-command/src/filters/skip/skip_.rs | 2 +- crates/nu-command/src/network/fetch.rs | 4 +- crates/nu-command/src/network/post.rs | 4 +- crates/nu-command/src/strings/decode.rs | 6 ++- crates/nu-command/src/system/complete.rs | 29 +++++++------ crates/nu-command/src/system/run_external.rs | 10 ++++- crates/nu-command/src/viewers/table.rs | 8 ++-- crates/nu-command/tests/commands/mod.rs | 1 + .../nu-command/tests/commands/run_external.rs | 15 +++++++ crates/nu-protocol/src/pipeline_data.rs | 43 ++++++++++++++++--- src/eval_file.rs | 11 ++++- src/main.rs | 4 +- 18 files changed, 138 insertions(+), 41 deletions(-) create mode 100644 crates/nu-command/tests/commands/run_external.rs diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs index 0674c872b..f7d12481d 100644 --- 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 index 570d765ac..c046c30d4 100644 --- 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 = 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 index f4eaa90a6..298a072a8 100644 --- 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 index c4c00f23e..f2d47c5cb 100644 --- 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 index 8901795bb..2dda3e93d 100644 --- 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 index ce602fb14..b23abf131 100644 --- 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 index bd0d1722a..5cb285321 100644 --- 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 index d21de6069..404b3fb66 100644 --- 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 index afc6dfd5a..178154d11 100644 --- 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 index 71e5f9d24..5f9322718 100644 --- 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 = 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 = 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 index 5ae77360d..a1c758e30 100644 --- 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 index 46daf6ab3..9244c56fc 100644 --- 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 index bb2ed6e7c..9c789e74e 100644 --- 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 index 4e09f4f28..bb17eddad 100644 --- 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 new file mode 100644 index 000000000..707105091 --- /dev/null +++ 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 index b88164652..cb6f16640 100644 --- 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), ListStream(ListStream, Option), ExternalStream { - stdout: RawStream, + stdout: Option, stderr: Option, exit_code: Option, 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 index d96f95f40..e6f77d0eb 100644 --- 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 index 7d2889672..fbf163ecd 100644 --- 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,