From 705f12c1d998df8520afef0bc10ec24bda0662d5 Mon Sep 17 00:00:00 2001 From: Reilly Wood <26268125+rgwood@users.noreply.github.com> Date: Sat, 17 Dec 2022 09:14:12 -0800 Subject: [PATCH] Fix cell path when getting columns of non-records (#7508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A follow-up to #7497. That change made it so that `get foo` would eliminate non-record rows; I think that was an unintentional and undesirable side-effect. Before #7497: ```bash 〉[$nothing, { item: "foo" }] | get item ╭───┬─────╮ │ 0 │ │ │ 1 │ foo │ ╰───┴─────╯ ``` After #7497: ```bash 〉[$nothing, {item: "foo"}] | get item ╭───┬─────╮ │ 0 │ foo │ ╰───┴─────╯ ``` After this PR: ```bash 〉[$nothing, { item: "foo" }] | get item ╭───┬─────╮ │ 0 │ │ │ 1 │ foo │ ╰───┴─────╯ ``` cc: @merelymyself --- crates/nu-command/tests/commands/open.rs | 2 +- crates/nu-protocol/src/value/mod.rs | 32 +++++++++++++----------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/nu-command/tests/commands/open.rs b/crates/nu-command/tests/commands/open.rs index 5bce98f045..b19925c4ff 100644 --- a/crates/nu-command/tests/commands/open.rs +++ b/crates/nu-command/tests/commands/open.rs @@ -179,7 +179,7 @@ fn parses_json() { fn parses_xml() { let actual = nu!( cwd: "tests/fixtures/formats", - "open jonathan.xml | get rss.children.channel.children | get 0.item.children | get 0.link.children.0.0" + "open jonathan.xml | get rss.children.channel.children | get 0.item.children | get 3.link.children.3.0" ); assert_eq!( diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 110d627348..3f4ee7e0d1 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -715,31 +715,33 @@ impl Value { } Value::List { vals, span } => { let mut output = vec![]; - let mut hasvalue = false; - let mut temp: Result = Err(ShellError::NotFound(*span)); - let vals = vals.iter().filter(|v| matches!(v, Value::Record { .. })); + let mut found_at_least_1_value = false; for val in vals { - temp = val.clone().follow_cell_path( - &[PathMember::String { - val: column_name.clone(), - span: *origin_span, - }], - insensitive, - ); - if let Ok(result) = temp.clone() { - hasvalue = true; - output.push(result); + // only look in records; this avoids unintentionally recursing into deeply nested tables + if matches!(val, Value::Record { .. }) { + if let Ok(result) = val.clone().follow_cell_path( + &[PathMember::String { + val: column_name.clone(), + span: *origin_span, + }], + insensitive, + ) { + found_at_least_1_value = true; + output.push(result); + } else { + output.push(Value::Nothing { span: *span }); + } } else { output.push(Value::Nothing { span: *span }); } } - if hasvalue { + if found_at_least_1_value { current = Value::List { vals: output, span: *span, }; } else { - return temp; + return Err(ShellError::NotFound(*span)); } } Value::CustomValue { val, .. } => {