IO and redirection overhaul (#11934)

# Description
The PR overhauls how IO redirection is handled, allowing more explicit
and fine-grain control over `stdout` and `stderr` output as well as more
efficient IO and piping.

To summarize the changes in this PR:
- Added a new `IoStream` type to indicate the intended destination for a
pipeline element's `stdout` and `stderr`.
- The `stdout` and `stderr` `IoStream`s are stored in the `Stack` and to
avoid adding 6 additional arguments to every eval function and
`Command::run`. The `stdout` and `stderr` streams can be temporarily
overwritten through functions on `Stack` and these functions will return
a guard that restores the original `stdout` and `stderr` when dropped.
- In the AST, redirections are now directly part of a `PipelineElement`
as a `Option<Redirection>` field instead of having multiple different
`PipelineElement` enum variants for each kind of redirection. This
required changes to the parser, mainly in `lite_parser.rs`.
- `Command`s can also set a `IoStream` override/redirection which will
apply to the previous command in the pipeline. This is used, for
example, in `ignore` to allow the previous external command to have its
stdout redirected to `Stdio::null()` at spawn time. In contrast, the
current implementation has to create an os pipe and manually consume the
output on nushell's side. File and pipe redirections (`o>`, `e>`, `e>|`,
etc.) have precedence over overrides from commands.

This PR improves piping and IO speed, partially addressing #10763. Using
the `throughput` command from that issue, this PR gives the following
speedup on my setup for the commands below:
| Command | Before (MB/s) | After (MB/s) | Bash (MB/s) |
| --------------------------- | -------------:| ------------:|
-----------:|
| `throughput o> /dev/null` | 1169 | 52938 | 54305 |
| `throughput \| ignore` | 840 | 55438 | N/A |
| `throughput \| null` | Error | 53617 | N/A |
| `throughput \| rg 'x'` | 1165 | 3049 | 3736 |
| `(throughput) \| rg 'x'` | 810 | 3085 | 3815 |

(Numbers above are the median samples for throughput)

This PR also paves the way to refactor our `ExternalStream` handling in
the various commands. For example, this PR already fixes the following
code:
```nushell
^sh -c 'echo -n "hello "; sleep 0; echo "world"' | find "hello world"
```
This returns an empty list on 0.90.1 and returns a highlighted "hello
world" on this PR.

Since the `stdout` and `stderr` `IoStream`s are available to commands
when they are run, then this unlocks the potential for more convenient
behavior. E.g., the `find` command can disable its ansi highlighting if
it detects that the output `IoStream` is not the terminal. Knowing the
output streams will also allow background job output to be redirected
more easily and efficiently.

# User-Facing Changes
- External commands returned from closures will be collected (in most
cases):
  ```nushell
  1..2 | each {|_| nu -c "print a" }
  ```
This gives `["a", "a"]` on this PR, whereas this used to print "a\na\n"
and then return an empty list.

  ```nushell
  1..2 | each {|_| nu -c "print -e a" }
  ```
This gives `["", ""]` and prints "a\na\n" to stderr, whereas this used
to return an empty list and print "a\na\n" to stderr.

- Trailing new lines are always trimmed for external commands when
piping into internal commands or collecting it as a value. (Failure to
decode the output as utf-8 will keep the trailing newline for the last
binary value.) In the current nushell version, the following three code
snippets differ only in parenthesis placement, but they all also have
different outputs:

  1. `1..2 | each { ^echo a }`
     ```
     a
     a
     ╭────────────╮
     │ empty list │
     ╰────────────╯
     ```
  2. `1..2 | each { (^echo a) }`
     ```
     ╭───┬───╮
     │ 0 │ a │
     │ 1 │ a │
     ╰───┴───╯
     ```
  3. `1..2 | (each { ^echo a })`
     ```
     ╭───┬───╮
     │ 0 │ a │
     │   │   │
     │ 1 │ a │
     │   │   │
     ╰───┴───╯
     ```

  But in this PR, the above snippets will all have the same output:
  ```
  ╭───┬───╮
  │ 0 │ a │
  │ 1 │ a │
  ╰───┴───╯
  ```

- All existing flags on `run-external` are now deprecated.

- File redirections now apply to all commands inside a code block:
  ```nushell
  (nu -c "print -e a"; nu -c "print -e b") e> test.out
  ```
This gives "a\nb\n" in `test.out` and prints nothing. The same result
would happen when printing to stdout and using a `o>` file redirection.

- External command output will (almost) never be ignored, and ignoring
output must be explicit now:
  ```nushell
  (^echo a; ^echo b)
  ```
This prints "a\nb\n", whereas this used to print only "b\n". This only
applies to external commands; values and internal commands not in return
position will not print anything (e.g., `(echo a; echo b)` still only
prints "b").

- `complete` now always captures stderr (`do` is not necessary).

# After Submitting
The language guide and other documentation will need to be updated.
This commit is contained in:
Ian Manske
2024-03-14 20:51:55 +00:00
committed by GitHub
parent e2907e7e3a
commit b6c7656194
113 changed files with 3272 additions and 4022 deletions

View File

