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

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,
+    Category, Example, ListStream, PipelineData, ShellError, Signature, Span, SyntaxShape, Value,
-    Value,
 };
 
 #[derive(Clone)]
@@ -35,14 +36,31 @@ impl Command for Format {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let config = engine_state.get_config();
+        let mut working_set = StateWorkingSet::new(engine_state);
+
         let specified_pattern: Result<Value, ShellError> = 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);
+                let string_span = pattern.span()?;
-                format(input, &ops, call.head, config)
+                // 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> {
+/// 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<FormatOperation> {
     let mut output = vec![];
 
-    let mut characters = input.chars();
+    let mut characters = input.char_indices();
-    'outer: loop {
+
+    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<FormatOperation> {
         }
 
         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<FormatOperation> {
 
 /// Format the incoming PipelineData according to the pattern
 fn format(
-    input_data: PipelineData,
+    input_data: Value,
     format_operations: &[FormatOperation],
-    span: Span,
+    engine_state: &EngineState,
-    config: &Config,
+    working_set: &mut StateWorkingSet,
+    stack: &mut Stack,
+    head_span: Span,
 ) -> Result<PipelineData, ShellError> {
-    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) {
+            match format_record(
-                Ok(value) => Ok(PipelineData::Value(Value::string(value, span), None)),
+                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,
+    engine_state: &EngineState,
-    config: &Config,
+    working_set: &mut StateWorkingSet,
+    stack: &mut Stack,
 ) -> Result<String, ShellError> {
+    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()),
-
+            FormatOperation::ValueFromColumn(col_name, span) => {
-            //  The referenced code suggests to use the correct Spans
+                // path member should split by '.' to handle for nested structure.
-            //  See: https://github.com/nushell/nushell/blob/c4af5df828135159633d4bc3070ce800518a42a2/crates/nu-command/src/commands/strings/format/command.rs#L61
+                let path_members: Vec<PathMember> = col_name
-            FormatOperation::ValueFromColumn(col_name) => {
+                    .split('.')
-                match data_as_value
+                    .map(|path| PathMember::String {
-                    .clone()
+                        val: path.to_string(),
-                    .follow_cell_path(&[PathMember::String {
+                        span: *span,
-                        val: col_name.clone(),
+                    })
-                        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)

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!(

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,
+    is_math_expression_like, parse, parse_block, parse_duration_bytes, parse_expression,
-    trim_quotes, trim_quotes_str, unescape_unquote_string, Import,
+    parse_external_call, trim_quotes, trim_quotes_str, unescape_unquote_string, Import,
 };
 
 #[cfg(feature = "plugin")]