diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index 9d74c9aa07..c56f41234a 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -127,7 +127,6 @@ impl NuCompleter { match pipeline_element { PipelineElement::Expression(_, expr) | PipelineElement::Redirection(_, _, expr) - | PipelineElement::And(_, expr) | PipelineElement::Or(_, expr) => { let flattened: Vec<_> = flatten_expression(&working_set, &expr); let span_offset: usize = alias_offset.iter().sum(); diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index 1f585acdad..f5f3ab79ba 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -232,7 +232,6 @@ fn find_matching_block_end_in_block( match e { PipelineElement::Expression(_, e) | PipelineElement::Redirection(_, _, e) - | PipelineElement::And(_, e) | PipelineElement::Or(_, e) => { if e.span.contains(global_cursor_offset) { if let Some(pos) = find_matching_block_end_in_expr( diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 8e20f92668..beb091cc96 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -146,7 +146,6 @@ impl Command for FromNuon { match pipeline.elements.remove(0) { PipelineElement::Expression(_, expression) | PipelineElement::Redirection(_, _, expression) - | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) => expression, } } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 085fe9f7f1..79ae027349 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -789,14 +789,16 @@ pub fn eval_element_with_input( redirect_stderr: bool, ) -> Result<(PipelineData, bool), ShellError> { match element { - PipelineElement::Expression(_, expr) => eval_expression_with_input( - engine_state, - stack, - expr, - input, - redirect_stdout, - redirect_stderr, - ), + PipelineElement::Expression(_, expr) | PipelineElement::Or(_, expr) => { + eval_expression_with_input( + engine_state, + stack, + expr, + input, + redirect_stdout, + redirect_stderr, + ) + } PipelineElement::Redirection(span, redirection, expr) => match &expr.expr { Expr::String(_) => { let input = match (redirection, input) { @@ -903,22 +905,6 @@ pub fn eval_element_with_input( } _ => Err(ShellError::CommandNotFound(*span)), }, - PipelineElement::And(_, expr) => eval_expression_with_input( - engine_state, - stack, - expr, - input, - redirect_stdout, - redirect_stderr, - ), - PipelineElement::Or(_, expr) => eval_expression_with_input( - engine_state, - stack, - expr, - input, - redirect_stdout, - redirect_stderr, - ), } } @@ -952,10 +938,16 @@ pub fn eval_block( redirect_stderr: bool, ) -> Result { let num_pipelines = block.len(); - for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { + let mut pipeline_idx = 0; + + while pipeline_idx < block.pipelines.len() { + let pipeline = &block.pipelines[pipeline_idx]; + let mut i = 0; - while i < pipeline.elements.len() { + let mut last_element_is_pipeline_or = false; + + while i < pipeline.elements.len() && !last_element_is_pipeline_or { let redirect_stderr = redirect_stderr || ((i < pipeline.elements.len() - 1) && (matches!( @@ -964,6 +956,8 @@ pub fn eval_block( | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _) ))); + last_element_is_pipeline_or = matches!(&pipeline.elements[i], PipelineElement::Or(..)); + // if eval internal command failed, it can just make early return with `Err(ShellError)`. let eval_result = eval_element_with_input( engine_state, @@ -990,10 +984,16 @@ pub fn eval_block( // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. } (Err(error), true) => input = PipelineData::Value(Value::Error { error }, None), + (output, false) if last_element_is_pipeline_or => match output { + Ok(output) => { + input = output.0; + } + Err(error) => input = PipelineData::Value(Value::Error { error }, None), + }, (output, false) => { let output = output?; input = output.0; - // external command may runs to failed + // external command may have failed // make early return so remaining commands will not be executed. // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. if output.1 { @@ -1006,83 +1006,109 @@ pub fn eval_block( } if pipeline_idx < (num_pipelines) - 1 { - match input { - PipelineData::Value(Value::Nothing { .. }, ..) => {} - PipelineData::ExternalStream { - ref mut exit_code, .. - } => { - let exit_code = exit_code.take(); + if last_element_is_pipeline_or { + let input_is_error = matches!(input, PipelineData::Value(Value::Error { .. }, ..)); - // Drain the input to the screen via tabular output - let config = engine_state.get_config(); + let result = + drain_and_print(engine_state, stack, input, redirect_stdout, redirect_stderr); - match engine_state.find_decl("table".as_bytes(), &[]) { - Some(decl_id) => { - let table = engine_state.get_decl(decl_id).run( - engine_state, - stack, - &Call::new(Span::new(0, 0)), - input, - )?; + let last_exit_code = stack.last_exit_code(engine_state).unwrap_or(0); - print_or_return(table, config)?; - } - None => { - print_or_return(input, config)?; - } - }; - - if let Some(exit_code) = exit_code { - let mut v: Vec<_> = exit_code.collect(); - - if let Some(v) = v.pop() { - stack.add_env_var("LAST_EXIT_CODE".into(), v); - } - } + if last_exit_code == 0 && result.is_ok() && !input_is_error { + // Skip the next pipeline ot run because this pipeline was successful and the + // user used the `a || b` connector + pipeline_idx += 1; } - _ => { - // Drain the input to the screen via tabular output - let config = engine_state.get_config(); + input = PipelineData::empty() + } else { + drain_and_print(engine_state, stack, input, redirect_stdout, redirect_stderr)?; - match engine_state.find_decl("table".as_bytes(), &[]) { - Some(decl_id) => { - let table = engine_state.get_decl(decl_id); - - if let Some(block_id) = table.get_block_id() { - let block = engine_state.get_block(block_id); - eval_block( - engine_state, - stack, - block, - input, - redirect_stdout, - redirect_stderr, - )?; - } else { - let table = table.run( - engine_state, - stack, - &Call::new(Span::new(0, 0)), - input, - )?; - - print_or_return(table, config)?; - } - } - None => { - print_or_return(input, config)?; - } - }; - } + input = PipelineData::empty() } - - input = PipelineData::empty() } + + pipeline_idx += 1; } Ok(input) } +fn drain_and_print( + engine_state: &EngineState, + stack: &mut Stack, + mut input: PipelineData, + redirect_stdout: bool, + redirect_stderr: bool, +) -> Result<(), ShellError> { + match input { + PipelineData::Value(Value::Nothing { .. }, ..) => {} + PipelineData::ExternalStream { + ref mut exit_code, .. + } => { + let exit_code = exit_code.take(); + + // Drain the input to the screen via tabular output + let config = engine_state.get_config(); + + match engine_state.find_decl("table".as_bytes(), &[]) { + Some(decl_id) => { + let table = engine_state.get_decl(decl_id).run( + engine_state, + stack, + &Call::new(Span::new(0, 0)), + input, + )?; + + print_or_return(table, config)?; + } + None => { + print_or_return(input, config)?; + } + }; + + if let Some(exit_code) = exit_code { + let mut v: Vec<_> = exit_code.collect(); + + if let Some(v) = v.pop() { + stack.add_env_var("LAST_EXIT_CODE".into(), v); + } + } + } + _ => { + // Drain the input to the screen via tabular output + let config = engine_state.get_config(); + + match engine_state.find_decl("table".as_bytes(), &[]) { + Some(decl_id) => { + let table = engine_state.get_decl(decl_id); + + if let Some(block_id) = table.get_block_id() { + let block = engine_state.get_block(block_id); + eval_block( + engine_state, + stack, + block, + input, + redirect_stdout, + redirect_stderr, + )?; + } else { + let table = + table.run(engine_state, stack, &Call::new(Span::new(0, 0)), input)?; + + print_or_return(table, config)?; + } + } + None => { + print_or_return(input, config)?; + } + }; + } + } + + Ok(()) +} + fn print_or_return(pipeline_data: PipelineData, config: &Config) -> Result<(), ShellError> { for item in pipeline_data { if let Value::Error { error } = item { diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 14464ed423..12a48f2a8a 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -455,14 +455,10 @@ pub fn flatten_pipeline_element( output.append(&mut flatten_expression(working_set, expr)); output } - PipelineElement::And(span, expr) => { - let mut output = vec![(*span, FlatShape::And)]; - output.append(&mut flatten_expression(working_set, expr)); - output - } PipelineElement::Or(span, expr) => { - let mut output = vec![(*span, FlatShape::Or)]; + let mut output = vec![]; output.append(&mut flatten_expression(working_set, expr)); + output.push((*span, FlatShape::Or)); output } } diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index 8dce2f90aa..10f8571818 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -7,6 +7,7 @@ pub enum TokenContents { Comment, Pipe, PipePipe, + AndAnd, Semicolon, OutGreaterThan, ErrGreaterThan, @@ -254,10 +255,10 @@ pub fn lex_item( ), b"&&" => ( Token { - contents: TokenContents::Item, + contents: TokenContents::AndAnd, span, }, - Some(ParseError::ShellAndAnd(span)), + None, ), b"2>" => ( Token { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 2241b13c63..09b3713a26 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1420,7 +1420,9 @@ pub fn parse_module_block( pipeline } - LiteElement::Redirection(_, _, command) => garbage_pipeline(&command.parts), + LiteElement::Redirection(_, _, command) | LiteElement::Or(_, command) => { + garbage_pipeline(&command.parts) + } } } else { error = Some(ParseError::Expected("not a pipeline".into(), span)); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 9b86b531c1..4800624015 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1223,6 +1223,7 @@ fn parse_binary_with_base( } TokenContents::Pipe | TokenContents::PipePipe + | TokenContents::AndAnd | TokenContents::OutGreaterThan | TokenContents::ErrGreaterThan | TokenContents::OutErrGreaterThan => { @@ -3804,7 +3805,9 @@ pub fn parse_table_expression( } _ => { match &output.block[0].commands[0] { - LiteElement::Command(_, command) | LiteElement::Redirection(_, _, command) => { + LiteElement::Command(_, command) + | LiteElement::Redirection(_, _, command) + | LiteElement::Or(_, command) => { let mut table_headers = vec![]; let (headers, err) = parse_value( @@ -3825,7 +3828,8 @@ pub fn parse_table_expression( match &output.block[1].commands[0] { LiteElement::Command(_, command) - | LiteElement::Redirection(_, _, command) => { + | LiteElement::Redirection(_, _, command) + | LiteElement::Or(_, command) => { let mut rows = vec![]; for part in &command.parts { let (values, err) = parse_value( @@ -5282,7 +5286,9 @@ pub fn parse_block( for pipeline in &lite_block.block { if pipeline.commands.len() == 1 { match &pipeline.commands[0] { - LiteElement::Command(_, command) | LiteElement::Redirection(_, _, command) => { + LiteElement::Command(_, command) + | LiteElement::Redirection(_, _, command) + | LiteElement::Or(_, command) => { if let Some(err) = parse_def_predecl(working_set, &command.parts, expand_aliases_denylist) { @@ -5319,6 +5325,21 @@ pub fn parse_block( PipelineElement::Expression(*span, expr) } + LiteElement::Or(span, command) => { + let (expr, err) = parse_expression( + working_set, + &command.parts, + expand_aliases_denylist, + is_subexpression, + ); + working_set.type_scope.add_type(expr.ty.clone()); + + if error.is_none() { + error = err; + } + + PipelineElement::Or(*span, expr) + } LiteElement::Redirection(span, redirection, command) => { trace!("parsing: pipeline element: redirection"); let (expr, err) = parse_string( @@ -5354,8 +5375,16 @@ pub fn parse_block( Pipeline { elements: output } } else { - match &pipeline.commands[0] { + let (mut pipeline, err) = match &pipeline.commands[0] { LiteElement::Command(_, command) | LiteElement::Redirection(_, _, command) => { + parse_builtin_commands( + working_set, + command, + expand_aliases_denylist, + is_subexpression, + ) + } + LiteElement::Or(span, command) => { let (mut pipeline, err) = parse_builtin_commands( working_set, command, @@ -5363,61 +5392,63 @@ pub fn parse_block( is_subexpression, ); - if idx == 0 { - if let Some(let_decl_id) = working_set.find_decl(b"let", &Type::Any) { - if let Some(let_env_decl_id) = - working_set.find_decl(b"let-env", &Type::Any) + if let PipelineElement::Expression(_, expr) = &pipeline.elements[0] { + pipeline.elements[0] = PipelineElement::Or(*span, expr.clone()) + } + + (pipeline, err) + } + }; + + if idx == 0 { + if let Some(let_decl_id) = working_set.find_decl(b"let", &Type::Any) { + if let Some(let_env_decl_id) = working_set.find_decl(b"let-env", &Type::Any) + { + for element in pipeline.elements.iter_mut() { + if let PipelineElement::Expression( + _, + Expression { + expr: Expr::Call(call), + .. + }, + ) = element { - for element in pipeline.elements.iter_mut() { - if let PipelineElement::Expression( - _, - Expression { - expr: Expr::Call(call), - .. - }, - ) = element + if call.decl_id == let_decl_id + || call.decl_id == let_env_decl_id + { + // Do an expansion + if let Some(Expression { + expr: Expr::Keyword(_, _, expr), + .. + }) = call.positional_iter_mut().nth(1) { - if call.decl_id == let_decl_id - || call.decl_id == let_env_decl_id - { - // Do an expansion - if let Some(Expression { - expr: Expr::Keyword(_, _, expr), - .. - }) = call.positional_iter_mut().nth(1) - { - if expr.has_in_variable(working_set) { - *expr = Box::new(wrap_expr_with_collect( - working_set, - expr, - )); - } - } - continue; - } else if element.has_in_variable(working_set) - && !is_subexpression - { - *element = - wrap_element_with_collect(working_set, element); + if expr.has_in_variable(working_set) { + *expr = Box::new(wrap_expr_with_collect( + working_set, + expr, + )); } - } else if element.has_in_variable(working_set) - && !is_subexpression - { - *element = - wrap_element_with_collect(working_set, element); } + continue; + } else if element.has_in_variable(working_set) + && !is_subexpression + { + *element = wrap_element_with_collect(working_set, element); } + } else if element.has_in_variable(working_set) && !is_subexpression + { + *element = wrap_element_with_collect(working_set, element); } } } - - if error.is_none() { - error = err; - } - - pipeline } } + + if error.is_none() { + error = err; + } + + pipeline } }) .into(); @@ -5494,7 +5525,6 @@ pub fn discover_captures_in_pipeline_element( match element { PipelineElement::Expression(_, expression) | PipelineElement::Redirection(_, _, expression) - | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) => { discover_captures_in_expr(working_set, expression, seen, seen_blocks) } @@ -5755,9 +5785,6 @@ fn wrap_element_with_collect( wrap_expr_with_collect(working_set, expression), ) } - PipelineElement::And(span, expression) => { - PipelineElement::And(*span, wrap_expr_with_collect(working_set, expression)) - } PipelineElement::Or(span, expression) => { PipelineElement::Or(*span, wrap_expr_with_collect(working_set, expression)) } @@ -5861,6 +5888,7 @@ impl LiteCommand { pub enum LiteElement { Command(Option, LiteCommand), Redirection(Span, Redirection, LiteCommand), + Or(Span, LiteCommand), } #[derive(Debug)] @@ -5929,15 +5957,8 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { let mut curr_comment: Option> = None; - let mut error = None; - for token in tokens.iter() { match &token.contents { - TokenContents::PipePipe => { - error = error.or(Some(ParseError::ShellOrOr(token.span))); - curr_command.push(token.span); - last_token = TokenContents::Item; - } TokenContents::Item => { // If we have a comment, go ahead and attach it if let Some(curr_comment) = curr_comment.take() { @@ -6073,7 +6094,25 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { last_token = TokenContents::Eol; } - TokenContents::Semicolon => { + TokenContents::PipePipe => { + // Different to redirection, for PipePipe, we'll wrap the current command + // in an "Or". This lets us know during eval that it's the current command + // whose error will be ignored rather than having to look ahead in the pipeline + if !curr_command.is_empty() { + curr_pipeline.push(LiteElement::Or(token.span, curr_command)); + curr_command = LiteCommand::new(); + } + if !curr_pipeline.is_empty() { + block.push(curr_pipeline); + + curr_pipeline = LitePipeline::new(); + last_connector = TokenContents::Pipe; + last_connector_span = None; + } + + last_token = TokenContents::PipePipe; + } + TokenContents::Semicolon | TokenContents::AndAnd => { if !curr_command.is_empty() { match last_connector { TokenContents::OutGreaterThan => { @@ -6102,7 +6141,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { } _ => { curr_pipeline - .push(LiteElement::Command(last_connector_span, curr_command)); + .push(LiteElement::Command(Some(token.span), curr_command)); } } @@ -6174,7 +6213,10 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { block.push(curr_pipeline); } - if last_token == TokenContents::Pipe { + if last_token == TokenContents::Pipe + || last_token == TokenContents::PipePipe + || last_token == TokenContents::AndAnd + { ( block, Some(ParseError::UnexpectedEof( @@ -6183,7 +6225,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { )), ) } else { - (block, error) + (block, None) } } diff --git a/crates/nu-protocol/src/ast/pipeline.rs b/crates/nu-protocol/src/ast/pipeline.rs index a7df63bc1b..aa13fe70f7 100644 --- a/crates/nu-protocol/src/ast/pipeline.rs +++ b/crates/nu-protocol/src/ast/pipeline.rs @@ -14,7 +14,6 @@ pub enum Redirection { pub enum PipelineElement { Expression(Option, Expression), Redirection(Span, Redirection, Expression), - And(Span, Expression), Or(Span, Expression), } @@ -23,9 +22,8 @@ impl PipelineElement { match self { PipelineElement::Expression(None, expression) => expression.span, PipelineElement::Expression(Some(span), expression) - | PipelineElement::Redirection(span, _, expression) - | PipelineElement::And(span, expression) - | PipelineElement::Or(span, expression) => Span { + | PipelineElement::Or(span, expression) + | PipelineElement::Redirection(span, _, expression) => Span { start: span.start, end: expression.span.end, }, @@ -35,7 +33,6 @@ impl PipelineElement { match self { PipelineElement::Expression(_, expression) | PipelineElement::Redirection(_, _, expression) - | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) => expression.has_in_variable(working_set), } } @@ -43,9 +40,8 @@ impl PipelineElement { pub fn replace_in_variable(&mut self, working_set: &mut StateWorkingSet, new_var_id: VarId) { match self { PipelineElement::Expression(_, expression) - | PipelineElement::Redirection(_, _, expression) - | PipelineElement::And(_, expression) - | PipelineElement::Or(_, expression) => { + | PipelineElement::Or(_, expression) + | PipelineElement::Redirection(_, _, expression) => { expression.replace_in_variable(working_set, new_var_id) } } @@ -59,9 +55,8 @@ impl PipelineElement { ) { match self { PipelineElement::Expression(_, expression) - | PipelineElement::Redirection(_, _, expression) - | PipelineElement::And(_, expression) - | PipelineElement::Or(_, expression) => { + | PipelineElement::Or(_, expression) + | PipelineElement::Redirection(_, _, expression) => { expression.replace_span(working_set, replaced, new_span) } } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index dfec8989f4..353dfc1408 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -113,6 +113,13 @@ impl Stack { .ok_or_else(|| ShellError::NushellFailed("No active overlay".into())) } + pub fn last_exit_code(&self, engine_state: &EngineState) -> Option { + match self.get_env_var(engine_state, "LAST_EXIT_CODE") { + Some(Value::Int { val, .. }) => Some(val), + _ => None, + } + } + pub fn captures_to_stack(&self, captures: &HashMap) -> Stack { // FIXME: this is probably slow let mut env_vars = self.env_vars.clone(); diff --git a/tests/shell/pipeline/mod.rs b/tests/shell/pipeline/mod.rs index e8fb3af058..e6fae4054c 100644 --- a/tests/shell/pipeline/mod.rs +++ b/tests/shell/pipeline/mod.rs @@ -1,4 +1,5 @@ mod commands; +mod pipeline_operators; use nu_test_support::nu; diff --git a/tests/shell/pipeline/pipeline_operators.rs b/tests/shell/pipeline/pipeline_operators.rs new file mode 100644 index 0000000000..a5a107dd82 --- /dev/null +++ b/tests/shell/pipeline/pipeline_operators.rs @@ -0,0 +1,74 @@ +use nu_test_support::nu; + +#[test] +fn and_operator1() { + let actual = nu!( + cwd: ".", + r#" + 3 / 0 && 2 + 3 + "# + ); + + assert!(actual.err.contains("division by zero")); +} + +#[test] +fn and_operator2() { + let actual = nu!( + cwd: ".", + r#" + 0 | 3 / $in && 2 + 3 + "# + ); + + assert!(actual.err.contains("division by zero")); +} + +#[test] +fn or_operator1() { + let actual = nu!( + cwd: ".", + r#" + 3 / 0 || 2 + 3 + "# + ); + + assert!(actual.out.contains('5')); +} + +#[test] +fn or_operator2() { + let actual = nu!( + cwd: ".", + r#" + 0 | 3 / $in || 2 + 3 + "# + ); + + assert!(actual.out.contains('5')); +} + +#[test] +fn or_operator3() { + // On success, don't run the next step + let actual = nu!( + cwd: ".", + r#" + 0 || 2 + 3 + "# + ); + + assert_eq!(actual.out, "0"); +} + +#[test] +fn or_operator4() { + let actual = nu!( + cwd: ".", + r#" + 1 / 0 || 2 / 0 || 10 + 9 + "# + ); + + assert!(actual.out.contains("19")); +}