@ -73,8 +73,9 @@ pub fn get_pipeline_elements(
let mut i = 0;
while i < pipeline.elements.len() {
let pipeline_element = &pipeline.elements[i];
let pipeline_expression = pipeline_element.expression().clone();
let pipeline_span = pipeline_element.span();
let pipeline_expression = &pipeline_element.expr;
let pipeline_span = pipeline_element.expr.span;
let element_str =
String::from_utf8_lossy(engine_state.get_span_contents(pipeline_span));
let value = Value::string(element_str.to_string(), pipeline_span);

View File

@ -133,8 +133,6 @@ confusing the id/parent_id hierarchy. The --expr flag is helpful for investigati
&mut callee_stack,
block,
input,
call.redirect_stdout,
call.redirect_stdout,
);
// TODO: See eval_source()

View File

@ -55,25 +55,11 @@ impl Command for TimeIt {
if let Some(block_id) = command_to_run.as_block() {
let eval_block = get_eval_block(engine_state);
let block = engine_state.get_block(block_id);
eval_block(
engine_state,
stack,
block,
input,
call.redirect_stdout,
call.redirect_stderr,
)?
eval_block(engine_state, stack, block, input)?
} else {
let eval_expression_with_input = get_eval_expression_with_input(engine_state);
eval_expression_with_input(
engine_state,
stack,
command_to_run,
input,
call.redirect_stdout,
call.redirect_stderr,
)
.map(|res| res.0)?
eval_expression_with_input(engine_state, stack, command_to_run, input)
.map(|res| res.0)?
}
} else {
PipelineData::empty()

View File

@ -1,7 +1,7 @@
use std::collections::HashMap;
use std::path::PathBuf;
use nu_protocol::{Span, Spanned};
use nu_protocol::{IoStream, Span, Spanned};
use crate::ExternalCommand;
@ -32,10 +32,8 @@ pub(crate) fn gen_command(
name,
args,
arg_keep_raw: vec![false; number_of_args],
redirect_stdout: false,
redirect_stderr: false,
redirect_combine: false,
out: IoStream::Inherit,
err: IoStream::Inherit,
env_vars: env_vars_str,
trim_end_newline: false,
}
}

View File

@ -38,18 +38,13 @@ impl Command for ExportEnv {
) -> Result<PipelineData, ShellError> {
let capture_block: Closure = call.req(engine_state, caller_stack, 0)?;
let block = engine_state.get_block(capture_block.block_id);
let mut callee_stack = caller_stack.captures_to_stack(capture_block.captures);
let mut callee_stack = caller_stack
.captures_to_stack(capture_block.captures)
.reset_pipes();
let eval_block = get_eval_block(engine_state);
let _ = eval_block(
engine_state,
&mut callee_stack,
block,
input,
call.redirect_stdout,
call.redirect_stderr,
);
let _ = eval_block(engine_state, &mut callee_stack, block, input);
redirect_env(engine_state, caller_stack, &callee_stack);

View File

@ -76,18 +76,13 @@ impl Command for SourceEnv {
// Evaluate the block
let block = engine_state.get_block(block_id as usize).clone();
let mut callee_stack = caller_stack.gather_captures(engine_state, &block.captures);
let mut callee_stack = caller_stack
.gather_captures(engine_state, &block.captures)
.reset_pipes();
let eval_block_with_early_return = get_eval_block_with_early_return(engine_state);
let result = eval_block_with_early_return(
engine_state,
&mut callee_stack,
&block,
input,
call.redirect_stdout,
call.redirect_stderr,
);
let result = eval_block_with_early_return(engine_state, &mut callee_stack, &block, input);
// Merge the block's environment to the current stack
redirect_env(engine_state, caller_stack, &callee_stack);

View File

@ -81,12 +81,11 @@ fn with_env(
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
// let external_redirection = args.call_info.args.external_redirection;
let variable: Value = call.req(engine_state, stack, 0)?;
let capture_block: Closure = call.req(engine_state, stack, 1)?;
let block = engine_state.get_block(capture_block.block_id);
let mut stack = stack.captures_to_stack(capture_block.captures);
let mut stack = stack.captures_to_stack_preserve_stdio(capture_block.captures);
let mut env: HashMap<String, Value> = HashMap::new();
@ -145,14 +144,7 @@ fn with_env(
stack.add_env_var(k, v);
}
eval_block::<WithoutDebug>(
engine_state,
&mut stack,
block,
input,
call.redirect_stdout,
call.redirect_stderr,
)
eval_block::<WithoutDebug>(engine_state, &mut stack, block, input)
}
#[cfg(test)]

View File

@ -194,7 +194,7 @@ impl Command for Open {
let decl = engine_state.get_decl(converter_id);
let command_output = if let Some(block_id) = decl.get_block_id() {
let block = engine_state.get_block(block_id);
eval_block(engine_state, stack, block, file_contents, false, false)
eval_block(engine_state, stack, block, file_contents)
} else {
decl.run(engine_state, stack, &Call::new(call_span), file_contents)
};

View File

@ -5,7 +5,7 @@ use nu_protocol::ast::{Call, Expr, Expression};
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::IntoSpanned;
use nu_protocol::{
Category, DataSource, Example, PipelineData, PipelineMetadata, RawStream, ShellError,
Category, DataSource, Example, IoStream, PipelineData, PipelineMetadata, RawStream, ShellError,
Signature, Span, Spanned, SyntaxShape, Type, Value,
};
use std::fs::File;
@ -104,16 +104,7 @@ impl Command for Save {
});
match input {
PipelineData::ExternalStream { stdout: None, .. } => {
// Open files to possibly truncate them
let _ = get_files(&path, stderr_path.as_ref(), append, false, false, force)?;
Ok(PipelineData::empty())
}
PipelineData::ExternalStream {
stdout: Some(stream),
stderr,
..
} => {
PipelineData::ExternalStream { stdout, stderr, .. } => {
let (file, stderr_file) = get_files(
&path,
stderr_path.as_ref(),
@ -123,35 +114,42 @@ impl Command for Save {
force,
)?;
// delegate a thread to redirect stderr to result.
let handler = stderr
.map(|stderr_stream| match stderr_file {
Some(stderr_file) => thread::Builder::new()
.name("stderr redirector".to_string())
.spawn(move || {
stream_to_file(stderr_stream, stderr_file, span, progress)
}),
None => thread::Builder::new()
.name("stderr redirector".to_string())
.spawn(move || {
let _ = stderr_stream.into_bytes();
Ok(PipelineData::empty())
}),
})
.transpose()
.map_err(|e| e.into_spanned(span))?;
match (stdout, stderr) {
(Some(stdout), stderr) => {
// delegate a thread to redirect stderr to result.
let handler = stderr
.map(|stderr| match stderr_file {
Some(stderr_file) => thread::Builder::new()
.name("stderr redirector".to_string())
.spawn(move || {
stream_to_file(stderr, stderr_file, span, progress)
}),
None => thread::Builder::new()
.name("stderr redirector".to_string())
.spawn(move || stderr.drain()),
})
.transpose()
.map_err(|e| e.into_spanned(span))?;
let res = stream_to_file(stream, file, span, progress);
if let Some(h) = handler {
h.join().map_err(|err| ShellError::ExternalCommand {
label: "Fail to receive external commands stderr message".to_string(),
help: format!("{err:?}"),
span,
})??;
res
} else {
res
}
let res = stream_to_file(stdout, file, span, progress);
if let Some(h) = handler {
h.join().map_err(|err| ShellError::ExternalCommand {
label: "Fail to receive external commands stderr message"
.to_string(),
help: format!("{err:?}"),
span,
})??;
}
res?;
}
(None, Some(stderr)) => match stderr_file {
Some(stderr_file) => stream_to_file(stderr, stderr_file, span, progress)?,
None => stderr.drain()?,
},
(None, None) => {}
};
Ok(PipelineData::Empty)
}
PipelineData::ListStream(ls, pipeline_metadata)
if raw || prepare_path(&path, append, force)?.0.extension().is_none() =>
@ -265,6 +263,10 @@ impl Command for Save {
},
]
}
fn stdio_redirect(&self) -> (Option<IoStream>, Option<IoStream>) {
(Some(IoStream::Capture), Some(IoStream::Capture))
}
}
/// Convert [`PipelineData`] bytes to write in file, possibly converting
@ -430,13 +432,13 @@ fn get_files(
fn stream_to_file(
mut stream: RawStream,
file: File,
mut file: File,
span: Span,
progress: bool,
) -> Result<PipelineData, ShellError> {
) -> Result<(), ShellError> {
// https://github.com/nushell/nushell/pull/9377 contains the reason
// for not using BufWriter<File>
let mut writer = file;
let writer = &mut file;
let mut bytes_processed: u64 = 0;
let bytes_processed_p = &mut bytes_processed;
@ -456,47 +458,45 @@ fn stream_to_file(
(None, None)
};
let result = stream
.try_for_each(move |result| {
let buf = match result {
Ok(v) => match v {
Value::String { val, .. } => val.into_bytes(),
Value::Binary { val, .. } => val,
// Propagate errors by explicitly matching them before the final case.
Value::Error { error, .. } => return Err(*error),
other => {
return Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "string or binary".into(),
wrong_type: other.get_type().to_string(),
dst_span: span,
src_span: other.span(),
});
}
},
Err(err) => {
*process_failed_p = true;
return Err(err);
stream.try_for_each(move |result| {
let buf = match result {
Ok(v) => match v {
Value::String { val, .. } => val.into_bytes(),
Value::Binary { val, .. } => val,
// Propagate errors by explicitly matching them before the final case.
Value::Error { error, .. } => return Err(*error),
other => {
return Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "string or binary".into(),
wrong_type: other.get_type().to_string(),
dst_span: span,
src_span: other.span(),
});
}
};
// If the `progress` flag is set then
if progress {
// Update the total amount of bytes that has been saved and then print the progress bar
*bytes_processed_p += buf.len() as u64;
if let Some(bar) = &mut bar_opt {
bar.update_bar(*bytes_processed_p);
}
}
if let Err(err) = writer.write(&buf) {
},
Err(err) => {
*process_failed_p = true;
return Err(ShellError::IOError {
msg: err.to_string(),
});
return Err(err);
}
Ok(())
})
.map(|_| PipelineData::empty());
};
// If the `progress` flag is set then
if progress {
// Update the total amount of bytes that has been saved and then print the progress bar
*bytes_processed_p += buf.len() as u64;
if let Some(bar) = &mut bar_opt {
bar.update_bar(*bytes_processed_p);
}
}
if let Err(err) = writer.write_all(&buf) {
*process_failed_p = true;
return Err(ShellError::IOError {
msg: err.to_string(),
});
}
Ok(())
})?;
// If the `progress` flag is set then
if progress {
@ -508,6 +508,7 @@ fn stream_to_file(
}
}
// And finally return the stream result.
result
file.flush()?;
Ok(())
}

View File

@ -217,8 +217,6 @@ impl Command for Watch {
stack,
&block,
Value::nothing(call.span()).into_pipeline_data(),
call.redirect_stdout,
call.redirect_stderr,
);
match eval_result {

View File

@ -129,8 +129,6 @@ with 'transpose' first."#
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let span = call.head;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
match input {
PipelineData::Empty => Ok(PipelineData::Empty),
@ -157,10 +155,6 @@ with 'transpose' first."#
&mut stack,
&block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
// WithoutDebug,
// &None,
) {
Ok(v) => Some(v.into_value(span)),
Err(ShellError::Continue { span }) => Some(Value::nothing(span)),
@ -205,8 +199,6 @@ with 'transpose' first."#
&mut stack,
&block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => Some(v.into_value(span)),
Err(ShellError::Continue { span }) => Some(Value::nothing(span)),
@ -232,8 +224,6 @@ with 'transpose' first."#
&mut stack,
&block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
)
}
}

View File

@ -62,8 +62,6 @@ a variable. On the other hand, the "row condition" syntax is not supported."#
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let span = call.head;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(&engine_state);
match input {
@ -92,8 +90,6 @@ a variable. On the other hand, the "row condition" syntax is not supported."#
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
@ -136,8 +132,6 @@ a variable. On the other hand, the "row condition" syntax is not supported."#
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
@ -171,8 +165,6 @@ a variable. On the other hand, the "row condition" syntax is not supported."#
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {

View File

@ -171,7 +171,7 @@ pub fn group_by(
Value::CellPath { val, .. } => group_cell_path(val, values)?,
Value::Block { .. } | Value::Closure { .. } => {
let block: Option<Closure> = call.opt(engine_state, stack, 0)?;
group_closure(values, span, block, stack, engine_state, call)?
group_closure(values, span, block, stack, engine_state)?
}
_ => {
@ -234,7 +234,6 @@ fn group_closure(
block: Option<Closure>,
stack: &mut Stack,
engine_state: &EngineState,
call: &Call,
) -> Result<IndexMap<String, Vec<Value>>, ShellError> {
let error_key = "error";
let mut groups: IndexMap<String, Vec<Value>> = IndexMap::new();
@ -251,8 +250,6 @@ fn group_closure(
&mut stack,
block,
value.clone().into_pipeline_data(),
call.redirect_stdout,
call.redirect_stderr,
);
let group_key = match pipeline {

View File

@ -130,9 +130,6 @@ fn insert(
let cell_path: CellPath = call.req(engine_state, stack, 0)?;
let replacement: Value = call.req(engine_state, stack, 1)?;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let ctrlc = engine_state.ctrlc.clone();
let eval_block = get_eval_block(engine_state);
@ -153,8 +150,6 @@ fn insert(
span,
engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
block,
&cell_path.members,
false,
@ -168,8 +163,6 @@ fn insert(
replacement,
engine_state,
stack,
redirect_stdout,
redirect_stderr,
&cell_path.members,
matches!(first, Some(PathMember::Int { .. })),
eval_block,
@ -225,8 +218,6 @@ fn insert(
&mut stack,
block,
value.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
)?;
pre_elems.push(output.into_value(span));
@ -243,8 +234,6 @@ fn insert(
replacement,
engine_state,
stack,
redirect_stdout,
redirect_stderr,
path,
true,
eval_block,
@ -282,8 +271,6 @@ fn insert(
replacement_span,
&engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
&block,
&cell_path.members,
false,
@ -330,8 +317,6 @@ fn insert_value_by_closure(
span: Span,
engine_state: &EngineState,
stack: &mut Stack,
redirect_stdout: bool,
redirect_stderr: bool,
block: &Block,
cell_path: &[PathMember],
first_path_member_int: bool,
@ -356,14 +341,7 @@ fn insert_value_by_closure(
.map(IntoPipelineData::into_pipeline_data)
.unwrap_or(PipelineData::Empty);
let output = eval_block_fn(
engine_state,
stack,
block,
input_at_path,
redirect_stdout,
redirect_stderr,
)?;
let output = eval_block_fn(engine_state, stack, block, input_at_path)?;
value.insert_data_at_cell_path(cell_path, output.into_value(span), span)
}
@ -374,8 +352,6 @@ fn insert_single_value_by_closure(
replacement: Value,
engine_state: &EngineState,
stack: &mut Stack,
redirect_stdout: bool,
redirect_stderr: bool,
cell_path: &[PathMember],
first_path_member_int: bool,
eval_block_fn: EvalBlockFn,
@ -390,8 +366,6 @@ fn insert_single_value_by_closure(
span,
engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
block,
cell_path,
first_path_member_int,

View File

@ -128,14 +128,7 @@ interleave
// Evaluate the closure on this thread
let block = engine_state.get_block(closure.block_id);
let mut stack = stack.captures_to_stack(closure.captures);
eval_block_with_early_return(
engine_state,
&mut stack,
block,
PipelineData::Empty,
true,
false,
)
eval_block_with_early_return(engine_state, &mut stack, block, PipelineData::Empty)
}))
.try_for_each(|stream| {
stream.and_then(|stream| {

View File

@ -54,7 +54,6 @@ impl Command for Items {
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let span = call.head;
let redirect_stderr = call.redirect_stderr;
let eval_block_with_early_return = get_eval_block_with_early_return(&engine_state);
let input_span = input.span().unwrap_or(call.head);
@ -81,8 +80,6 @@ impl Command for Items {
&mut stack,
&block,
PipelineData::empty(),
true,
redirect_stderr,
) {
Ok(v) => Some(v.into_value(span)),
Err(ShellError::Break { .. }) => None,

View File

@ -129,8 +129,6 @@ impl Command for ParEach {
let block_id = capture_block.block_id;
let mut stack = stack.captures_to_stack(capture_block.captures);
let span = call.head;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
// A helper function sorts the output if needed
let apply_order = |mut vec: Vec<(usize, Value)>| {
@ -173,8 +171,6 @@ impl Command for ParEach {
&mut stack,
block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => v.into_value(span),
Err(error) => Value::error(
@ -213,8 +209,6 @@ impl Command for ParEach {
&mut stack,
block,
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => v.into_value(span),
Err(error) => Value::error(
@ -252,8 +246,6 @@ impl Command for ParEach {
&mut stack,
block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => v.into_value(span),
Err(error) => Value::error(
@ -297,8 +289,6 @@ impl Command for ParEach {
&mut stack,
block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => v.into_value(span),
Err(error) => Value::error(error, span),
@ -326,8 +316,6 @@ impl Command for ParEach {
&mut stack,
block,
x.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
)
}
}

View File

@ -107,9 +107,6 @@ impl Command for Reduce {
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
// To enumerate over the input (for the index argument),
// it must be converted into an iterator using into_iter().
let mut input_iter = input.into_iter();
@ -130,9 +127,7 @@ impl Command for Reduce {
let mut acc = start_val;
let mut input_iter = input_iter.peekable();
while let Some(x) = input_iter.next() {
for x in input_iter {
// with_env() is used here to ensure that each iteration uses
// a different set of environment variables.
// Hence, a 'cd' in the first loop won't affect the next loop.
@ -157,9 +152,6 @@ impl Command for Reduce {
&mut stack,
block,
PipelineData::empty(),
// redirect stdout until its the last input value
redirect_stdout || input_iter.peek().is_some(),
redirect_stderr,
)?
.into_value(span);

View File

@ -140,8 +140,6 @@ fn rename(
}
None => None,
};
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let block_info =
if let Some(capture_block) = call.get_flag::<Closure>(engine_state, stack, "block")? {
let engine_state = engine_state.clone();
@ -185,8 +183,6 @@ fn rename(
&mut stack,
&block,
Value::string(c.clone(), span).into_pipeline_data(),
redirect_stdout,
redirect_stderr,
);
match eval_result {

View File

@ -92,9 +92,6 @@ impl Command for SkipUntil {
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(&engine_state);
Ok(input
@ -104,17 +101,10 @@ impl Command for SkipUntil {
stack.add_var(var_id, value.clone());
}
!eval_block(
&engine_state,
&mut stack,
&block,
PipelineData::empty(),
redirect_stdout,
redirect_stderr,
)
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
!eval_block(&engine_state, &mut stack, &block, PipelineData::empty())
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
})
.into_pipeline_data_with_metadata(metadata, ctrlc))
}

View File

@ -97,9 +97,6 @@ impl Command for SkipWhile {
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(&engine_state);
Ok(input
@ -109,17 +106,10 @@ impl Command for SkipWhile {
stack.add_var(var_id, value.clone());
}
eval_block(
&engine_state,
&mut stack,
&block,
PipelineData::empty(),
redirect_stdout,
redirect_stderr,
)
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
eval_block(&engine_state, &mut stack, &block, PipelineData::empty())
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
})
.into_pipeline_data_with_metadata(metadata, ctrlc))
}

View File

@ -89,9 +89,6 @@ impl Command for TakeUntil {
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(&engine_state);
Ok(input
@ -101,17 +98,10 @@ impl Command for TakeUntil {
stack.add_var(var_id, value.clone());
}
!eval_block(
&engine_state,
&mut stack,
&block,
PipelineData::empty(),
redirect_stdout,
redirect_stderr,
)
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
!eval_block(&engine_state, &mut stack, &block, PipelineData::empty())
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
})
.into_pipeline_data_with_metadata(metadata, ctrlc))
}

View File

@ -88,9 +88,6 @@ impl Command for TakeWhile {
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(&engine_state);
Ok(input
@ -100,17 +97,10 @@ impl Command for TakeWhile {
stack.add_var(var_id, value.clone());
}
eval_block(
&engine_state,
&mut stack,
&block,
PipelineData::empty(),
redirect_stdout,
redirect_stderr,
)
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
eval_block(&engine_state, &mut stack, &block, PipelineData::empty())
.map_or(false, |pipeline_data| {
pipeline_data.into_value(span).is_true()
})
})
.into_pipeline_data_with_metadata(metadata, ctrlc))
}

View File

@ -4,8 +4,8 @@ use nu_engine::{get_eval_block_with_early_return, CallExt};
use nu_protocol::{
ast::Call,
engine::{Closure, Command, EngineState, Stack},
Category, Example, IntoInterruptiblePipelineData, IntoSpanned, PipelineData, RawStream,
ShellError, Signature, Spanned, SyntaxShape, Type, Value,
Category, Example, IntoInterruptiblePipelineData, IntoSpanned, IoStream, PipelineData,
RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, Value,
};
#[derive(Clone)]
@ -49,8 +49,8 @@ use it in your pipeline."#
result: None,
},
Example {
example: "do { nu --commands 'print -e error; print ok' } | \
tee --stderr { save error.log } | complete",
example:
"nu -c 'print -e error; print ok' | tee --stderr { save error.log } | complete",
description: "Save error messages from an external command to a file without \
redirecting them",
result: None,
@ -78,7 +78,9 @@ use it in your pipeline."#
} = call.req(engine_state, stack, 0)?;
let closure_engine_state = engine_state.clone();
let mut closure_stack = stack.captures_to_stack(captures);
let mut closure_stack = stack
.captures_to_stack_preserve_stdio(captures)
.reset_pipes();
let metadata = input.metadata();
let metadata_clone = metadata.clone();
@ -121,46 +123,32 @@ use it in your pipeline."#
&mut closure_stack,
closure_engine_state.get_block(block_id),
input_from_channel,
false,
false,
);
// Make sure to drain any iterator produced to avoid unexpected behavior
result.and_then(|data| data.drain())
};
if use_stderr {
if let Some(stderr) = stderr {
let iter = tee(stderr.stream, with_stream)
.map_err(|e| e.into_spanned(call.head))?;
let raw_stream = RawStream::new(
Box::new(iter.map(flatten_result)),
stderr.ctrlc,
stderr.span,
stderr.known_size,
);
Ok(PipelineData::ExternalStream {
stdout,
stderr: Some(raw_stream),
exit_code,
span,
metadata,
trim_end_newline,
let stderr = stderr
.map(|stderr| {
let iter = tee(stderr.stream, with_stream)
.map_err(|e| e.into_spanned(call.head))?;
Ok::<_, ShellError>(RawStream::new(
Box::new(iter.map(flatten_result)),
stderr.ctrlc,
stderr.span,
stderr.known_size,
))
})
} else {
// Throw an error if the stream doesn't have stderr. This is probably the
// user's mistake (e.g., forgetting to use `do`)
Err(ShellError::GenericError {
error: "Stream passed to `tee --stderr` does not have stderr".into(),
msg: "this stream does not contain stderr".into(),
span: Some(span),
help: Some(
"if this is an external command, you probably need to wrap \
it in `do { ... }`"
.into(),
),
inner: vec![],
})
}
.transpose()?;
Ok(PipelineData::ExternalStream {
stdout,
stderr,
exit_code,
span,
metadata,
trim_end_newline,
})
} else {
let stdout = stdout
.map(|stdout| {
@ -203,8 +191,6 @@ use it in your pipeline."#
&mut closure_stack,
closure_engine_state.get_block(block_id),
input_from_channel,
false,
false,
);
// Make sure to drain any iterator produced to avoid unexpected behavior
result.and_then(|data| data.drain())
@ -217,6 +203,10 @@ use it in your pipeline."#
}
}
}
fn stdio_redirect(&self) -> (Option<IoStream>, Option<IoStream>) {
(Some(IoStream::Capture), Some(IoStream::Capture))
}
}
fn panic_error() -> ShellError {

View File

@ -112,9 +112,6 @@ fn update(
let cell_path: CellPath = call.req(engine_state, stack, 0)?;
let replacement: Value = call.req(engine_state, stack, 1)?;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let ctrlc = engine_state.ctrlc.clone();
let eval_block = get_eval_block(engine_state);
@ -135,8 +132,6 @@ fn update(
span,
engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
block,
&cell_path.members,
false,
@ -150,8 +145,6 @@ fn update(
replacement,
engine_state,
stack,
redirect_stdout,
redirect_stderr,
&cell_path.members,
matches!(first, Some(PathMember::Int { .. })),
eval_block,
@ -197,8 +190,6 @@ fn update(
replacement,
engine_state,
stack,
redirect_stdout,
redirect_stderr,
path,
true,
eval_block,
@ -229,8 +220,6 @@ fn update(
replacement_span,
&engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
&block,
&cell_path.members,
false,
@ -275,8 +264,6 @@ fn update_value_by_closure(
span: Span,
engine_state: &EngineState,
stack: &mut Stack,
redirect_stdout: bool,
redirect_stderr: bool,
block: &Block,
cell_path: &[PathMember],
first_path_member_int: bool,
@ -302,8 +289,6 @@ fn update_value_by_closure(
stack,
block,
input_at_path.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
)?;
value.update_data_at_cell_path(cell_path, output.into_value(span))
@ -315,8 +300,6 @@ fn update_single_value_by_closure(
replacement: Value,
engine_state: &EngineState,
stack: &mut Stack,
redirect_stdout: bool,
redirect_stderr: bool,
cell_path: &[PathMember],
first_path_member_int: bool,
eval_block_fn: EvalBlockFn,
@ -331,8 +314,6 @@ fn update_single_value_by_closure(
span,
engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
block,
cell_path,
first_path_member_int,

View File

@ -156,9 +156,6 @@ fn upsert(
let cell_path: CellPath = call.req(engine_state, stack, 0)?;
let replacement: Value = call.req(engine_state, stack, 1)?;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(engine_state);
let ctrlc = engine_state.ctrlc.clone();
@ -179,8 +176,6 @@ fn upsert(
span,
engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
block,
&cell_path.members,
false,
@ -194,8 +189,6 @@ fn upsert(
replacement,
engine_state,
stack,
redirect_stdout,
redirect_stderr,
&cell_path.members,
matches!(first, Some(PathMember::Int { .. })),
eval_block,
@ -249,8 +242,6 @@ fn upsert(
&mut stack,
block,
value.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
)?;
pre_elems.push(output.into_value(span));
@ -264,8 +255,6 @@ fn upsert(
replacement,
engine_state,
stack,
redirect_stdout,
redirect_stderr,
path,
true,
eval_block,
@ -303,8 +292,6 @@ fn upsert(
replacement_span,
&engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
&block,
&cell_path.members,
false,
@ -349,8 +336,6 @@ fn upsert_value_by_closure(
span: Span,
engine_state: &EngineState,
stack: &mut Stack,
redirect_stdout: bool,
redirect_stderr: bool,
block: &Block,
cell_path: &[PathMember],
first_path_member_int: bool,
@ -375,14 +360,7 @@ fn upsert_value_by_closure(
.map(IntoPipelineData::into_pipeline_data)
.unwrap_or(PipelineData::Empty);
let output = eval_block_fn(
engine_state,
stack,
block,
input_at_path,
redirect_stdout,
redirect_stderr,
)?;
let output = eval_block_fn(engine_state, stack, block, input_at_path)?;
value.upsert_data_at_cell_path(cell_path, output.into_value(span))
}
@ -393,8 +371,6 @@ fn upsert_single_value_by_closure(
replacement: Value,
engine_state: &EngineState,
stack: &mut Stack,
redirect_stdout: bool,
redirect_stderr: bool,
cell_path: &[PathMember],
first_path_member_int: bool,
eval_block_fn: EvalBlockFn,
@ -409,8 +385,6 @@ fn upsert_single_value_by_closure(
span,
engine_state,
&mut stack,
redirect_stdout,
redirect_stderr,
block,
cell_path,
first_path_member_int,

View File

@ -53,14 +53,7 @@ pub fn boolean_fold(
stack.add_var(var_id, value.clone());
}
let eval = eval_block(
engine_state,
&mut stack,
block,
value.into_pipeline_data(),
call.redirect_stdout,
call.redirect_stderr,
);
let eval = eval_block(engine_state, &mut stack, block, value.into_pipeline_data());
match eval {
Err(e) => {
return Err(e);

View File

@ -69,9 +69,6 @@ not supported."#
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block = get_eval_block(&engine_state);
Ok(input
@ -91,8 +88,6 @@ not supported."#
&block,
// clone() is used here because x is given to Ok() below.
value.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
);
match result {

View File

@ -109,14 +109,7 @@ impl Command for Zip {
Value::Closure { val, .. } => {
let block = engine_state.get_block(val.block_id);
let mut stack = stack.captures_to_stack(val.captures);
eval_block_with_early_return(
engine_state,
&mut stack,
block,
PipelineData::Empty,
true,
false,
)?
eval_block_with_early_return(engine_state, &mut stack, block, PipelineData::Empty)?
}
// If any other value, use it as-is.
val => val.into_pipeline_data(),

View File

@ -1,4 +1,4 @@
use nu_protocol::ast::{Call, Expr, Expression, PipelineElement, RecordItem};
use nu_protocol::ast::{Call, Expr, Expression, RecordItem};
use nu_protocol::engine::{Command, EngineState, Stack, StateWorkingSet};
use nu_protocol::{
record, Category, Example, IntoPipelineData, PipelineData, Range, Record, ShellError,
@ -69,7 +69,7 @@ impl Command for FromNuon {
src: string_input,
error: "error when loading".into(),
msg: "excess values when loading".into(),
span: element.span(),
span: element.expr.span,
}],
});
} else {
@ -109,7 +109,7 @@ impl Command for FromNuon {
src: string_input,
error: "error when loading".into(),
msg: "detected a pipeline in nuon file".into(),
span: expr.span(),
span: expr.expr.span,
}],
});
}
@ -122,22 +122,7 @@ impl Command for FromNuon {
ty: Type::Nothing,
}
} else {
match pipeline.elements.remove(0) {
PipelineElement::Expression(_, expression)
| PipelineElement::ErrPipedExpression(_, expression)
| PipelineElement::OutErrPipedExpression(_, expression)
| PipelineElement::Redirection(_, _, expression, _)
| PipelineElement::And(_, expression)
| PipelineElement::Or(_, expression)
| PipelineElement::SameTargetRedirection {
cmd: (_, expression),
..
}
| PipelineElement::SeparateRedirection {
out: (_, expression, _),
..
} => expression,
}
pipeline.elements.remove(0).expr
}
};

View File

@ -106,8 +106,6 @@ used as the next argument to the closure, otherwise generation stops.
let mut stack = stack.captures_to_stack(capture_block.item.captures);
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
let eval_block_with_early_return = get_eval_block_with_early_return(&engine_state);
// A type of Option<S> is used to represent state. Invocation
@ -135,8 +133,6 @@ used as the next argument to the closure, otherwise generation stops.
&mut stack,
&block,
arg.into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
// no data -> output nothing and stop.
Ok(PipelineData::Empty) => (None, None),

View File

@ -51,14 +51,7 @@ impl Command for Source {
let eval_block_with_early_return = get_eval_block_with_early_return(engine_state);
eval_block_with_early_return(
engine_state,
stack,
&block,
input,
call.redirect_stdout,
call.redirect_stderr,
)
eval_block_with_early_return(engine_state, stack, &block, input)
}
fn examples(&self) -> Vec<Example> {

View File

@ -1,8 +1,8 @@
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
Category, Example, IntoPipelineData, IntoSpanned, PipelineData, Record, ShellError, Signature,
Type, Value,
Category, Example, IntoPipelineData, IntoSpanned, IoStream, PipelineData, Record, ShellError,
Signature, Type, Value,
};
use std::thread;
@ -115,19 +115,15 @@ impl Command for Complete {
}
fn examples(&self) -> Vec<Example> {
vec![
Example {
description:
"Run the external command to completion, capturing stdout and exit_code",
example: "^external arg1 | complete",
result: None,
},
Example {
description:
"Run external command to completion, capturing, stdout, stderr and exit_code",
example: "do { ^external arg1 } | complete",
result: None,
},
]
vec![Example {
description:
"Run the external command to completion, capturing stdout, stderr, and exit_code",
example: "^external arg1 | complete",
result: None,
}]
}
fn stdio_redirect(&self) -> (Option<IoStream>, Option<IoStream>) {
(Some(IoStream::Capture), Some(IoStream::Capture))
}
}

View File

@ -3,7 +3,7 @@ use nu_engine::current_dir;
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
Category, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type,
Category, Example, IoStream, PipelineData, ShellError, Signature, Span, SyntaxShape, Type,
};
#[derive(Clone)]
@ -62,8 +62,9 @@ fn exec(
stack: &mut Stack,
call: &Call,
) -> Result<PipelineData, ShellError> {
let external_command =
create_external_command(engine_state, stack, call, false, false, false, false)?;
let mut external_command = create_external_command(engine_state, stack, call)?;
external_command.out = IoStream::Inherit;
external_command.err = IoStream::Inherit;
let cwd = current_dir(engine_state, stack)?;
let mut command = external_command.spawn_simple_command(&cwd.to_string_lossy())?;

View File

@ -2,14 +2,12 @@ use nu_cmd_base::hook::eval_hook;
use nu_engine::env_to_strings;
use nu_engine::get_eval_expression;
use nu_engine::CallExt;
use nu_protocol::IntoSpanned;
use nu_protocol::NuGlob;
use nu_protocol::{
ast::{Call, Expr},
did_you_mean,
engine::{Command, EngineState, Stack},
Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, Span, Spanned,
SyntaxShape, Type, Value,
Category, Example, IntoSpanned, IoStream, ListStream, NuGlob, PipelineData, RawStream,
ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
};
use nu_system::ForegroundChild;
use nu_utils::IgnoreCaseExt;
@ -19,14 +17,9 @@ use std::collections::HashMap;
use std::io::{BufRead, BufReader, Read, Write};
use std::path::{Path, PathBuf};
use std::process::{Command as CommandSys, Stdio};
use std::sync::atomic::AtomicBool;
use std::sync::mpsc::{self, SyncSender};
use std::sync::Arc;
use std::sync::mpsc;
use std::thread;
const OUTPUT_BUFFER_SIZE: usize = 1024;
const OUTPUT_BUFFERS_IN_FLIGHT: usize = 3;
#[derive(Clone)]
pub struct External;
@ -76,15 +69,61 @@ impl Command for External {
});
}
let command = create_external_command(
engine_state,
stack,
call,
redirect_stdout,
redirect_stderr,
redirect_combine,
trim_end_newline,
)?;
if trim_end_newline {
nu_protocol::report_error_new(
engine_state,
&ShellError::GenericError {
error: "Deprecated flag".into(),
msg: "`--trim-end-newline` is deprecated".into(),
span: Some(call.arguments_span()),
help: Some(
"trailing new lines are now removed by default when collecting into a value"
.into(),
),
inner: vec![],
},
);
}
if redirect_combine {
nu_protocol::report_error_new(
engine_state,
&ShellError::GenericError {
error: "Deprecated flag".into(),
msg: "`--redirect-combine` is deprecated".into(),
span: Some(call.arguments_span()),
help: Some("use the `o+e>|` pipe redirection instead".into()),
inner: vec![],
},
);
} else if redirect_stdout {
nu_protocol::report_error_new(
engine_state,
&ShellError::GenericError {
error: "Deprecated flag".into(),
msg: "`--redirect-stdout` is deprecated".into(),
span: Some(call.arguments_span()),
help: Some(
"`run-external` will now always redirect stdout if there is a pipe `|` afterwards"
.into(),
),
inner: vec![],
},
);
} else if redirect_stderr {
nu_protocol::report_error_new(
engine_state,
&ShellError::GenericError {
error: "Deprecated flag".into(),
msg: "`--redirect-stderr` is deprecated".into(),
span: Some(call.arguments_span()),
help: Some("use the `e>|` stderr pipe redirection instead".into()),
inner: vec![],
},
);
}
let command = create_external_command(engine_state, stack, call)?;
command.run_with_input(engine_state, stack, input, false)
}
@ -98,7 +137,12 @@ impl Command for External {
},
Example {
description: "Redirect stdout from an external command into the pipeline",
example: r#"run-external --redirect-stdout "echo" "-n" "hello" | split chars"#,
example: r#"run-external "echo" "-n" "hello" | split chars"#,
result: None,
},
Example {
description: "Redirect stderr from an external command into the pipeline",
example: r#"run-external "nu" "-c" "print -e hello" e>| split chars"#,
result: None,
},
]
@ -110,10 +154,6 @@ pub fn create_external_command(
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
redirect_stdout: bool,
redirect_stderr: bool,
redirect_combine: bool,
trim_end_newline: bool,
) -> Result<ExternalCommand, ShellError> {
let name: Spanned<String> = call.req(engine_state, stack, 0)?;
@ -180,11 +220,9 @@ pub fn create_external_command(
name,
args: spanned_args,
arg_keep_raw,
redirect_stdout,
redirect_stderr,
redirect_combine,
out: stack.stdout().clone(),
err: stack.stderr().clone(),
env_vars: env_vars_str,
trim_end_newline,
})
}
@ -193,11 +231,9 @@ pub struct ExternalCommand {
pub name: Spanned<String>,
pub args: Vec<Spanned<String>>,
pub arg_keep_raw: Vec<bool>,
pub redirect_stdout: bool,
pub redirect_stderr: bool,
pub redirect_combine: bool,
pub out: IoStream,
pub err: IoStream,
pub env_vars: HashMap<String, String>,
pub trim_end_newline: bool,
}
impl ExternalCommand {
@ -364,6 +400,7 @@ impl ExternalCommand {
let mut engine_state = engine_state.clone();
if let Some(hook) = engine_state.config.hooks.command_not_found.clone()
{
let stack = &mut stack.start_capture();
if let Ok(PipelineData::Value(Value::String { val, .. }, ..)) =
eval_hook(
&mut engine_state,
@ -412,6 +449,7 @@ impl ExternalCommand {
thread::Builder::new()
.name("external stdin worker".to_string())
.spawn(move || {
let stack = &mut stack.start_capture();
// Attempt to render the input as a table before piping it to the external.
// This is important for pagers like `less`;
// they need to get Nu data rendered for display to users.
@ -421,7 +459,7 @@ impl ExternalCommand {
let input = crate::Table::run(
&crate::Table,
&engine_state,
&mut stack,
stack,
&Call::new(head),
input,
);
@ -447,63 +485,66 @@ impl ExternalCommand {
#[cfg(unix)]
let commandname = self.name.item.clone();
let redirect_stdout = self.redirect_stdout;
let redirect_stderr = self.redirect_stderr;
let redirect_combine = self.redirect_combine;
let span = self.name.span;
let output_ctrlc = ctrlc.clone();
let stderr_ctrlc = ctrlc.clone();
let (stdout_tx, stdout_rx) = mpsc::sync_channel(OUTPUT_BUFFERS_IN_FLIGHT);
let (exit_code_tx, exit_code_rx) = mpsc::channel();
let stdout = child.as_mut().stdout.take();
let stderr = child.as_mut().stderr.take();
let (stdout, stderr) = if let Some(combined) = reader {
(
Some(RawStream::new(
Box::new(ByteLines::new(combined)),
ctrlc.clone(),
head,
None,
)),
None,
)
} else {
let stdout = child.as_mut().stdout.take().map(|out| {
RawStream::new(Box::new(ByteLines::new(out)), ctrlc.clone(), head, None)
});
// If this external is not the last expression, then its output is piped to a channel
// and we create a ListStream that can be consumed
let stderr = child.as_mut().stderr.take().map(|err| {
RawStream::new(Box::new(ByteLines::new(err)), ctrlc.clone(), head, None)
});
// First create a thread to redirect the external's stdout and wait for an exit code.
if matches!(self.err, IoStream::Pipe) {
(stderr, stdout)
} else {
(stdout, stderr)
}
};
// Create a thread to wait for an exit code.
thread::Builder::new()
.name("stdout redirector + exit code waiter".to_string())
.name("exit code waiter".into())
.spawn(move || {
if redirect_stdout {
let stdout = stdout.ok_or_else(|| {
ShellError::ExternalCommand { label: "Error taking stdout from external".to_string(), help: "Redirects need access to stdout of an external command"
.to_string(), span }
})?;
match child.as_mut().wait() {
Err(err) => Err(ShellError::ExternalCommand {
label: "External command exited with error".into(),
help: err.to_string(),
span
}),
Ok(x) => {
#[cfg(unix)]
{
use nu_ansi_term::{Color, Style};
use std::ffi::CStr;
use std::os::unix::process::ExitStatusExt;
read_and_redirect_message(stdout, stdout_tx, ctrlc)
} else if redirect_combine {
let stdout = reader.ok_or_else(|| {
ShellError::ExternalCommand { label: "Error taking combined stdout and stderr from external".to_string(), help: "Combined redirects need access to reader pipe of an external command"
.to_string(), span }
})?;
read_and_redirect_message(stdout, stdout_tx, ctrlc)
}
if x.core_dumped() {
let cause = x.signal().and_then(|sig| unsafe {
// SAFETY: We should be the first to call `char * strsignal(int sig)`
let sigstr_ptr = libc::strsignal(sig);
if sigstr_ptr.is_null() {
return None;
}
match child.as_mut().wait() {
Err(err) => Err(ShellError::ExternalCommand { label: "External command exited with error".into(), help: err.to_string(), span }),
Ok(x) => {
#[cfg(unix)]
{
use nu_ansi_term::{Color, Style};
use std::ffi::CStr;
use std::os::unix::process::ExitStatusExt;
// SAFETY: The pointer points to a valid non-null string
let sigstr = CStr::from_ptr(sigstr_ptr);
sigstr.to_str().map(String::from).ok()
});
if x.core_dumped() {
let cause = x.signal().and_then(|sig| unsafe {
// SAFETY: We should be the first to call `char * strsignal(int sig)`
let sigstr_ptr = libc::strsignal(sig);
if sigstr_ptr.is_null() {
return None;
}
// SAFETY: The pointer points to a valid non-null string
let sigstr = CStr::from_ptr(sigstr_ptr);
sigstr.to_str().map(String::from).ok()
});
let cause = cause.as_deref().unwrap_or("Something went wrong");
let cause = cause.as_deref().unwrap_or("Something went wrong");
let style = Style::new().bold().on(Color::Red);
eprintln!(
@ -531,56 +572,18 @@ impl ExternalCommand {
}
}).map_err(|e| e.into_spanned(head))?;
let (stderr_tx, stderr_rx) = mpsc::sync_channel(OUTPUT_BUFFERS_IN_FLIGHT);
if redirect_stderr {
thread::Builder::new()
.name("stderr redirector".to_string())
.spawn(move || {
let stderr = stderr.ok_or_else(|| ShellError::ExternalCommand {
label: "Error taking stderr from external".to_string(),
help: "Redirects need access to stderr of an external command"
.to_string(),
span,
})?;
read_and_redirect_message(stderr, stderr_tx, stderr_ctrlc);
Ok::<(), ShellError>(())
})
.map_err(|e| e.into_spanned(head))?;
}
let stdout_receiver = ChannelReceiver::new(stdout_rx);
let stderr_receiver = ChannelReceiver::new(stderr_rx);
let exit_code_receiver = ValueReceiver::new(exit_code_rx);
Ok(PipelineData::ExternalStream {
stdout: if redirect_stdout || redirect_combine {
Some(RawStream::new(
Box::new(stdout_receiver),
output_ctrlc.clone(),
head,
None,
))
} else {
None
},
stderr: if redirect_stderr {
Some(RawStream::new(
Box::new(stderr_receiver),
output_ctrlc.clone(),
head,
None,
))
} else {
None
},
stdout,
stderr,
exit_code: Some(ListStream::from_stream(
Box::new(exit_code_receiver),
output_ctrlc,
ctrlc.clone(),
)),
span: head,
metadata: None,
trim_end_newline: self.trim_end_newline,
trim_end_newline: true,
})
}
}
@ -621,20 +624,15 @@ impl ExternalCommand {
// If the external is not the last command, its output will get piped
// either as a string or binary
let reader = if self.redirect_combine {
let reader = if matches!(self.out, IoStream::Pipe) && matches!(self.err, IoStream::Pipe) {
let (reader, writer) = os_pipe::pipe()?;
let writer_clone = writer.try_clone()?;
process.stdout(writer);
process.stderr(writer_clone);
Some(reader)
} else {
if self.redirect_stdout {
process.stdout(Stdio::piped());
}
if self.redirect_stderr {
process.stderr(Stdio::piped());
}
process.stdout(Stdio::try_from(&self.out)?);
process.stderr(Stdio::try_from(&self.err)?);
None
};
@ -824,63 +822,27 @@ fn remove_quotes(input: String) -> String {
}
}
// read message from given `reader`, and send out through `sender`.
//
// `ctrlc` is used to control the process, if ctrl-c is pressed, the read and redirect
// process will be breaked.
fn read_and_redirect_message<R>(
reader: R,
sender: SyncSender<Vec<u8>>,
ctrlc: Option<Arc<AtomicBool>>,
) where
R: Read,
{
// read using the BufferReader. It will do so until there is an
// error or there are no more bytes to read
let mut buf_read = BufReader::with_capacity(OUTPUT_BUFFER_SIZE, reader);
while let Ok(bytes) = buf_read.fill_buf() {
if bytes.is_empty() {
break;
}
struct ByteLines<R: Read>(BufReader<R>);
// The Cow generated from the function represents the conversion
// from bytes to String. If no replacements are required, then the
// borrowed value is a proper UTF-8 string. The Owned option represents
// a string where the values had to be replaced, thus marking it as bytes
let bytes = bytes.to_vec();
let length = bytes.len();
buf_read.consume(length);
if nu_utils::ctrl_c::was_pressed(&ctrlc) {
break;
}
match sender.send(bytes) {
Ok(_) => continue,
Err(_) => break,
}
impl<R: Read> ByteLines<R> {
fn new(read: R) -> Self {
Self(BufReader::new(read))
}
}
// Receiver used for the RawStream
// It implements iterator so it can be used as a RawStream
struct ChannelReceiver {
rx: mpsc::Receiver<Vec<u8>>,
}
impl ChannelReceiver {
pub fn new(rx: mpsc::Receiver<Vec<u8>>) -> Self {
Self { rx }
}
}
impl Iterator for ChannelReceiver {
impl<R: Read> Iterator for ByteLines<R> {
type Item = Result<Vec<u8>, ShellError>;
fn next(&mut self) -> Option<Self::Item> {
match self.rx.recv() {
Ok(v) => Some(Ok(v)),
Err(_) => None,
let mut buf = Vec::new();
// `read_until` will never stop reading unless `\n` or EOF is encountered,
// so let's limit the number of bytes using `take` as the Rust docs suggest.
let capacity = self.0.capacity() as u64;
let mut reader = (&mut self.0).take(capacity);
match reader.read_until(b'\n', &mut buf) {
Ok(0) => None,
Ok(_) => Some(Ok(buf)),
Err(e) => Some(Err(e.into())),
}
}
}

View File

@ -9,10 +9,10 @@ use nu_engine::{env::get_config, env_to_string, CallExt};
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
Category, Config, DataSource, Example, IntoPipelineData, ListStream, PipelineData,
PipelineMetadata, RawStream, Record, ShellError, Signature, Span, SyntaxShape, Type, Value,
record, Category, Config, DataSource, Example, IntoPipelineData, IoStream, ListStream,
PipelineData, PipelineMetadata, RawStream, Record, ShellError, Signature, Span, SyntaxShape,
TableMode, Type, Value,
};
use nu_protocol::{record, TableMode};
use nu_table::common::create_nu_table_config;
use nu_table::{
CollapsedTable, ExpandedTable, JustTable, NuTable, NuTableCell, StringResult, TableOpts,
@ -370,7 +370,10 @@ fn handle_table_command(
match input.data {
PipelineData::ExternalStream { .. } => Ok(input.data),
PipelineData::Value(Value::Binary { val, .. }, ..) => {
let stream_list = if input.call.redirect_stdout {
let stream_list = if matches!(
input.stack.stdout(),
IoStream::Pipe | IoStream::Capture | IoStream::Null
) {
vec![Ok(val)]
} else {
let hex = format!("{}\n", nu_pretty_hex::pretty_hex(&val))

View File

@ -23,7 +23,72 @@ fn basic_exit_code() {
#[test]
fn error() {
let actual = nu!("do { not-found } | complete");
let actual = nu!("not-found | complete");
assert!(actual.err.contains("executable was not found"));
}
#[test]
#[cfg(not(windows))]
fn capture_error_with_too_much_stderr_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::playground::Playground;
Playground::setup("external with many stderr message", |dirs, sandbox| {
let bytes: usize = 81920;
let mut large_file_body = String::with_capacity(bytes);
for _ in 0..bytes {
large_file_body.push('a');
}
sandbox.with_files(vec![FileWithContent("a_large_file.txt", &large_file_body)]);
let actual =
nu!(cwd: dirs.test(), "sh -c 'cat a_large_file.txt 1>&2' | complete | get stderr");
assert_eq!(actual.out, large_file_body);
})
}
#[test]
#[cfg(not(windows))]
fn capture_error_with_too_much_stdout_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::playground::Playground;
Playground::setup("external with many stdout message", |dirs, sandbox| {
let bytes: usize = 81920;
let mut large_file_body = String::with_capacity(bytes);
for _ in 0..bytes {
large_file_body.push('a');
}
sandbox.with_files(vec![FileWithContent("a_large_file.txt", &large_file_body)]);
let actual = nu!(cwd: dirs.test(), "sh -c 'cat a_large_file.txt' | complete | get stdout");
assert_eq!(actual.out, large_file_body);
})
}
#[test]
#[cfg(not(windows))]
fn capture_error_with_both_stdout_stderr_messages_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::playground::Playground;
Playground::setup(
"external with many stdout and stderr messages",
|dirs, sandbox| {
let script_body = r#"
x=$(printf '=%.0s' $(seq 40960))
echo $x
echo $x 1>&2
"#;
let expect_body = "=".repeat(40960);
sandbox.with_files(vec![FileWithContent("test.sh", script_body)]);
// check for stdout
let actual = nu!(cwd: dirs.test(), "sh test.sh | complete | get stdout | str trim");
assert_eq!(actual.out, expect_body);
// check for stderr
let actual = nu!(cwd: dirs.test(), "sh test.sh | complete | get stderr | str trim");
assert_eq!(actual.out, expect_body);
},
)
}

View File

@ -1,6 +1,4 @@
use nu_test_support::nu;
#[cfg(not(windows))]
use nu_test_support::pipeline;
#[test]
fn capture_errors_works() {
@ -63,89 +61,6 @@ fn ignore_error_should_work_for_external_command() {
assert_eq!(actual.out, "post");
}
#[test]
#[cfg(not(windows))]
fn capture_error_with_too_much_stderr_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::pipeline;
use nu_test_support::playground::Playground;
Playground::setup("external with many stderr message", |dirs, sandbox| {
let bytes: usize = 81920;
let mut large_file_body = String::with_capacity(bytes);
for _ in 0..bytes {
large_file_body.push('a');
}
sandbox.with_files(vec![FileWithContent("a_large_file.txt", &large_file_body)]);
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
do -c {sh -c "cat a_large_file.txt 1>&2"} | complete | get stderr
"#,
));
assert_eq!(actual.out, large_file_body);
})
}
#[test]
#[cfg(not(windows))]
fn capture_error_with_too_much_stdout_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::pipeline;
use nu_test_support::playground::Playground;
Playground::setup("external with many stdout message", |dirs, sandbox| {
let bytes: usize = 81920;
let mut large_file_body = String::with_capacity(bytes);
for _ in 0..bytes {
large_file_body.push('a');
}
sandbox.with_files(vec![FileWithContent("a_large_file.txt", &large_file_body)]);
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
do -c {sh -c "cat a_large_file.txt"} | complete | get stdout
"#,
));
assert_eq!(actual.out, large_file_body);
})
}
#[test]
#[cfg(not(windows))]
fn capture_error_with_both_stdout_stderr_messages_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::playground::Playground;
Playground::setup(
"external with many stdout and stderr messages",
|dirs, sandbox| {
let script_body = r#"
x=$(printf '=%.0s' $(seq 40960))
echo $x
echo $x 1>&2
"#;
let expect_body = "=".repeat(40960);
sandbox.with_files(vec![FileWithContent("test.sh", script_body)]);
// check for stdout
let actual = nu!(
cwd: dirs.test(), pipeline(
"do -c {sh test.sh} | complete | get stdout | str trim",
));
assert_eq!(actual.out, expect_body);
// check for stderr
let actual = nu!(
cwd: dirs.test(), pipeline(
"do -c {sh test.sh} | complete | get stderr | str trim",
));
assert_eq!(actual.out, expect_body);
},
)
}
#[test]
fn ignore_error_works_with_list_stream() {
let actual = nu!(r#"do -i { ["a", null, "b"] | ansi strip }"#);

View File

@ -65,9 +65,7 @@ fn let_err_pipeline_redirects_externals() {
let actual = nu!(
r#"let x = with-env [FOO "foo"] {nu --testbin echo_env_stderr FOO e>| str length}; $x"#
);
// have an extra \n, so length is 4.
assert_eq!(actual.out, "4");
assert_eq!(actual.out, "3");
}
#[test]
@ -75,9 +73,7 @@ fn let_outerr_pipeline_redirects_externals() {
let actual = nu!(
r#"let x = with-env [FOO "foo"] {nu --testbin echo_env_stderr FOO o+e>| str length}; $x"#
);
// have an extra \n, so length is 4.
assert_eq!(actual.out, "4");
assert_eq!(actual.out, "3");
}
#[ignore]

View File

@ -164,7 +164,7 @@ fn redirection_keep_exit_codes() {
Playground::setup("redirection preserves exit code", |dirs, _| {
let out = nu!(
cwd: dirs.test(),
"do -i { nu --testbin fail e> a.txt } | complete | get exit_code"
"nu --testbin fail e> a.txt | complete | get exit_code"
);
// needs to use contains "1", because it complete will output `Some(RawStream)`.
assert!(out.out.contains('1'));
@ -358,7 +358,7 @@ fn redirection_with_out_pipe() {
r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ err> tmp_file | str length"#,
);
assert_eq!(actual.out, "8");
assert_eq!(actual.out, "7");
// check for stderr redirection file.
let expected_out_file = dirs.test().join("tmp_file");
let actual_len = file_contents(expected_out_file).len();
@ -376,7 +376,7 @@ fn redirection_with_err_pipe() {
r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ out> tmp_file e>| str length"#,
);
assert_eq!(actual.out, "8");
assert_eq!(actual.out, "7");
// check for stdout redirection file.
let expected_out_file = dirs.test().join("tmp_file");
let actual_len = file_contents(expected_out_file).len();
@ -392,7 +392,7 @@ fn no_redirection_with_outerr_pipe() {
cwd: dirs.test(),
&format!("echo 3 {redirect_type} a.txt o+e>| str length")
);
assert!(actual.err.contains("not allowed to use with redirection"));
assert!(actual.err.contains("Multiple redirections provided"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"
@ -404,7 +404,7 @@ fn no_redirection_with_outerr_pipe() {
cwd: dirs.test(),
"echo 3 o> a.txt e> b.txt o+e>| str length"
);
assert!(actual.err.contains("not allowed to use with redirection"));
assert!(actual.err.contains("Multiple redirections provided"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"
@ -423,7 +423,7 @@ fn no_duplicate_redirection() {
cwd: dirs.test(),
"echo 3 o> a.txt o> a.txt"
);
assert!(actual.err.contains("Redirection can be set only once"));
assert!(actual.err.contains("Multiple redirections provided"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"
@ -432,7 +432,7 @@ fn no_duplicate_redirection() {
cwd: dirs.test(),
"echo 3 e> a.txt e> a.txt"
);
assert!(actual.err.contains("Redirection can be set only once"));
assert!(actual.err.contains("Multiple redirections provided"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"

View File

@ -325,7 +325,7 @@ fn redirect_combine() {
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
run-external --redirect-combine sh ...[-c 'echo Foo; echo >&2 Bar']
run-external sh ...[-c 'echo Foo; echo >&2 Bar'] o+e>| print
"#
));