diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index c56f41234a..9d74c9aa07 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -127,6 +127,7 @@ 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 f5f3ab79ba..1f585acdad 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -232,6 +232,7 @@ 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 beb091cc96..8e20f92668 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -146,6 +146,7 @@ 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 79ae027349..085fe9f7f1 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -789,16 +789,14 @@ pub fn eval_element_with_input( redirect_stderr: bool, ) -> Result<(PipelineData, bool), ShellError> { match element { - PipelineElement::Expression(_, expr) | PipelineElement::Or(_, expr) => { - eval_expression_with_input( - engine_state, - stack, - expr, - input, - redirect_stdout, - redirect_stderr, - ) - } + PipelineElement::Expression(_, 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) { @@ -905,6 +903,22 @@ 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, + ), } } @@ -938,16 +952,10 @@ pub fn eval_block( redirect_stderr: bool, ) -> Result { let num_pipelines = block.len(); - let mut pipeline_idx = 0; - - while pipeline_idx < block.pipelines.len() { - let pipeline = &block.pipelines[pipeline_idx]; - + for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { let mut i = 0; - let mut last_element_is_pipeline_or = false; - - while i < pipeline.elements.len() && !last_element_is_pipeline_or { + while i < pipeline.elements.len() { let redirect_stderr = redirect_stderr || ((i < pipeline.elements.len() - 1) && (matches!( @@ -956,8 +964,6 @@ 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, @@ -984,16 +990,10 @@ 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 have failed + // external command may runs to 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,109 +1006,83 @@ pub fn eval_block( } if pipeline_idx < (num_pipelines) - 1 { - if last_element_is_pipeline_or { - let input_is_error = matches!(input, PipelineData::Value(Value::Error { .. }, ..)); + match input { + PipelineData::Value(Value::Nothing { .. }, ..) => {} + PipelineData::ExternalStream { + ref mut exit_code, .. + } => { + let exit_code = exit_code.take(); - let result = - drain_and_print(engine_state, stack, input, redirect_stdout, redirect_stderr); + // Drain the input to the screen via tabular output + let config = engine_state.get_config(); - let last_exit_code = stack.last_exit_code(engine_state).unwrap_or(0); + 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, + )?; - 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; + 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); + } + } } - input = PipelineData::empty() - } else { - drain_and_print(engine_state, stack, input, redirect_stdout, redirect_stderr)?; + _ => { + // Drain the input to the screen via tabular output + let config = engine_state.get_config(); - input = PipelineData::empty() + 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)?; + } + }; + } } - } - pipeline_idx += 1; + input = PipelineData::empty() + } } 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 12a48f2a8a..14464ed423 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -455,10 +455,14 @@ pub fn flatten_pipeline_element( output.append(&mut flatten_expression(working_set, expr)); output } - PipelineElement::Or(span, expr) => { - let mut output = vec![]; + 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)]; 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 10f8571818..8dce2f90aa 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -7,7 +7,6 @@ pub enum TokenContents { Comment, Pipe, PipePipe, - AndAnd, Semicolon, OutGreaterThan, ErrGreaterThan, @@ -255,10 +254,10 @@ pub fn lex_item( ), b"&&" => ( Token { - contents: TokenContents::AndAnd, + contents: TokenContents::Item, span, }, - None, + Some(ParseError::ShellAndAnd(span)), ), b"2>" => ( Token { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 09b3713a26..2241b13c63 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1420,9 +1420,7 @@ pub fn parse_module_block( pipeline } - LiteElement::Redirection(_, _, command) | LiteElement::Or(_, command) => { - garbage_pipeline(&command.parts) - } + LiteElement::Redirection(_, _, 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 4800624015..9b86b531c1 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1223,7 +1223,6 @@ fn parse_binary_with_base( } TokenContents::Pipe | TokenContents::PipePipe - | TokenContents::AndAnd | TokenContents::OutGreaterThan | TokenContents::ErrGreaterThan | TokenContents::OutErrGreaterThan => { @@ -3805,9 +3804,7 @@ pub fn parse_table_expression( } _ => { match &output.block[0].commands[0] { - LiteElement::Command(_, command) - | LiteElement::Redirection(_, _, command) - | LiteElement::Or(_, command) => { + LiteElement::Command(_, command) | LiteElement::Redirection(_, _, command) => { let mut table_headers = vec![]; let (headers, err) = parse_value( @@ -3828,8 +3825,7 @@ pub fn parse_table_expression( match &output.block[1].commands[0] { LiteElement::Command(_, command) - | LiteElement::Redirection(_, _, command) - | LiteElement::Or(_, command) => { + | LiteElement::Redirection(_, _, command) => { let mut rows = vec![]; for part in &command.parts { let (values, err) = parse_value( @@ -5286,9 +5282,7 @@ 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::Or(_, command) => { + LiteElement::Command(_, command) | LiteElement::Redirection(_, _, command) => { if let Some(err) = parse_def_predecl(working_set, &command.parts, expand_aliases_denylist) { @@ -5325,21 +5319,6 @@ 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( @@ -5375,16 +5354,8 @@ pub fn parse_block( Pipeline { elements: output } } else { - let (mut pipeline, err) = match &pipeline.commands[0] { + 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, @@ -5392,63 +5363,61 @@ pub fn parse_block( is_subexpression, ); - 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 + 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 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) + for element in pipeline.elements.iter_mut() { + if let PipelineElement::Expression( + _, + Expression { + expr: Expr::Call(call), + .. + }, + ) = element { - if expr.has_in_variable(working_set) { - *expr = Box::new(wrap_expr_with_collect( - working_set, - expr, - )); + 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); } + } 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(); @@ -5525,6 +5494,7 @@ 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) } @@ -5785,6 +5755,9 @@ 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)) } @@ -5888,7 +5861,6 @@ impl LiteCommand { pub enum LiteElement { Command(Option, LiteCommand), Redirection(Span, Redirection, LiteCommand), - Or(Span, LiteCommand), } #[derive(Debug)] @@ -5957,8 +5929,15 @@ 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() { @@ -6094,25 +6073,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { last_token = TokenContents::Eol; } - 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 => { + TokenContents::Semicolon => { if !curr_command.is_empty() { match last_connector { TokenContents::OutGreaterThan => { @@ -6141,7 +6102,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { } _ => { curr_pipeline - .push(LiteElement::Command(Some(token.span), curr_command)); + .push(LiteElement::Command(last_connector_span, curr_command)); } } @@ -6213,10 +6174,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { block.push(curr_pipeline); } - if last_token == TokenContents::Pipe - || last_token == TokenContents::PipePipe - || last_token == TokenContents::AndAnd - { + if last_token == TokenContents::Pipe { ( block, Some(ParseError::UnexpectedEof( @@ -6225,7 +6183,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { )), ) } else { - (block, None) + (block, error) } } diff --git a/crates/nu-protocol/src/ast/pipeline.rs b/crates/nu-protocol/src/ast/pipeline.rs index aa13fe70f7..a7df63bc1b 100644 --- a/crates/nu-protocol/src/ast/pipeline.rs +++ b/crates/nu-protocol/src/ast/pipeline.rs @@ -14,6 +14,7 @@ pub enum Redirection { pub enum PipelineElement { Expression(Option, Expression), Redirection(Span, Redirection, Expression), + And(Span, Expression), Or(Span, Expression), } @@ -22,8 +23,9 @@ impl PipelineElement { match self { PipelineElement::Expression(None, expression) => expression.span, PipelineElement::Expression(Some(span), expression) - | PipelineElement::Or(span, expression) - | PipelineElement::Redirection(span, _, expression) => Span { + | PipelineElement::Redirection(span, _, expression) + | PipelineElement::And(span, expression) + | PipelineElement::Or(span, expression) => Span { start: span.start, end: expression.span.end, }, @@ -33,6 +35,7 @@ impl PipelineElement { match self { PipelineElement::Expression(_, expression) | PipelineElement::Redirection(_, _, expression) + | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) => expression.has_in_variable(working_set), } } @@ -40,8 +43,9 @@ impl PipelineElement { pub fn replace_in_variable(&mut self, working_set: &mut StateWorkingSet, new_var_id: VarId) { match self { PipelineElement::Expression(_, expression) - | PipelineElement::Or(_, expression) - | PipelineElement::Redirection(_, _, expression) => { + | PipelineElement::Redirection(_, _, expression) + | PipelineElement::And(_, expression) + | PipelineElement::Or(_, expression) => { expression.replace_in_variable(working_set, new_var_id) } } @@ -55,8 +59,9 @@ impl PipelineElement { ) { match self { PipelineElement::Expression(_, expression) - | PipelineElement::Or(_, expression) - | PipelineElement::Redirection(_, _, expression) => { + | PipelineElement::Redirection(_, _, expression) + | PipelineElement::And(_, expression) + | PipelineElement::Or(_, 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 353dfc1408..dfec8989f4 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -113,13 +113,6 @@ 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 e6fae4054c..e8fb3af058 100644 --- a/tests/shell/pipeline/mod.rs +++ b/tests/shell/pipeline/mod.rs @@ -1,5 +1,4 @@ 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 deleted file mode 100644 index a5a107dd82..0000000000 --- a/tests/shell/pipeline/pipeline_operators.rs +++ /dev/null @@ -1,74 +0,0 @@ -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")); -}