From 7a888c9e9bb16d3410ddabb4149e3873ef2a239c Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Thu, 22 Aug 2024 02:38:43 -0700 Subject: [PATCH] Change behavior of `into record` on lists to be more useful (#13637) # Description The previous behaviour of `into record` on lists was to create a new record with each list index as the key. This was not very useful for creating meaningful records, though, and most people would end up using commands like `headers` or `transpose` to turn a list of keys and values into a record. This PR changes that instead to do what I think the most ergonomic thing is, and instead: - A list of records is merged into one record. - A list of pairs (two element lists) is folded into a record with the first element of each pair being the key, and the second being the value. The former is just generally more useful than having to use `reduce` with `merge` for such a common operation, and the latter is useful because it means that `$a | zip $b | into record` *just works* in the way that seems most obvious. Example: ```nushell [[foo bar] [baz quux]] | into record # => {foo: bar, baz: quux} [{foo: bar} {baz: quux}] | into record # => {foo: bar, baz: quux} [foo baz] | zip [bar quux] | into record # => {foo: bar, baz: quux} ``` The support for range input has been removed, as it would no longer reflect the treatment of an equivalent list. The following is equivalent to the old behavior, in case that's desired: ``` 0.. | zip [a b c] | into record # => {0: a, 1: b, 2: c} ``` # User-Facing Changes - `into record` changed as described above (breaking) - `into record` no longer supports range input (breaking) # Tests + Formatting Examples changed to match, everything works. Some usage in stdlib and `nu_plugin_nu_example` had to be changed. # After Submitting - [ ] release notes (commands, breaking change) --- .../nu-command/src/conversions/into/record.rs | 136 +++++++++++------- .../tests/commands/conversions/into/mod.rs | 1 + .../tests/commands/conversions/into/record.rs | 8 ++ crates/nu-std/std/iter.nu | 6 +- .../nu_plugin_nu_example.nu | 3 +- 5 files changed, 98 insertions(+), 56 deletions(-) create mode 100644 crates/nu-command/tests/commands/conversions/into/record.rs diff --git a/crates/nu-command/src/conversions/into/record.rs b/crates/nu-command/src/conversions/into/record.rs index 1f332d9c85..ed86c5848e 100644 --- a/crates/nu-command/src/conversions/into/record.rs +++ b/crates/nu-command/src/conversions/into/record.rs @@ -16,7 +16,6 @@ impl Command for SubCommand { (Type::Date, Type::record()), (Type::Duration, Type::record()), (Type::List(Box::new(Type::Any)), Type::record()), - (Type::Range, Type::record()), (Type::record(), Type::record()), ]) .category(Category::Conversions) @@ -32,12 +31,12 @@ impl Command for SubCommand { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { - into_record(engine_state, call, input) + into_record(call, input) } fn examples(&self) -> Vec { @@ -50,21 +49,19 @@ impl Command for SubCommand { })), }, Example { - description: "Convert from list to record", - example: "[1 2 3] | into record", + description: "Convert from list of records to record", + example: "[{foo: bar} {baz: quux}] | into record", result: Some(Value::test_record(record! { - "0" => Value::test_int(1), - "1" => Value::test_int(2), - "2" => Value::test_int(3), + "foo" => Value::test_string("bar"), + "baz" => Value::test_string("quux"), })), }, Example { - description: "Convert from range to record", - example: "0..2 | into record", + description: "Convert from list of pairs into record", + example: "[[foo bar] [baz quux]] | into record", result: Some(Value::test_record(record! { - "0" => Value::test_int(0), - "1" => Value::test_int(1), - "2" => Value::test_int(2), + "foo" => Value::test_string("bar"), + "baz" => Value::test_string("quux"), })), }, Example { @@ -103,45 +100,82 @@ impl Command for SubCommand { } } -fn into_record( - engine_state: &EngineState, - call: &Call, - input: PipelineData, -) -> Result { - let input = input.into_value(call.head)?; - let input_type = input.get_type(); - let span = input.span(); - let res = match input { - Value::Date { val, .. } => parse_date_into_record(val, span), - Value::Duration { val, .. } => parse_duration_into_record(val, span), - Value::List { mut vals, .. } => match input_type { - Type::Table(..) if vals.len() == 1 => vals.pop().expect("already checked 1 item"), - _ => Value::record( - vals.into_iter() - .enumerate() - .map(|(idx, val)| (format!("{idx}"), val)) - .collect(), - span, - ), - }, - Value::Range { val, .. } => Value::record( - val.into_range_iter(span, engine_state.signals().clone()) - .enumerate() - .map(|(idx, val)| (format!("{idx}"), val)) - .collect(), +fn into_record(call: &Call, input: PipelineData) -> Result { + let span = input.span().unwrap_or(call.head); + match input { + PipelineData::Value(Value::Date { val, .. }, _) => { + Ok(parse_date_into_record(val, span).into_pipeline_data()) + } + PipelineData::Value(Value::Duration { val, .. }, _) => { + Ok(parse_duration_into_record(val, span).into_pipeline_data()) + } + PipelineData::Value(Value::List { .. }, _) | PipelineData::ListStream(..) => { + let mut record = Record::new(); + let metadata = input.metadata(); + + enum ExpectedType { + Record, + Pair, + } + let mut expected_type = None; + + for item in input.into_iter() { + let span = item.span(); + match item { + Value::Record { val, .. } + if matches!(expected_type, None | Some(ExpectedType::Record)) => + { + // Don't use .extend() unless that gets changed to check for duplicate keys + for (key, val) in val.into_owned() { + record.insert(key, val); + } + expected_type = Some(ExpectedType::Record); + } + Value::List { mut vals, .. } + if matches!(expected_type, None | Some(ExpectedType::Pair)) => + { + if vals.len() == 2 { + let (val, key) = vals.pop().zip(vals.pop()).expect("length is < 2"); + record.insert(key.coerce_into_string()?, val); + } else { + return Err(ShellError::IncorrectValue { + msg: format!( + "expected inner list with two elements, but found {} element(s)", + vals.len() + ), + val_span: span, + call_span: call.head, + }); + } + expected_type = Some(ExpectedType::Pair); + } + Value::Nothing { .. } => {} + Value::Error { error, .. } => return Err(*error), + _ => { + return Err(ShellError::TypeMismatch { + err_message: format!( + "expected {}, found {} (while building record from list)", + match expected_type { + Some(ExpectedType::Record) => "record", + Some(ExpectedType::Pair) => "list with two elements", + None => "record or list with two elements", + }, + item.get_type(), + ), + span, + }) + } + } + } + Ok(Value::record(record, span).into_pipeline_data_with_metadata(metadata)) + } + PipelineData::Value(Value::Record { .. }, _) => Ok(input), + PipelineData::Value(Value::Error { error, .. }, _) => Err(*error), + other => Err(ShellError::TypeMismatch { + err_message: format!("Can't convert {} to record", other.get_type()), span, - ), - Value::Record { .. } => input, - Value::Error { .. } => input, - other => Value::error( - ShellError::TypeMismatch { - err_message: format!("Can't convert {} to record", other.get_type()), - span: other.span(), - }, - call.head, - ), - }; - Ok(res.into_pipeline_data()) + }), + } } fn parse_date_into_record(date: DateTime, span: Span) -> Value { diff --git a/crates/nu-command/tests/commands/conversions/into/mod.rs b/crates/nu-command/tests/commands/conversions/into/mod.rs index ad10199b5c..2d959033af 100644 --- a/crates/nu-command/tests/commands/conversions/into/mod.rs +++ b/crates/nu-command/tests/commands/conversions/into/mod.rs @@ -1,2 +1,3 @@ mod binary; mod int; +mod record; diff --git a/crates/nu-command/tests/commands/conversions/into/record.rs b/crates/nu-command/tests/commands/conversions/into/record.rs new file mode 100644 index 0000000000..d446ec22f3 --- /dev/null +++ b/crates/nu-command/tests/commands/conversions/into/record.rs @@ -0,0 +1,8 @@ +use nu_test_support::nu; + +#[test] +fn doesnt_accept_mixed_type_list_as_input() { + let actual = nu!("[{foo: bar} [quux baz]] | into record"); + assert!(!actual.status.success()); + assert!(actual.err.contains("type_mismatch")); +} diff --git a/crates/nu-std/std/iter.nu b/crates/nu-std/std/iter.nu index b64659bcc4..8c25269690 100644 --- a/crates/nu-std/std/iter.nu +++ b/crates/nu-std/std/iter.nu @@ -216,7 +216,7 @@ export def zip-with [ # -> list export def zip-into-record [ # -> table other: list # the values to zip with ] { - into record - | append ($other | into record) - | headers + zip $other + | into record + | [$in] } diff --git a/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu b/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu index 578f0dc5f3..51e2ef1ac8 100755 --- a/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu +++ b/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu @@ -148,8 +148,7 @@ def process_call [ } } }) | - each { into record } | - transpose --as-record --header-row + into record ), span: $span }