From 5fa42eeb8c3bbcc4577fa902788a006af9559b62 Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Wed, 18 May 2022 19:08:43 +0800 Subject: [PATCH] Make format support nested column and use variable (#5570) * fix format for nested structure * make little revert * add tests * fix format * better comment * make better comment --- .../nu-command/src/strings/format/command.rs | 163 ++++++++++++++---- crates/nu-command/tests/commands/format.rs | 4 - crates/nu-parser/src/lib.rs | 4 +- 3 files changed, 128 insertions(+), 43 deletions(-) diff --git a/crates/nu-command/src/strings/format/command.rs b/crates/nu-command/src/strings/format/command.rs index b0ce93f2d0..d7de9454b2 100644 --- a/crates/nu-command/src/strings/format/command.rs +++ b/crates/nu-command/src/strings/format/command.rs @@ -1,9 +1,10 @@ -use nu_engine::CallExt; +use nu_engine::{eval_expression, CallExt}; +use nu_parser::parse_expression; use nu_protocol::ast::{Call, PathMember}; -use nu_protocol::engine::{Command, EngineState, Stack}; +use nu_protocol::engine::{Command, EngineState, Stack, StateWorkingSet}; +use nu_protocol::Type; use nu_protocol::{ - Category, Config, Example, ListStream, PipelineData, ShellError, Signature, Span, SyntaxShape, - Value, + Category, Example, ListStream, PipelineData, ShellError, Signature, Span, SyntaxShape, Value, }; #[derive(Clone)] @@ -35,14 +36,31 @@ impl Command for Format { call: &Call, input: PipelineData, ) -> Result { - let config = engine_state.get_config(); + let mut working_set = StateWorkingSet::new(engine_state); + let specified_pattern: Result = call.req(engine_state, stack, 0); + let input_val = input.into_value(call.head); + // add '$it' variable to support format like this: $it.column1.column2. + let it_id = working_set.add_variable(b"$it".to_vec(), call.head, Type::Any); + stack.add_var(it_id, input_val.clone()); + match specified_pattern { Err(e) => Err(e), Ok(pattern) => { let string_pattern = pattern.as_string()?; - let ops = extract_formatting_operations(string_pattern); - format(input, &ops, call.head, config) + let string_span = pattern.span()?; + // the string span is start as `"`, we don't need the character + // to generate proper span for sub expression. + let ops = extract_formatting_operations(string_pattern, string_span.start + 1); + + format( + input_val, + &ops, + engine_state, + &mut working_set, + stack, + call.head, + ) } } } @@ -66,10 +84,20 @@ impl Command for Format { } } +// NOTE: The reason to split {column1.column2} and {$it.column1.column2}: +// for {column1.column2}, we just need to follow given record or list. +// for {$it.column1.column2} or {$variable}, we need to manually evaluate the expression. +// +// Have thought about converting from {column1.column2} to {$it.column1.column2}, but that +// will extend input relative span, finally make `nu` panic out with message: span missing in file +// contents cache. #[derive(Debug)] enum FormatOperation { FixedText(String), - ValueFromColumn(String), + // raw input is something like {column1.column2} + ValueFromColumn(String, Span), + // raw input is something like {$it.column1.column2} or {$var}. + ValueNeedEval(String, Span), } /// Given a pattern that is fed into the Format command, we can process it and subdivide it @@ -78,15 +106,21 @@ enum FormatOperation { /// there without any further processing. /// FormatOperation::ValueFromColumn contains the name of a column whose values will be /// formatted according to the input pattern. -fn extract_formatting_operations(input: String) -> Vec { +/// FormatOperation::ValueNeedEval contains expression which need to eval, it has the following form: +/// "$it.column1.column2" or "$variable" +fn extract_formatting_operations(input: String, span_start: usize) -> Vec { let mut output = vec![]; - let mut characters = input.chars(); - 'outer: loop { + let mut characters = input.char_indices(); + + let mut column_span_start = 0; + let mut column_span_end = 0; + loop { let mut before_bracket = String::new(); - for ch in &mut characters { + for (index, ch) in &mut characters { if ch == '{' { + column_span_start = index + 1; // not include '{' character. break; } before_bracket.push(ch); @@ -97,20 +131,41 @@ fn extract_formatting_operations(input: String) -> Vec { } let mut column_name = String::new(); + let mut column_need_eval = false; + for (index, ch) in &mut characters { + if ch == '$' { + column_need_eval = true; + } - for ch in &mut characters { if ch == '}' { + column_span_end = index; // not include '}' character. break; } column_name.push(ch); } if !column_name.is_empty() { - output.push(FormatOperation::ValueFromColumn(column_name.clone())); + if column_need_eval { + output.push(FormatOperation::ValueNeedEval( + column_name.clone(), + Span { + start: span_start + column_span_start, + end: span_start + column_span_end, + }, + )); + } else { + output.push(FormatOperation::ValueFromColumn( + column_name.clone(), + Span { + start: span_start + column_span_start, + end: span_start + column_span_end, + }, + )); + } } if before_bracket.is_empty() && column_name.is_empty() { - break 'outer; + break; } } output @@ -118,18 +173,26 @@ fn extract_formatting_operations(input: String) -> Vec { /// Format the incoming PipelineData according to the pattern fn format( - input_data: PipelineData, + input_data: Value, format_operations: &[FormatOperation], - span: Span, - config: &Config, + engine_state: &EngineState, + working_set: &mut StateWorkingSet, + stack: &mut Stack, + head_span: Span, ) -> Result { - let data_as_value = input_data.into_value(span); + let data_as_value = input_data; // We can only handle a Record or a List of Records match data_as_value { Value::Record { .. } => { - match format_record(format_operations, &data_as_value, span, config) { - Ok(value) => Ok(PipelineData::Value(Value::string(value, span), None)), + match format_record( + format_operations, + &data_as_value, + engine_state, + working_set, + stack, + ) { + Ok(value) => Ok(PipelineData::Value(Value::string(value, head_span), None)), Err(value) => Err(value), } } @@ -139,9 +202,15 @@ fn format( for val in vals.iter() { match val { Value::Record { .. } => { - match format_record(format_operations, val, span, config) { + match format_record( + format_operations, + val, + engine_state, + working_set, + stack, + ) { Ok(value) => { - list.push(Value::string(value, span)); + list.push(Value::string(value, head_span)); } Err(value) => { return Err(value); @@ -152,7 +221,7 @@ fn format( _ => { return Err(ShellError::UnsupportedInput( "Input data is not supported by this command.".to_string(), - span, + head_span, )) } } @@ -165,7 +234,7 @@ fn format( } _ => Err(ShellError::UnsupportedInput( "Input data is not supported by this command.".to_string(), - span, + head_span, )), } } @@ -173,29 +242,49 @@ fn format( fn format_record( format_operations: &[FormatOperation], data_as_value: &Value, - span: Span, - config: &Config, + engine_state: &EngineState, + working_set: &mut StateWorkingSet, + stack: &mut Stack, ) -> Result { + let config = engine_state.get_config(); let mut output = String::new(); + for op in format_operations { match op { FormatOperation::FixedText(s) => output.push_str(s.as_str()), - - // The referenced code suggests to use the correct Spans - // See: https://github.com/nushell/nushell/blob/c4af5df828135159633d4bc3070ce800518a42a2/crates/nu-command/src/commands/strings/format/command.rs#L61 - FormatOperation::ValueFromColumn(col_name) => { - match data_as_value - .clone() - .follow_cell_path(&[PathMember::String { - val: col_name.clone(), - span, - }]) { + FormatOperation::ValueFromColumn(col_name, span) => { + // path member should split by '.' to handle for nested structure. + let path_members: Vec = col_name + .split('.') + .map(|path| PathMember::String { + val: path.to_string(), + span: *span, + }) + .collect(); + match data_as_value.clone().follow_cell_path(&path_members) { Ok(value_at_column) => { output.push_str(value_at_column.into_string(", ", config).as_str()) } Err(se) => return Err(se), } } + FormatOperation::ValueNeedEval(_col_name, span) => { + let (exp, may_parse_err) = parse_expression(working_set, &[*span], &[]); + match may_parse_err { + None => { + let parsed_result = eval_expression(engine_state, stack, &exp); + if let Ok(val) = parsed_result { + output.push_str(&val.into_abbreviated_string(config)) + } + } + Some(err) => { + return Err(ShellError::UnsupportedInput( + format!("expression is invalid, detail message: {:?}", err), + *span, + )) + } + } + } } } Ok(output) diff --git a/crates/nu-command/tests/commands/format.rs b/crates/nu-command/tests/commands/format.rs index 128f89c32f..d3b585ecf8 100644 --- a/crates/nu-command/tests/commands/format.rs +++ b/crates/nu-command/tests/commands/format.rs @@ -16,8 +16,6 @@ fn creates_the_resulting_string_from_the_given_fields() { assert_eq!(actual.out, "nu has license ISC"); } -// FIXME: jt: needs more work -#[ignore] #[test] fn given_fields_can_be_column_paths() { let actual = nu!( @@ -31,8 +29,6 @@ fn given_fields_can_be_column_paths() { assert_eq!(actual.out, "nu is a new type of shell"); } -// FIXME: jt: needs more work -#[ignore] #[test] fn can_use_variables() { let actual = nu!( diff --git a/crates/nu-parser/src/lib.rs b/crates/nu-parser/src/lib.rs index cea85bcf31..0847ee33c2 100644 --- a/crates/nu-parser/src/lib.rs +++ b/crates/nu-parser/src/lib.rs @@ -17,8 +17,8 @@ pub use lite_parse::{lite_parse, LiteBlock}; pub use parse_keywords::*; pub use parser::{ - is_math_expression_like, parse, parse_block, parse_duration_bytes, parse_external_call, - trim_quotes, trim_quotes_str, unescape_unquote_string, Import, + is_math_expression_like, parse, parse_block, parse_duration_bytes, parse_expression, + parse_external_call, trim_quotes, trim_quotes_str, unescape_unquote_string, Import, }; #[cfg(feature = "plugin")]