From 4d3283e23555610bd5443e1e57afdd5f748d25dc Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Sun, 24 Nov 2024 10:59:54 -0800 Subject: [PATCH] Change append operator to concatenation operator (#14344) # Description The "append" operator currently serves as both the append operator and the concatenation operator. This dual role creates ambiguity when operating on nested lists. ```nu [1 2] ++ 3 # appends a value to a list [1 2 3] [1 2] ++ [3 4] # concatenates two lists [1 2 3 4] [[1 2] [3 4]] ++ [5 6] # does this give [[1 2] [3 4] [5 6]] # or [[1 2] [3 4] 5 6] ``` Another problem is that `++=` can change the type of a variable: ```nu mut str = 'hello ' $str ++= ['world'] ($str | describe) == list ``` Note that appending is only relevant for lists, but concatenation is relevant for lists, strings, and binary values. Additionally, appending can be expressed in terms of concatenation (see example below). So, this PR changes the `++` operator to only perform concatenation. # User-Facing Changes Using the `++` operator with a list and a non-list value will now be a compile time or runtime error. ```nu mut list = [] $list ++= 1 # error ``` Instead, concatenate a list with one element: ```nu $list ++= [1] ``` Or use `append`: ```nu $list = $list | append 1 ``` # After Submitting Update book and docs. --------- Co-authored-by: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> --- .../src/completions/operator_completions.rs | 18 +--- crates/nu-command/src/help/help_operators.rs | 12 +-- .../commands/assignment/append_assign.rs | 88 ------------------- .../tests/commands/assignment/concat.rs | 76 ++++++++++++++++ .../tests/commands/assignment/mod.rs | 2 +- crates/nu-command/tests/commands/math/mod.rs | 30 +------ crates/nu-command/tests/commands/url/join.rs | 17 ++-- crates/nu-engine/src/compile/operator.rs | 2 +- crates/nu-engine/src/eval.rs | 4 +- crates/nu-engine/src/eval_ir.rs | 2 +- crates/nu-parser/src/parser.rs | 4 +- crates/nu-parser/src/type_check.rs | 22 ++--- .../nu-plugin-engine/src/interface/tests.rs | 4 +- crates/nu-protocol/src/ast/operator.rs | 10 +-- crates/nu-protocol/src/eval_base.rs | 2 +- crates/nu-protocol/src/value/mod.rs | 30 ++----- crates/nu-std/std/help/mod.nu | 4 +- .../src/cool_custom_value.rs | 2 +- tests/const_/mod.rs | 1 - tests/eval/mod.rs | 2 +- 20 files changed, 131 insertions(+), 201 deletions(-) delete mode 100644 crates/nu-command/tests/commands/assignment/append_assign.rs create mode 100644 crates/nu-command/tests/commands/assignment/concat.rs diff --git a/crates/nu-cli/src/completions/operator_completions.rs b/crates/nu-cli/src/completions/operator_completions.rs index 0ce4e02e17..aebb176b76 100644 --- a/crates/nu-cli/src/completions/operator_completions.rs +++ b/crates/nu-cli/src/completions/operator_completions.rs @@ -60,10 +60,6 @@ impl Completer for OperatorCompletion { ("bit-shr", "Bitwise shift right"), ("in", "Is a member of (doesn't use regex)"), ("not-in", "Is not a member of (doesn't use regex)"), - ( - "++", - "Appends two lists, a list and a value, two strings, or two binary values", - ), ], Expr::String(_) => vec![ ("=~", "Contains regex match"), @@ -72,7 +68,7 @@ impl Completer for OperatorCompletion { ("not-like", "Does not contain regex match"), ( "++", - "Appends two lists, a list and a value, two strings, or two binary values", + "Concatenates two lists, two strings, or two binary values", ), ("in", "Is a member of (doesn't use regex)"), ("not-in", "Is not a member of (doesn't use regex)"), @@ -95,10 +91,6 @@ impl Completer for OperatorCompletion { ("**", "Power of"), ("in", "Is a member of (doesn't use regex)"), ("not-in", "Is not a member of (doesn't use regex)"), - ( - "++", - "Appends two lists, a list and a value, two strings, or two binary values", - ), ], Expr::Bool(_) => vec![ ( @@ -113,15 +105,11 @@ impl Completer for OperatorCompletion { ("not", "Negates a value or expression"), ("in", "Is a member of (doesn't use regex)"), ("not-in", "Is not a member of (doesn't use regex)"), - ( - "++", - "Appends two lists, a list and a value, two strings, or two binary values", - ), ], Expr::FullCellPath(path) => match path.head.expr { Expr::List(_) => vec![( "++", - "Appends two lists, a list and a value, two strings, or two binary values", + "Concatenates two lists, two strings, or two binary values", )], Expr::Var(id) => get_variable_completions(id, working_set), _ => vec![], @@ -161,7 +149,7 @@ pub fn get_variable_completions<'a>( Type::List(_) | Type::String | Type::Binary => vec![ ( "++=", - "Appends a list, a value, a string, or a binary value to a variable.", + "Concatenates two lists, two strings, or two binary values", ), ("=", "Assigns a value to a variable."), ], diff --git a/crates/nu-command/src/help/help_operators.rs b/crates/nu-command/src/help/help_operators.rs index 28be6a68ed..28bb223e61 100644 --- a/crates/nu-command/src/help/help_operators.rs +++ b/crates/nu-command/src/help/help_operators.rs @@ -31,7 +31,7 @@ impl Command for HelpOperators { let mut operators = [ Operator::Assignment(Assignment::Assign), Operator::Assignment(Assignment::PlusAssign), - Operator::Assignment(Assignment::AppendAssign), + Operator::Assignment(Assignment::ConcatAssign), Operator::Assignment(Assignment::MinusAssign), Operator::Assignment(Assignment::MultiplyAssign), Operator::Assignment(Assignment::DivideAssign), @@ -48,7 +48,7 @@ impl Command for HelpOperators { Operator::Comparison(Comparison::StartsWith), Operator::Comparison(Comparison::EndsWith), Operator::Math(Math::Plus), - Operator::Math(Math::Append), + Operator::Math(Math::Concat), Operator::Math(Math::Minus), Operator::Math(Math::Multiply), Operator::Math(Math::Divide), @@ -144,8 +144,8 @@ fn description(operator: &Operator) -> &'static str { Operator::Comparison(Comparison::StartsWith) => "Checks if a string starts with another.", Operator::Comparison(Comparison::EndsWith) => "Checks if a string ends with another.", Operator::Math(Math::Plus) => "Adds two values.", - Operator::Math(Math::Append) => { - "Appends two lists, a list and a value, two strings, or two binary values." + Operator::Math(Math::Concat) => { + "Concatenates two lists, two strings, or two binary values." } Operator::Math(Math::Minus) => "Subtracts two values.", Operator::Math(Math::Multiply) => "Multiplies two values.", @@ -163,8 +163,8 @@ fn description(operator: &Operator) -> &'static str { Operator::Bits(Bits::ShiftRight) => "Bitwise shifts a value right by another.", Operator::Assignment(Assignment::Assign) => "Assigns a value to a variable.", Operator::Assignment(Assignment::PlusAssign) => "Adds a value to a variable.", - Operator::Assignment(Assignment::AppendAssign) => { - "Appends a list, a value, a string, or a binary value to a variable." + Operator::Assignment(Assignment::ConcatAssign) => { + "Concatenates two lists, two strings, or two binary values." } Operator::Assignment(Assignment::MinusAssign) => "Subtracts a value from a variable.", Operator::Assignment(Assignment::MultiplyAssign) => "Multiplies a variable by a value.", diff --git a/crates/nu-command/tests/commands/assignment/append_assign.rs b/crates/nu-command/tests/commands/assignment/append_assign.rs deleted file mode 100644 index 97abc8eb6f..0000000000 --- a/crates/nu-command/tests/commands/assignment/append_assign.rs +++ /dev/null @@ -1,88 +0,0 @@ -use nu_test_support::nu; - -#[test] -fn append_assign_int() { - let actual = nu!(r#" - mut a = [1 2]; - $a ++= [3 4]; - $a == [1 2 3 4] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_string() { - let actual = nu!(r#" - mut a = [a b]; - $a ++= [c d]; - $a == [a b c d] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_any() { - let actual = nu!(r#" - mut a = [1 2 a]; - $a ++= [b 3]; - $a == [1 2 a b 3] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_both_empty() { - let actual = nu!(r#" - mut a = []; - $a ++= []; - $a == [] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_type_mismatch() { - let actual = nu!(r#" - mut a = [1 2]; - $a ++= [a]; - $a == [1 2 "a"] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_single_element() { - let actual = nu!(r#" - mut a = ["list" "and"]; - $a ++= "a single element"; - $a == ["list" "and" "a single element"] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_to_single_element() { - let actual = nu!(r#" - mut a = "string"; - $a ++= ["and" "the" "list"]; - $a == ["string" "and" "the" "list"] - "#); - - assert_eq!(actual.out, "true") -} - -#[test] -fn append_assign_single_to_single() { - let actual = nu!(r#" - mut a = 1; - $a ++= "and a single element"; - "#); - - assert!(actual.err.contains("nu::parser::unsupported_operation")); -} diff --git a/crates/nu-command/tests/commands/assignment/concat.rs b/crates/nu-command/tests/commands/assignment/concat.rs new file mode 100644 index 0000000000..603a486f14 --- /dev/null +++ b/crates/nu-command/tests/commands/assignment/concat.rs @@ -0,0 +1,76 @@ +use nu_test_support::nu; + +#[test] +fn concat_assign_list_int() { + let actual = nu!(r#" + mut a = [1 2]; + $a ++= [3 4]; + $a == [1 2 3 4] + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn concat_assign_list_string() { + let actual = nu!(r#" + mut a = [a b]; + $a ++= [c d]; + $a == [a b c d] + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn concat_assign_any() { + let actual = nu!(r#" + mut a = [1 2 a]; + $a ++= [b 3]; + $a == [1 2 a b 3] + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn concat_assign_both_empty() { + let actual = nu!(r#" + mut a = []; + $a ++= []; + $a == [] + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn concat_assign_string() { + let actual = nu!(r#" + mut a = 'hello'; + $a ++= ' world'; + $a == 'hello world' + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn concat_assign_type_mismatch() { + let actual = nu!(r#" + mut a = []; + $a ++= 'str' + "#); + + assert!(actual.err.contains("nu::parser::unsupported_operation")); +} + +#[test] +fn concat_assign_runtime_type_mismatch() { + let actual = nu!(r#" + mut a = []; + $a ++= if true { 'str' } + "#); + + assert!(actual.err.contains("nu::shell::type_mismatch")); +} diff --git a/crates/nu-command/tests/commands/assignment/mod.rs b/crates/nu-command/tests/commands/assignment/mod.rs index 8235f532f5..0859deee1a 100644 --- a/crates/nu-command/tests/commands/assignment/mod.rs +++ b/crates/nu-command/tests/commands/assignment/mod.rs @@ -1 +1 @@ -mod append_assign; +mod concat; diff --git a/crates/nu-command/tests/commands/math/mod.rs b/crates/nu-command/tests/commands/math/mod.rs index cde6a2da35..5d04f450a4 100644 --- a/crates/nu-command/tests/commands/math/mod.rs +++ b/crates/nu-command/tests/commands/math/mod.rs @@ -483,7 +483,7 @@ fn compound_where_paren() { // TODO: these ++ tests are not really testing *math* functionality, maybe find another place for them #[test] -fn adding_lists() { +fn concat_lists() { let actual = nu!(pipeline( r#" [1 3] ++ [5 6] | to nuon @@ -494,29 +494,7 @@ fn adding_lists() { } #[test] -fn adding_list_and_value() { - let actual = nu!(pipeline( - r#" - [1 3] ++ 5 | to nuon - "# - )); - - assert_eq!(actual.out, "[1, 3, 5]"); -} - -#[test] -fn adding_value_and_list() { - let actual = nu!(pipeline( - r#" - 1 ++ [3 5] | to nuon - "# - )); - - assert_eq!(actual.out, "[1, 3, 5]"); -} - -#[test] -fn adding_tables() { +fn concat_tables() { let actual = nu!(pipeline( r#" [[a b]; [1 2]] ++ [[c d]; [10 11]] | to nuon @@ -526,7 +504,7 @@ fn adding_tables() { } #[test] -fn append_strings() { +fn concat_strings() { let actual = nu!(pipeline( r#" "foo" ++ "bar" @@ -536,7 +514,7 @@ fn append_strings() { } #[test] -fn append_binary_values() { +fn concat_binary_values() { let actual = nu!(pipeline( r#" 0x[01 02] ++ 0x[03 04] | to nuon diff --git a/crates/nu-command/tests/commands/url/join.rs b/crates/nu-command/tests/commands/url/join.rs index 92257a41ff..086d230cc4 100644 --- a/crates/nu-command/tests/commands/url/join.rs +++ b/crates/nu-command/tests/commands/url/join.rs @@ -27,7 +27,7 @@ fn url_join_with_only_user() { "password": "", "host": "localhost", "port": "", - } | url join + } | url join "# )); @@ -44,7 +44,7 @@ fn url_join_with_only_pwd() { "password": "pwd", "host": "localhost", "port": "", - } | url join + } | url join "# )); @@ -61,7 +61,7 @@ fn url_join_with_user_and_pwd() { "password": "pwd", "host": "localhost", "port": "", - } | url join + } | url join "# )); @@ -79,7 +79,7 @@ fn url_join_with_query() { "host": "localhost", "query": "par_1=aaa&par_2=bbb" "port": "", - } | url join + } | url join "# )); @@ -411,12 +411,9 @@ fn url_join_with_params_invalid_table() { "host": "localhost", "params": ( [ - ["key", "value"]; - ["par_1", "aaa"], - ["par_2", "bbb"], - ["par_1", "ccc"], - ["par_2", "ddd"], - ] ++ ["not a record"] + { key: foo, value: bar } + "not a record" + ] ), "port": "1234", } | url join diff --git a/crates/nu-engine/src/compile/operator.rs b/crates/nu-engine/src/compile/operator.rs index bad206358d..0f7059e027 100644 --- a/crates/nu-engine/src/compile/operator.rs +++ b/crates/nu-engine/src/compile/operator.rs @@ -155,7 +155,7 @@ pub(crate) fn decompose_assignment(assignment: Assignment) -> Option { match assignment { Assignment::Assign => None, Assignment::PlusAssign => Some(Operator::Math(Math::Plus)), - Assignment::AppendAssign => Some(Operator::Math(Math::Append)), + Assignment::ConcatAssign => Some(Operator::Math(Math::Concat)), Assignment::MinusAssign => Some(Operator::Math(Math::Minus)), Assignment::MultiplyAssign => Some(Operator::Math(Math::Multiply)), Assignment::DivideAssign => Some(Operator::Math(Math::Divide)), diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index fc4a88f83e..0e3917290f 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -547,9 +547,9 @@ impl Eval for EvalRuntime { let lhs = eval_expression::(engine_state, stack, lhs)?; lhs.div(op_span, &rhs, op_span)? } - Assignment::AppendAssign => { + Assignment::ConcatAssign => { let lhs = eval_expression::(engine_state, stack, lhs)?; - lhs.append(op_span, &rhs, op_span)? + lhs.concat(op_span, &rhs, op_span)? } }; diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 8d809f182e..1aec6b30ea 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -956,7 +956,7 @@ fn binary_op( }, Operator::Math(mat) => match mat { Math::Plus => lhs_val.add(op_span, &rhs_val, span)?, - Math::Append => lhs_val.append(op_span, &rhs_val, span)?, + Math::Concat => lhs_val.concat(op_span, &rhs_val, span)?, Math::Minus => lhs_val.sub(op_span, &rhs_val, span)?, Math::Multiply => lhs_val.mul(op_span, &rhs_val, span)?, Math::Divide => lhs_val.div(op_span, &rhs_val, span)?, diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 4048bec2a9..d9f534cc60 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -4887,7 +4887,7 @@ pub fn parse_assignment_operator(working_set: &mut StateWorkingSet, span: Span) let operator = match contents { b"=" => Operator::Assignment(Assignment::Assign), b"+=" => Operator::Assignment(Assignment::PlusAssign), - b"++=" => Operator::Assignment(Assignment::AppendAssign), + b"++=" => Operator::Assignment(Assignment::ConcatAssign), b"-=" => Operator::Assignment(Assignment::MinusAssign), b"*=" => Operator::Assignment(Assignment::MultiplyAssign), b"/=" => Operator::Assignment(Assignment::DivideAssign), @@ -5013,7 +5013,7 @@ pub fn parse_operator(working_set: &mut StateWorkingSet, span: Span) -> Expressi b"=~" | b"like" => Operator::Comparison(Comparison::RegexMatch), b"!~" | b"not-like" => Operator::Comparison(Comparison::NotRegexMatch), b"+" => Operator::Math(Math::Plus), - b"++" => Operator::Math(Math::Append), + b"++" => Operator::Math(Math::Concat), b"-" => Operator::Math(Math::Minus), b"*" => Operator::Math(Math::Multiply), b"/" => Operator::Math(Math::Divide), diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index a7de3800e2..e5f2a22921 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -132,7 +132,7 @@ pub fn math_result_type( ) } }, - Operator::Math(Math::Append) => check_append(working_set, lhs, rhs, op), + Operator::Math(Math::Concat) => check_concat(working_set, lhs, rhs, op), Operator::Math(Math::Minus) => match (&lhs.ty, &rhs.ty) { (Type::Int, Type::Int) => (Type::Int, None), (Type::Float, Type::Int) => (Type::Float, None), @@ -935,8 +935,8 @@ pub fn math_result_type( ) } }, - Operator::Assignment(Assignment::AppendAssign) => { - check_append(working_set, lhs, rhs, op) + Operator::Assignment(Assignment::ConcatAssign) => { + check_concat(working_set, lhs, rhs, op) } Operator::Assignment(_) => match (&lhs.ty, &rhs.ty) { (x, y) if x == y => (Type::Nothing, None), @@ -1085,7 +1085,7 @@ pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) -> output_errors } -fn check_append( +fn check_concat( working_set: &mut StateWorkingSet, lhs: &Expression, rhs: &Expression, @@ -1099,23 +1099,17 @@ fn check_append( (Type::List(Box::new(Type::Any)), None) } } - (Type::List(a), b) | (b, Type::List(a)) => { - if a == &Box::new(b.clone()) { - (Type::List(a.clone()), None) - } else { - (Type::List(Box::new(Type::Any)), None) - } - } (Type::Table(a), Type::Table(_)) => (Type::Table(a.clone()), None), (Type::String, Type::String) => (Type::String, None), (Type::Binary, Type::Binary) => (Type::Binary, None), (Type::Any, _) | (_, Type::Any) => (Type::Any, None), - (Type::Table(_) | Type::String | Type::Binary, _) => { + (Type::Table(_) | Type::List(_) | Type::String | Type::Binary, _) + | (_, Type::Table(_) | Type::List(_) | Type::String | Type::Binary) => { *op = Expression::garbage(working_set, op.span); ( Type::Any, Some(ParseError::UnsupportedOperationRHS( - "append".into(), + "concatenation".into(), op.span, lhs.span, lhs.ty.clone(), @@ -1129,7 +1123,7 @@ fn check_append( ( Type::Any, Some(ParseError::UnsupportedOperationLHS( - "append".into(), + "concatenation".into(), op.span, lhs.span, lhs.ty.clone(), diff --git a/crates/nu-plugin-engine/src/interface/tests.rs b/crates/nu-plugin-engine/src/interface/tests.rs index 5058a45695..4761da6824 100644 --- a/crates/nu-plugin-engine/src/interface/tests.rs +++ b/crates/nu-plugin-engine/src/interface/tests.rs @@ -1485,7 +1485,7 @@ fn prepare_plugin_call_custom_value_op() { span, }, CustomValueOp::Operation( - Operator::Math(Math::Append).into_spanned(span), + Operator::Math(Math::Concat).into_spanned(span), cv_ok_val.clone(), ), ), @@ -1498,7 +1498,7 @@ fn prepare_plugin_call_custom_value_op() { span, }, CustomValueOp::Operation( - Operator::Math(Math::Append).into_spanned(span), + Operator::Math(Math::Concat).into_spanned(span), cv_bad_val.clone(), ), ), diff --git a/crates/nu-protocol/src/ast/operator.rs b/crates/nu-protocol/src/ast/operator.rs index 8c2e59d93a..bde866498c 100644 --- a/crates/nu-protocol/src/ast/operator.rs +++ b/crates/nu-protocol/src/ast/operator.rs @@ -24,7 +24,7 @@ pub enum Comparison { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum Math { Plus, - Append, + Concat, Minus, Multiply, Divide, @@ -53,7 +53,7 @@ pub enum Bits { pub enum Assignment { Assign, PlusAssign, - AppendAssign, + ConcatAssign, MinusAssign, MultiplyAssign, DivideAssign, @@ -90,7 +90,7 @@ impl Operator { | Self::Comparison(Comparison::NotEqual) | Self::Comparison(Comparison::In) | Self::Comparison(Comparison::NotIn) - | Self::Math(Math::Append) => 80, + | Self::Math(Math::Concat) => 80, Self::Bits(Bits::BitAnd) => 75, Self::Bits(Bits::BitXor) => 70, Self::Bits(Bits::BitOr) => 60, @@ -107,7 +107,7 @@ impl Display for Operator { match self { Operator::Assignment(Assignment::Assign) => write!(f, "="), Operator::Assignment(Assignment::PlusAssign) => write!(f, "+="), - Operator::Assignment(Assignment::AppendAssign) => write!(f, "++="), + Operator::Assignment(Assignment::ConcatAssign) => write!(f, "++="), Operator::Assignment(Assignment::MinusAssign) => write!(f, "-="), Operator::Assignment(Assignment::MultiplyAssign) => write!(f, "*="), Operator::Assignment(Assignment::DivideAssign) => write!(f, "/="), @@ -124,7 +124,7 @@ impl Display for Operator { Operator::Comparison(Comparison::In) => write!(f, "in"), Operator::Comparison(Comparison::NotIn) => write!(f, "not-in"), Operator::Math(Math::Plus) => write!(f, "+"), - Operator::Math(Math::Append) => write!(f, "++"), + Operator::Math(Math::Concat) => write!(f, "++"), Operator::Math(Math::Minus) => write!(f, "-"), Operator::Math(Math::Multiply) => write!(f, "*"), Operator::Math(Math::Divide) => write!(f, "/"), diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index 12070f9ca3..0a65c50817 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -238,7 +238,7 @@ pub trait Eval { Math::Minus => lhs.sub(op_span, &rhs, expr_span), Math::Multiply => lhs.mul(op_span, &rhs, expr_span), Math::Divide => lhs.div(op_span, &rhs, expr_span), - Math::Append => lhs.append(op_span, &rhs, expr_span), + Math::Concat => lhs.concat(op_span, &rhs, expr_span), Math::Modulo => lhs.modulo(op_span, &rhs, expr_span), Math::FloorDivision => lhs.floor_div(op_span, &rhs, expr_span), Math::Pow => lhs.pow(op_span, &rhs, expr_span), diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 1a1e8fada2..28b468a151 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -2503,34 +2503,20 @@ impl Value { } } - pub fn append(&self, op: Span, rhs: &Value, span: Span) -> Result { + pub fn concat(&self, op: Span, rhs: &Value, span: Span) -> Result { match (self, rhs) { (Value::List { vals: lhs, .. }, Value::List { vals: rhs, .. }) => { - let mut lhs = lhs.clone(); - let mut rhs = rhs.clone(); - lhs.append(&mut rhs); - Ok(Value::list(lhs, span)) - } - (Value::List { vals: lhs, .. }, val) => { - let mut lhs = lhs.clone(); - lhs.push(val.clone()); - Ok(Value::list(lhs, span)) - } - (val, Value::List { vals: rhs, .. }) => { - let mut rhs = rhs.clone(); - rhs.insert(0, val.clone()); - Ok(Value::list(rhs, span)) + Ok(Value::list([lhs.as_slice(), rhs.as_slice()].concat(), span)) } (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => { - Ok(Value::string(lhs.to_string() + rhs, span)) - } - (Value::Binary { val: lhs, .. }, Value::Binary { val: rhs, .. }) => { - let mut val = lhs.clone(); - val.extend(rhs); - Ok(Value::binary(val, span)) + Ok(Value::string([lhs.as_str(), rhs.as_str()].join(""), span)) } + (Value::Binary { val: lhs, .. }, Value::Binary { val: rhs, .. }) => Ok(Value::binary( + [lhs.as_slice(), rhs.as_slice()].concat(), + span, + )), (Value::Custom { val: lhs, .. }, rhs) => { - lhs.operation(self.span(), Operator::Math(Math::Append), op, rhs) + lhs.operation(self.span(), Operator::Math(Math::Concat), op, rhs) } _ => Err(ShellError::OperatorMismatch { op_span: op, diff --git a/crates/nu-std/std/help/mod.nu b/crates/nu-std/std/help/mod.nu index 089bbff7f8..9f13f1bb18 100644 --- a/crates/nu-std/std/help/mod.nu +++ b/crates/nu-std/std/help/mod.nu @@ -37,7 +37,7 @@ def get-all-operators [] { return [ [Assignment, =, Assign, "Assigns a value to a variable.", 10] [Assignment, +=, PlusAssign, "Adds a value to a variable.", 10] - [Assignment, ++=, AppendAssign, "Appends a list or a value to a variable.", 10] + [Assignment, ++=, ConcatAssign, "Concatenate two lists, two strings, or two binary values.", 10] [Assignment, -=, MinusAssign, "Subtracts a value from a variable.", 10] [Assignment, *=, MultiplyAssign, "Multiplies a variable by a value.", 10] [Assignment, /=, DivideAssign, "Divides a variable by a value.", 10] @@ -55,7 +55,7 @@ def get-all-operators [] { return [ [Comparison, ends-with, EndsWith, "Checks if a string ends with another.", 80] [Comparison, not, UnaryNot, "Negates a value or expression.", 0] [Math, +, Plus, "Adds two values.", 90] - [Math, ++, Append, "Appends two lists or a list and a value.", 80] + [Math, ++, Concat, "Concatenate two lists, two strings, or two binary values.", 80] [Math, -, Minus, "Subtracts two values.", 90] [Math, *, Multiply, "Multiplies two values.", 95] [Math, /, Divide, "Divides two values.", 95] diff --git a/crates/nu_plugin_custom_values/src/cool_custom_value.rs b/crates/nu_plugin_custom_values/src/cool_custom_value.rs index ea0cf8ec92..cd160e0d0c 100644 --- a/crates/nu_plugin_custom_values/src/cool_custom_value.rs +++ b/crates/nu_plugin_custom_values/src/cool_custom_value.rs @@ -112,7 +112,7 @@ impl CustomValue for CoolCustomValue { ) -> Result { match operator { // Append the string inside `cool` - ast::Operator::Math(ast::Math::Append) => { + ast::Operator::Math(ast::Math::Concat) => { if let Some(right) = right .as_custom_value() .ok() diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index 0c4ff5d77c..a1744e1601 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -154,7 +154,6 @@ fn const_unary_operator(#[case] inp: &[&str], #[case] expect: &str) { #[case(&[r#"const x = "a" ++ "b" "#, "$x"], "ab")] #[case(&[r#"const x = [1,2] ++ [3]"#, "$x | describe"], "list")] #[case(&[r#"const x = 0x[1,2] ++ 0x[3]"#, "$x | describe"], "binary")] -#[case(&[r#"const x = 0x[1,2] ++ [3]"#, "$x | describe"], "list")] #[case(&["const x = 1 < 2", "$x"], "true")] #[case(&["const x = (3 * 200) > (2 * 100)", "$x"], "true")] #[case(&["const x = (3 * 200) < (2 * 100)", "$x"], "false")] diff --git a/tests/eval/mod.rs b/tests/eval/mod.rs index 191b19eb89..b7ad76d32d 100644 --- a/tests/eval/mod.rs +++ b/tests/eval/mod.rs @@ -159,7 +159,7 @@ fn record_spread() { #[test] fn binary_op_example() { test_eval( - "(([1 2] ++ [3 4]) == [1 2 3 4]) and (([1 2 3] ++ 4) == ([1] ++ [2 3 4]))", + "(([1 2] ++ [3 4]) == [1 2 3 4]) and (([1] ++ [2 3 4]) == [1 2 3 4])", Eq("true"), ) }