From 59ea28cf06f7107296fcf04861ea281786c75208 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 8 Nov 2023 20:47:37 +0000 Subject: [PATCH] Use `Record::get` instead of `Value` functions (#10925) # Description Where appropriate, this PR replaces instances of `Value::get_data_by_key` and `Value::follow_cell_path` with `Record::get`. This avoids some unnecessary clones and simplifies the code in some places. --- .../src/completions/custom_completions.rs | 21 +++-- crates/nu-cli/src/menus/menu_completions.rs | 28 +++---- crates/nu-cmd-base/src/hook.rs | 44 ++++------- .../nu-cmd-extra/src/extra/formats/to/html.rs | 12 +-- .../src/core_commands/error_make.rs | 62 ++++++++------- crates/nu-command/src/debug/inspect_table.rs | 39 ++++------ crates/nu-command/src/filters/compact.rs | 2 +- crates/nu-command/src/filters/join.rs | 9 ++- crates/nu-command/src/filters/split_by.rs | 22 ++++-- crates/nu-command/src/formats/to/delimited.rs | 17 ++-- crates/nu-command/src/formats/to/md.rs | 42 +++++----- crates/nu-command/src/formats/to/xml.rs | 77 ++++++++++--------- crates/nu-command/src/viewers/griddle.rs | 28 +++---- crates/nu-explore/src/nu_common/value.rs | 37 +++------ crates/nu-protocol/src/value/mod.rs | 24 +++--- crates/nu-table/src/types/expanded.rs | 34 +++----- crates/nu-table/src/types/general.rs | 23 ++---- 17 files changed, 231 insertions(+), 290 deletions(-) diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index cbcbb2a3e191..06aed6739b40 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -79,21 +79,20 @@ impl Completer for CustomCompletion { .map(|pd| { let value = pd.into_value(span); match &value { - Value::Record { .. } => { - let completions = value - .get_data_by_key("completions") + Value::Record { val, .. } => { + let completions = val + .get("completions") .and_then(|val| { val.as_list() .ok() .map(|it| map_value_completions(it.iter(), span, offset)) }) .unwrap_or_default(); - let options = value.get_data_by_key("options"); + let options = val.get("options"); - if let Some(Value::Record { .. }) = &options { - let options = options.unwrap_or_default(); + if let Some(Value::Record { val: options, .. }) = &options { let should_sort = options - .get_data_by_key("sort") + .get("sort") .and_then(|val| val.as_bool().ok()) .unwrap_or(false); @@ -103,11 +102,11 @@ impl Completer for CustomCompletion { custom_completion_options = Some(CompletionOptions { case_sensitive: options - .get_data_by_key("case_sensitive") + .get("case_sensitive") .and_then(|val| val.as_bool().ok()) .unwrap_or(true), positional: options - .get_data_by_key("positional") + .get("positional") .and_then(|val| val.as_bool().ok()) .unwrap_or(true), sort_by: if should_sort { @@ -115,9 +114,7 @@ impl Completer for CustomCompletion { } else { SortBy::None }, - match_algorithm: match options - .get_data_by_key("completion_algorithm") - { + match_algorithm: match options.get("completion_algorithm") { Some(option) => option .as_string() .ok() diff --git a/crates/nu-cli/src/menus/menu_completions.rs b/crates/nu-cli/src/menus/menu_completions.rs index c2e8b7f4c50d..b18c6540b74d 100644 --- a/crates/nu-cli/src/menus/menu_completions.rs +++ b/crates/nu-cli/src/menus/menu_completions.rs @@ -80,24 +80,18 @@ fn convert_to_suggestions( only_buffer_difference: bool, ) -> Vec { match value { - Value::Record { .. } => { - let text = value - .get_data_by_key("value") + Value::Record { val, .. } => { + let text = val + .get("value") .and_then(|val| val.as_string().ok()) .unwrap_or_else(|| "No value key".to_string()); - let description = value - .get_data_by_key("description") - .and_then(|val| val.as_string().ok()); + let description = val.get("description").and_then(|val| val.as_string().ok()); - let span = match value.get_data_by_key("span") { - Some(span @ Value::Record { .. }) => { - let start = span - .get_data_by_key("start") - .and_then(|val| val.as_int().ok()); - let end = span - .get_data_by_key("end") - .and_then(|val| val.as_int().ok()); + let span = match val.get("span") { + Some(Value::Record { val: span, .. }) => { + let start = span.get("start").and_then(|val| val.as_int().ok()); + let end = span.get("end").and_then(|val| val.as_int().ok()); match (start, end) { (Some(start), Some(end)) => { let start = start.min(end); @@ -126,12 +120,12 @@ fn convert_to_suggestions( }, }; - let extra = match value.get_data_by_key("extra") { + let extra = match val.get("extra") { Some(Value::List { vals, .. }) => { let extra: Vec = vals - .into_iter() + .iter() .filter_map(|extra| match extra { - Value::String { val, .. } => Some(val), + Value::String { val, .. } => Some(val.clone()), _ => None, }) .collect(); diff --git a/crates/nu-cmd-base/src/hook.rs b/crates/nu-cmd-base/src/hook.rs index 4f326960ad50..ad7368f88c64 100644 --- a/crates/nu-cmd-base/src/hook.rs +++ b/crates/nu-cmd-base/src/hook.rs @@ -2,7 +2,6 @@ use crate::util::get_guaranteed_cwd; use miette::Result; use nu_engine::{eval_block, eval_block_with_early_return}; use nu_parser::parse; -use nu_protocol::ast::PathMember; use nu_protocol::cli_error::{report_error, report_error_new}; use nu_protocol::engine::{EngineState, Stack, StateWorkingSet}; use nu_protocol::{BlockId, PipelineData, PositionalArg, ShellError, Span, Type, Value, VarId}; @@ -62,28 +61,8 @@ pub fn eval_hook( value: &Value, hook_name: &str, ) -> Result { - let value_span = value.span(); - - // Hooks can optionally be a record in this form: - // { - // condition: {|before, after| ... } # block that evaluates to true/false - // code: # block or a string - // } - // The condition block will be run to check whether the main hook (in `code`) should be run. - // If it returns true (the default if a condition block is not specified), the hook should be run. - let condition_path = PathMember::String { - val: "condition".to_string(), - span: value_span, - optional: false, - }; let mut output = PipelineData::empty(); - let code_path = PathMember::String { - val: "code".to_string(), - span: value_span, - optional: false, - }; - let span = value.span(); match value { Value::String { val, .. } => { @@ -161,10 +140,15 @@ pub fn eval_hook( )?; } } - Value::Record { .. } => { - let do_run_hook = if let Ok(condition) = - value.clone().follow_cell_path(&[condition_path], false) - { + Value::Record { val, .. } => { + // Hooks can optionally be a record in this form: + // { + // condition: {|before, after| ... } # block that evaluates to true/false + // code: # block or a string + // } + // The condition block will be run to check whether the main hook (in `code`) should be run. + // If it returns true (the default if a condition block is not specified), the hook should be run. + let do_run_hook = if let Some(condition) = val.get("condition") { let other_span = condition.span(); if let Ok(block_id) = condition.as_block() { match run_hook_block( @@ -204,7 +188,13 @@ pub fn eval_hook( }; if do_run_hook { - let follow = value.clone().follow_cell_path(&[code_path], false)?; + let Some(follow) = val.get("code") else { + return Err(ShellError::CantFindColumn { + col_name: "code".into(), + span, + src_span: span, + }); + }; let source_span = follow.span(); match follow { Value::String { val, .. } => { @@ -270,7 +260,7 @@ pub fn eval_hook( run_hook_block( engine_state, stack, - block_id, + *block_id, input, arguments, source_span, diff --git a/crates/nu-cmd-extra/src/extra/formats/to/html.rs b/crates/nu-cmd-extra/src/extra/formats/to/html.rs index f724f32f37b2..bd9988532b7c 100644 --- a/crates/nu-cmd-extra/src/extra/formats/to/html.rs +++ b/crates/nu-cmd-extra/src/extra/formats/to/html.rs @@ -402,15 +402,15 @@ fn html_table(table: Vec, headers: Vec, config: &Config) -> Strin for row in table { let span = row.span(); - if let Value::Record { .. } = row { + if let Value::Record { val: row, .. } = row { output_string.push_str(""); for header in &headers { - let data = row.get_data_by_key(header); + let data = row + .get(header) + .cloned() + .unwrap_or_else(|| Value::nothing(span)); output_string.push_str(""); - output_string.push_str(&html_value( - data.unwrap_or_else(|| Value::nothing(span)), - config, - )); + output_string.push_str(&html_value(data, config)); output_string.push_str(""); } output_string.push_str(""); diff --git a/crates/nu-cmd-lang/src/core_commands/error_make.rs b/crates/nu-cmd-lang/src/core_commands/error_make.rs index e28d99e120a6..47fc2161b324 100644 --- a/crates/nu-cmd-lang/src/core_commands/error_make.rs +++ b/crates/nu-cmd-lang/src/core_commands/error_make.rs @@ -2,7 +2,7 @@ use nu_engine::CallExt; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, + Category, Example, PipelineData, Record, ShellError, Signature, Span, SyntaxShape, Type, Value, }; #[derive(Clone)] @@ -118,8 +118,9 @@ impl Command for ErrorMake { const UNABLE_TO_PARSE: &str = "Unable to parse error format."; fn make_other_error(value: &Value, throw_span: Option) -> ShellError { + let span = value.span(); let value = match value { - Value::Record { .. } => value, + Value::Record { val, .. } => val, _ => { return ShellError::GenericError( "Creating error value not supported.".into(), @@ -131,13 +132,13 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { } }; - let msg = match value.get_data_by_key("msg") { - Some(Value::String { val, .. }) => val, + let msg = match value.get("msg") { + Some(Value::String { val, .. }) => val.clone(), Some(_) => { return ShellError::GenericError( UNABLE_TO_PARSE.into(), "`$.msg` has wrong type, must be string".into(), - Some(value.span()), + Some(span), None, Vec::new(), ) @@ -146,20 +147,29 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { return ShellError::GenericError( UNABLE_TO_PARSE.into(), "missing required member `$.msg`".into(), - Some(value.span()), + Some(span), None, Vec::new(), ) } }; - let help = match value.get_data_by_key("help") { - Some(Value::String { val, .. }) => Some(val), + let help = match value.get("help") { + Some(Value::String { val, .. }) => Some(val.clone()), _ => None, }; - let label = match value.get_data_by_key("label") { - Some(value) => value, + let (label, label_span) = match value.get("label") { + Some(value @ Value::Record { val, .. }) => (val, value.span()), + Some(_) => { + return ShellError::GenericError( + UNABLE_TO_PARSE.into(), + "`$.label` has wrong type, must be a record".into(), + Some(span), + None, + Vec::new(), + ) + } // correct return: no label None => { return ShellError::GenericError( @@ -173,23 +183,23 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { }; // remove after a few versions - if label.get_data_by_key("start").is_some() || label.get_data_by_key("end").is_some() { + if label.get("start").is_some() || label.get("end").is_some() { return ShellError::GenericError( UNABLE_TO_PARSE.into(), "`start` and `end` are deprecated".into(), - Some(value.span()), + Some(span), Some("Use `$.label.span` instead".into()), Vec::new(), ); } - let text = match label.get_data_by_key("text") { - Some(Value::String { val, .. }) => val, + let text = match label.get("text") { + Some(Value::String { val, .. }) => val.clone(), Some(_) => { return ShellError::GenericError( UNABLE_TO_PARSE.into(), "`$.label.text` has wrong type, must be string".into(), - Some(label.span()), + Some(label_span), None, Vec::new(), ) @@ -198,15 +208,15 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { return ShellError::GenericError( UNABLE_TO_PARSE.into(), "missing required member `$.label.text`".into(), - Some(label.span()), + Some(label_span), None, Vec::new(), ) } }; - let span = match label.get_data_by_key("span") { - Some(val @ Value::Record { .. }) => val, + let (span, span_span) = match label.get("span") { + Some(value @ Value::Record { val, .. }) => (val, value.span()), Some(value) => { return ShellError::GenericError( UNABLE_TO_PARSE.into(), @@ -220,11 +230,11 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { None => return ShellError::GenericError(msg, text, throw_span, help, Vec::new()), }; - let span_start = match get_span_sides(&span, "start") { + let span_start = match get_span_sides(span, span_span, "start") { Ok(val) => val, Err(err) => return err, }; - let span_end = match get_span_sides(&span, "end") { + let span_end = match get_span_sides(span, span_span, "end") { Ok(val) => val, Err(err) => return err, }; @@ -233,7 +243,7 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { return ShellError::GenericError( "invalid error format.".into(), "`$.label.start` should be smaller than `$.label.end`".into(), - Some(label.span()), + Some(label_span), Some(format!("{} > {}", span_start, span_end)), Vec::new(), ); @@ -249,20 +259,20 @@ fn make_other_error(value: &Value, throw_span: Option) -> ShellError { ) } -fn get_span_sides(span: &Value, side: &str) -> Result { - match span.get_data_by_key(side) { - Some(Value::Int { val, .. }) => Ok(val), +fn get_span_sides(span: &Record, span_span: Span, side: &str) -> Result { + match span.get(side) { + Some(Value::Int { val, .. }) => Ok(*val), Some(_) => Err(ShellError::GenericError( UNABLE_TO_PARSE.into(), format!("`$.span.{side}` must be int"), - Some(span.span()), + Some(span_span), None, Vec::new(), )), None => Err(ShellError::GenericError( UNABLE_TO_PARSE.into(), format!("`$.span.{side}` must be present, if span is specified."), - Some(span.span()), + Some(span_span), None, Vec::new(), )), diff --git a/crates/nu-command/src/debug/inspect_table.rs b/crates/nu-command/src/debug/inspect_table.rs index 432ac47e53ad..a79201152f3d 100644 --- a/crates/nu-command/src/debug/inspect_table.rs +++ b/crates/nu-command/src/debug/inspect_table.rs @@ -194,7 +194,7 @@ fn push_empty_column(data: &mut Vec>) { mod util { use crate::debug::explain::debug_string_without_formatting; use nu_engine::get_columns; - use nu_protocol::{ast::PathMember, Span, Value}; + use nu_protocol::Value; /// Try to build column names and a table grid. pub fn collect_input(value: Value) -> (Vec, Vec>) { @@ -265,30 +265,19 @@ mod util { } fn record_create_row(headers: &[String], item: &Value) -> Vec { - let mut rows = vec![String::default(); headers.len()]; - - for (i, header) in headers.iter().enumerate() { - let value = record_lookup_value(item, header); - rows[i] = debug_string_without_formatting(&value); - } - - rows - } - - fn record_lookup_value(item: &Value, header: &str) -> Value { - match item { - Value::Record { .. } => { - let path = PathMember::String { - val: header.to_owned(), - span: Span::unknown(), - optional: false, - }; - - item.clone() - .follow_cell_path(&[path], false) - .unwrap_or_else(|_| item.clone()) - } - item => item.clone(), + if let Value::Record { val, .. } = item { + headers + .iter() + .map(|col| { + val.get(col) + .map(debug_string_without_formatting) + .unwrap_or_else(String::new) + }) + .collect() + } else { + // should never reach here due to `get_columns` above which will return + // empty columns if any value in the list is not a record + vec![String::new(); headers.len()] } } } diff --git a/crates/nu-command/src/filters/compact.rs b/crates/nu-command/src/filters/compact.rs index 46ca6237d3e7..47dc6c33fa1a 100644 --- a/crates/nu-command/src/filters/compact.rs +++ b/crates/nu-command/src/filters/compact.rs @@ -110,7 +110,7 @@ pub fn compact( Value::Nothing { .. } => false, Value::Record { val, .. } => { for column in columns.iter() { - match item.get_data_by_key(column) { + match val.get(column) { None => return false, Some(x) => { if let Value::Nothing { .. } = x { diff --git a/crates/nu-command/src/filters/join.rs b/crates/nu-command/src/filters/join.rs index a95839d2e2f0..24549f1796fc 100644 --- a/crates/nu-command/src/filters/join.rs +++ b/crates/nu-command/src/filters/join.rs @@ -258,7 +258,7 @@ fn join_rows( val: this_record, .. } = this_row { - if let Some(this_valkey) = this_row.get_data_by_key(this_join_key) { + if let Some(this_valkey) = this_record.get(this_join_key) { if let Some(other_rows) = other.get(&this_valkey.into_string(sep, config)) { if matches!(include_inner, IncludeInner::Yes) { for other_record in other_rows { @@ -287,8 +287,9 @@ fn join_rows( .iter() .map(|key| { let val = if Some(key.as_ref()) == shared_join_key { - this_row - .get_data_by_key(key) + this_record + .get(key) + .cloned() .unwrap_or_else(|| Value::nothing(span)) } else { Value::nothing(span) @@ -339,7 +340,7 @@ fn lookup_table<'a>( let mut map = HashMap::>::with_capacity(cap); for row in rows { if let Value::Record { val: record, .. } = row { - if let Some(val) = &row.get_data_by_key(on) { + if let Some(val) = record.get(on) { let valkey = val.into_string(sep, config); map.entry(valkey).or_default().push(record); } diff --git a/crates/nu-command/src/filters/split_by.rs b/crates/nu-command/src/filters/split_by.rs index 537a01394abd..b9f820934f3f 100644 --- a/crates/nu-command/src/filters/split_by.rs +++ b/crates/nu-command/src/filters/split_by.rs @@ -125,13 +125,21 @@ pub fn split( match grouper { Grouper::ByColumn(Some(column_name)) => { - let block = move |_, row: &Value| match row.get_data_by_key(&column_name.item) { - Some(group_key) => Ok(group_key.as_string()?), - None => Err(ShellError::CantFindColumn { - col_name: column_name.item.to_string(), - span: column_name.span, - src_span: row.span(), - }), + let block = move |_, row: &Value| { + let group_key = if let Value::Record { val: row, .. } = row { + row.get(&column_name.item) + } else { + None + }; + + match group_key { + Some(group_key) => Ok(group_key.as_string()?), + None => Err(ShellError::CantFindColumn { + col_name: column_name.item.to_string(), + span: column_name.span, + src_span: row.span(), + }), + } }; data_split(values, Some(&block), span) diff --git a/crates/nu-command/src/formats/to/delimited.rs b/crates/nu-command/src/formats/to/delimited.rs index 89b2d9a1d2b2..13e7dfd7b55c 100644 --- a/crates/nu-command/src/formats/to/delimited.rs +++ b/crates/nu-command/src/formats/to/delimited.rs @@ -75,14 +75,17 @@ fn table_to_delimited( .expect("can not write."); for l in vals { - let mut row = vec![]; - for desc in &merged_descriptors { - row.push(match l.to_owned().get_data_by_key(desc) { - Some(s) => to_string_tagged_value(&s, config, head, span)?, - None => String::new(), - }); + // should always be true because of `find_non_record` above + if let Value::Record { val: l, .. } = l { + let mut row = vec![]; + for desc in &merged_descriptors { + row.push(match l.get(desc) { + Some(s) => to_string_tagged_value(s, config, head, span)?, + None => String::new(), + }); + } + wtr.write_record(&row).expect("can not write"); } - wtr.write_record(&row).expect("can not write"); } } writer_to_string(wtr).map_err(|_| make_conversion_error("table", span)) diff --git a/crates/nu-command/src/formats/to/md.rs b/crates/nu-command/src/formats/to/md.rs index 5127f18e3873..0b69cd195ec4 100644 --- a/crates/nu-command/src/formats/to/md.rs +++ b/crates/nu-command/src/formats/to/md.rs @@ -105,27 +105,24 @@ fn to_md( } fn fragment(input: Value, pretty: bool, config: &Config) -> String { - let headers = input.columns(); let mut out = String::new(); - if headers.len() == 1 { - let markup = match headers[0].to_ascii_lowercase().as_ref() { - "h1" => "# ".to_string(), - "h2" => "## ".to_string(), - "h3" => "### ".to_string(), - "blockquote" => "> ".to_string(), - - _ => return table(input.into_pipeline_data(), pretty, config), - }; - - out.push_str(&markup); - let data = match input.get_data_by_key(&headers[0]) { - Some(v) => v, - None => input, - }; - out.push_str(&data.into_string("|", config)); - } else if let Value::Record { .. } = input { - out = table(input.into_pipeline_data(), pretty, config) + if let Value::Record { val, .. } = &input { + match val.get_index(0) { + Some((header, data)) if val.len() == 1 => { + let markup = match header.to_ascii_lowercase().as_ref() { + "h1" => "# ".to_string(), + "h2" => "## ".to_string(), + "h3" => "### ".to_string(), + "blockquote" => "> ".to_string(), + _ => return table(input.into_pipeline_data(), pretty, config), + }; + + out.push_str(&markup); + out.push_str(&data.into_string("|", config)); + } + _ => out = table(input.into_pipeline_data(), pretty, config), + } } else { out = input.into_string("|", config) } @@ -164,10 +161,11 @@ fn table(input: PipelineData, pretty: bool, config: &Config) -> String { let span = row.span(); match row.to_owned() { - Value::Record { .. } => { + Value::Record { val: row, .. } => { for i in 0..headers.len() { - let data = row.get_data_by_key(&headers[i]); - let value_string = data + let value_string = row + .get(&headers[i]) + .cloned() .unwrap_or_else(|| Value::nothing(span)) .into_string(", ", config); let new_column_width = value_string.len(); diff --git a/crates/nu-command/src/formats/to/xml.rs b/crates/nu-command/src/formats/to/xml.rs index 59fca82f4e66..95a3c16701dc 100644 --- a/crates/nu-command/src/formats/to/xml.rs +++ b/crates/nu-command/src/formats/to/xml.rs @@ -109,47 +109,50 @@ fn to_xml_entry( return to_xml_text(val.as_str(), span, writer); } - if !matches!(entry, Value::Record { .. }) { - return Err(ShellError::CantConvert { + if let Value::Record { val: record, .. } = &entry { + // If key is not found it is assumed to be nothing. This way + // user can write a tag like {tag: a content: [...]} instead + // of longer {tag: a attributes: {} content: [...]} + let tag = record + .get(COLUMN_TAG_NAME) + .cloned() + .unwrap_or_else(|| Value::nothing(Span::unknown())); + let attrs = record + .get(COLUMN_ATTRS_NAME) + .cloned() + .unwrap_or_else(|| Value::nothing(Span::unknown())); + let content = record + .get(COLUMN_CONTENT_NAME) + .cloned() + .unwrap_or_else(|| Value::nothing(Span::unknown())); + + let content_span = content.span(); + let tag_span = tag.span(); + match (tag, attrs, content) { + (Value::Nothing { .. }, Value::Nothing { .. }, Value::String { val, .. }) => { + // Strings can not appear on top level of document + if top_level { + return Err(ShellError::CantConvert { + to_type: "XML".into(), + from_type: entry.get_type().to_string(), + span: entry_span, + help: Some("Strings can not be a root element of document".into()), + }); + } + to_xml_text(val.as_str(), content_span, writer) + } + (Value::String { val: tag_name, .. }, attrs, children) => to_tag_like( + entry_span, tag_name, tag_span, attrs, children, top_level, writer, + ), + _ => Ok(()), + } + } else { + Err(ShellError::CantConvert { to_type: "XML".into(), from_type: entry.get_type().to_string(), span: entry_span, help: Some("Xml entry expected to be a record".into()), - }); - }; - - // If key is not found it is assumed to be nothing. This way - // user can write a tag like {tag: a content: [...]} instead - // of longer {tag: a attributes: {} content: [...]} - let tag = entry - .get_data_by_key(COLUMN_TAG_NAME) - .unwrap_or_else(|| Value::nothing(Span::unknown())); - let attrs = entry - .get_data_by_key(COLUMN_ATTRS_NAME) - .unwrap_or_else(|| Value::nothing(Span::unknown())); - let content = entry - .get_data_by_key(COLUMN_CONTENT_NAME) - .unwrap_or_else(|| Value::nothing(Span::unknown())); - - let content_span = content.span(); - let tag_span = tag.span(); - match (tag, attrs, content) { - (Value::Nothing { .. }, Value::Nothing { .. }, Value::String { val, .. }) => { - // Strings can not appear on top level of document - if top_level { - return Err(ShellError::CantConvert { - to_type: "XML".into(), - from_type: entry.get_type().to_string(), - span: entry_span, - help: Some("Strings can not be a root element of document".into()), - }); - } - to_xml_text(val.as_str(), content_span, writer) - } - (Value::String { val: tag_name, .. }, attrs, children) => to_tag_like( - entry_span, tag_name, tag_span, attrs, children, top_level, writer, - ), - _ => Ok(()), + }) } } diff --git a/crates/nu-command/src/viewers/griddle.rs b/crates/nu-command/src/viewers/griddle.rs index a250aa83fe18..6475c2111cba 100644 --- a/crates/nu-command/src/viewers/griddle.rs +++ b/crates/nu-command/src/viewers/griddle.rs @@ -4,10 +4,10 @@ use lscolors::Style; use nu_engine::env_to_string; use nu_engine::CallExt; use nu_protocol::{ - ast::{Call, PathMember}, + ast::Call, engine::{Command, EngineState, Stack}, - Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, - SyntaxShape, Type, Value, + Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, SyntaxShape, + Type, Value, }; use nu_term_grid::grid::{Alignment, Cell, Direction, Filling, Grid, GridOptions}; use nu_utils::get_ls_colors; @@ -77,7 +77,7 @@ prints out the list properly."# match input { PipelineData::Value(Value::List { vals, .. }, ..) => { // dbg!("value::list"); - let data = convert_to_list(vals, config, call.head)?; + let data = convert_to_list(vals, config)?; if let Some(items) = data { Ok(create_grid_output( items, @@ -94,7 +94,7 @@ prints out the list properly."# } PipelineData::ListStream(stream, ..) => { // dbg!("value::stream"); - let data = convert_to_list(stream, config, call.head)?; + let data = convert_to_list(stream, config)?; if let Some(items) = data { Ok(create_grid_output( items, @@ -253,7 +253,6 @@ fn create_grid_output( fn convert_to_list( iter: impl IntoIterator, config: &Config, - head: Span, ) -> Result>, ShellError> { let mut iter = iter.into_iter().peekable(); @@ -273,21 +272,14 @@ fn convert_to_list( row.push(item.nonerror_into_string(", ", config)?) } else { for header in headers.iter().skip(1) { - let result = match item { - Value::Record { .. } => item.clone().follow_cell_path( - &[PathMember::String { - val: header.into(), - span: head, - optional: false, - }], - false, - ), - _ => Ok(item.clone()), + let result = match &item { + Value::Record { val, .. } => val.get(header), + item => Some(item), }; match result { - Ok(value) => row.push(value.nonerror_into_string(", ", config)?), - Err(_) => row.push(String::new()), + Some(value) => row.push(value.nonerror_into_string(", ", config)?), + None => row.push(String::new()), } } } diff --git a/crates/nu-explore/src/nu_common/value.rs b/crates/nu-explore/src/nu_common/value.rs index 94a578aa5677..05f83ddfa83f 100644 --- a/crates/nu-explore/src/nu_common/value.rs +++ b/crates/nu-explore/src/nu_common/value.rs @@ -1,9 +1,7 @@ use std::collections::HashMap; use nu_engine::get_columns; -use nu_protocol::{ - ast::PathMember, record, ListStream, PipelineData, PipelineMetadata, RawStream, Value, -}; +use nu_protocol::{record, ListStream, PipelineData, PipelineMetadata, RawStream, Value}; use super::NuSpan; @@ -155,30 +153,15 @@ fn create_table_for_record(headers: &[String], items: &[Value]) -> Vec Vec { - let mut rows = vec![Value::default(); headers.len()]; - - for (i, header) in headers.iter().enumerate() { - let value = record_lookup_value(item, header); - rows[i] = value; - } - - rows -} - -fn record_lookup_value(item: &Value, header: &str) -> Value { - match item { - Value::Record { .. } => { - let path = PathMember::String { - val: header.to_owned(), - span: NuSpan::unknown(), - optional: false, - }; - - item.clone() - .follow_cell_path(&[path], false) - .unwrap_or_else(|_| unknown_error_value()) - } - item => item.clone(), + if let Value::Record { val, .. } = item { + headers + .iter() + .map(|col| val.get(col).cloned().unwrap_or_else(unknown_error_value)) + .collect() + } else { + // should never reach here due to `get_columns` above which will return + // empty columns if any value in the list is not a record + vec![Value::default(); headers.len()] } } diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 7b2956cd40ac..bea42a16b5a2 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -628,21 +628,17 @@ impl Value { pub fn get_data_by_key(&self, name: &str) -> Option { let span = self.span(); match self { - Value::Record { val, .. } => val - .iter() - .find(|(col, _)| col == &name) - .map(|(_, val)| val.clone()), + Value::Record { val, .. } => val.get(name).cloned(), Value::List { vals, .. } => { - let mut out = vec![]; - for item in vals { - match item { - Value::Record { .. } => match item.get_data_by_key(name) { - Some(v) => out.push(v), - None => out.push(Value::nothing(span)), - }, - _ => out.push(Value::nothing(span)), - } - } + let out = vals + .iter() + .map(|item| { + item.as_record() + .ok() + .and_then(|val| val.get(name).cloned()) + .unwrap_or(Value::nothing(span)) + }) + .collect::>(); if !out.is_empty() { Some(Value::list(out, span)) diff --git a/crates/nu-table/src/types/expanded.rs b/crates/nu-table/src/types/expanded.rs index 1fe6e9be7a7d..4880bac94ab9 100644 --- a/crates/nu-table/src/types/expanded.rs +++ b/crates/nu-table/src/types/expanded.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use nu_color_config::{Alignment, StyleComputer, TextStyle}; use nu_engine::column::get_columns; -use nu_protocol::{ast::PathMember, Config, Record, ShellError, Span, TableIndexMode, Value}; +use nu_protocol::{Config, Record, ShellError, Span, TableIndexMode, Value}; use tabled::grid::config::Position; use crate::{ @@ -116,10 +116,11 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult { } let index = row + cfg.opts.row_offset; - let text = matches!(item, Value::Record { .. }) - .then(|| { - lookup_index_value(item, cfg.opts.config).unwrap_or_else(|| index.to_string()) - }) + let text = item + .as_record() + .ok() + .and_then(|val| val.get(INDEX_COLUMN_NAME)) + .map(|value| value.into_string("", cfg.opts.config)) .unwrap_or_else(|| index.to_string()); let row = row + with_header as usize; @@ -461,20 +462,10 @@ fn get_key_style(cfg: &Cfg<'_>) -> TextStyle { fn expanded_table_entry(item: &Value, header: &str, cfg: Cfg<'_>) -> NuText { match item { - Value::Record { .. } => { - let val = header.to_owned(); - let path = PathMember::String { - val, - span: cfg.opts.span, - optional: false, - }; - let val = item.clone().follow_cell_path(&[path], false); - - match val { - Ok(val) => expanded_table_entry2(&val, cfg), - Err(_) => error_sign(cfg.opts.style_computer), - } - } + Value::Record { val, .. } => match val.get(header) { + Some(val) => expanded_table_entry2(val, cfg), + None => error_sign(cfg.opts.style_computer), + }, _ => expanded_table_entry2(item, cfg), } } @@ -564,11 +555,6 @@ fn dive_options<'b>(cfg: &Cfg<'b>, span: Span) -> Cfg<'b> { cfg } -fn lookup_index_value(item: &Value, config: &Config) -> Option { - item.get_data_by_key(INDEX_COLUMN_NAME) - .map(|value| value.into_string("", config)) -} - fn maybe_expand_table(out: TableOutput, term_width: usize, opts: &TableOpts<'_>) -> StringResult { let mut table_config = create_nu_table_config(opts.config, opts.style_computer, &out, false); let total_width = out.table.total_width(&table_config); diff --git a/crates/nu-table/src/types/general.rs b/crates/nu-table/src/types/general.rs index c76c319b9fb5..368b7f0df22e 100644 --- a/crates/nu-table/src/types/general.rs +++ b/crates/nu-table/src/types/general.rs @@ -1,6 +1,6 @@ use nu_color_config::TextStyle; use nu_engine::column::get_columns; -use nu_protocol::{ast::PathMember, Config, Record, ShellError, Span, TableIndexMode, Value}; +use nu_protocol::{Config, Record, ShellError, TableIndexMode, Value}; use crate::{ clean_charset, colorize_space, @@ -185,19 +185,10 @@ fn to_table_with_no_header( fn get_string_value_with_header(item: &Value, header: &str, opts: &TableOpts) -> NuText { match item { - Value::Record { .. } => { - let path = PathMember::String { - val: header.to_owned(), - span: Span::unknown(), - optional: false, - }; - let value = item.clone().follow_cell_path(&[path], false); - - match value { - Ok(value) => get_string_value(&value, opts), - Err(_) => get_empty_style(opts.style_computer), - } - } + Value::Record { val, .. } => match val.get(header) { + Some(value) => get_string_value(value, opts), + None => get_empty_style(opts.style_computer), + }, value => get_string_value(value, opts), } } @@ -214,8 +205,8 @@ fn get_string_value(item: &Value, opts: &TableOpts) -> NuText { fn get_table_row_index(item: &Value, config: &Config, row: usize, offset: usize) -> String { match item { - Value::Record { .. } => item - .get_data_by_key(INDEX_COLUMN_NAME) + Value::Record { val, .. } => val + .get(INDEX_COLUMN_NAME) .map(|value| value.into_string("", config)) .unwrap_or_else(|| (row + offset).to_string()), _ => (row + offset).to_string(),