describe refactor (#12770)

# Description

Refactors `describe` a bit. Namely, I added a `Description` enum to get
rid of `compact_primitive_description` and its awkward `Value` pattern
matching.

crates/nu-cmd-lang/src/core_commands/describe.rs

@@ -158,10 +158,10 @@ fn run(
     input: PipelineData,
     options: Options,
 ) -> Result<PipelineData, ShellError> {
-    let metadata = input.metadata().clone().map(Box::new);
     let head = call.head;
+    let metadata = input.metadata();
 
-    let description: Value = match input {
+    let description = match input {
         PipelineData::ExternalStream {
             ref stdout,
             ref stderr,
@@ -169,45 +169,54 @@ fn run(
             ..
         } => {
             if options.detailed {
+                let stdout = if stdout.is_some() {
+                    Value::record(
+                        record! {
+                            "type" => Value::string("stream", head),
+                            "origin" => Value::string("external", head),
+                            "subtype" => Value::string("any", head),
+                        },
+                        head,
+                    )
+                } else {
+                    Value::nothing(head)
+                };
+
+                let stderr = if stderr.is_some() {
+                    Value::record(
+                        record! {
+                            "type" => Value::string("stream", head),
+                            "origin" => Value::string("external", head),
+                            "subtype" => Value::string("any", head),
+                        },
+                        head,
+                    )
+                } else {
+                    Value::nothing(head)
+                };
+
+                let exit_code = if exit_code.is_some() {
+                    Value::record(
+                        record! {
+                            "type" => Value::string("stream", head),
+                            "origin" => Value::string("external", head),
+                            "subtype" => Value::string("int", head),
+                        },
+                        head,
+                    )
+                } else {
+                    Value::nothing(head)
+                };
+
                 Value::record(
-                    record!(
+                    record! {
                         "type" => Value::string("stream", head),
                         "origin" => Value::string("external", head),
-                        "stdout" => match stdout {
+                        "stdout" => stdout,
-                            Some(_) => Value::record(
+                        "stderr" => stderr,
-                                    record!(
+                        "exit_code" => exit_code,
-                                        "type" => Value::string("stream", head),
-                                        "origin" => Value::string("external", head),
-                                        "subtype" => Value::string("any", head),
-                                    ),
-                                    head,
-                                ),
-                            None => Value::nothing(head),
-                        },
-                        "stderr" => match stderr {
-                            Some(_) => Value::record(
-                                    record!(
-                                        "type" => Value::string("stream", head),
-                                        "origin" => Value::string("external", head),
-                                        "subtype" => Value::string("any", head),
-                                    ),
-                                    head,
-                                ),
-                            None => Value::nothing(head),
-                        },
-                        "exit_code" => match exit_code {
-                            Some(_) => Value::record(
-                                    record!(
-                                        "type" => Value::string("stream", head),
-                                        "origin" => Value::string("external", head),
-                                        "subtype" => Value::string("int", head),
-                                    ),
-                                    head,
-                                ),
-                            None => Value::nothing(head),
-                        },
                         "metadata" => metadata_to_value(metadata, head),
-                    ),
+                    },
                     head,
                 )
             } else {
@@ -216,19 +225,18 @@ fn run(
         }
         PipelineData::ListStream(_, _) => {
             if options.detailed {
+                let subtype = if options.no_collect {
+                    Value::string("any", head)
+                } else {
+                    describe_value(input.into_value(head), head, engine_state)
+                };
                 Value::record(
-                    record!(
+                    record! {
                         "type" => Value::string("stream", head),
                         "origin" => Value::string("nushell", head),
-                        "subtype" => {
+                        "subtype" => subtype,
-                           if options.no_collect {
-                            Value::string("any", head)
-                           } else {
-                            describe_value(input.into_value(head), head, engine_state, )
-                           }
-                        },
                         "metadata" => metadata_to_value(metadata, head),
-                    ),
+                    },
                     head,
                 )
             } else if options.no_collect {
@@ -236,7 +244,6 @@ fn run(
             } else {
                 let value = input.into_value(head);
                 let base_description = value.get_type().to_string();
-
                 Value::string(format!("{} (stream)", base_description), head)
             }
         }
@@ -253,31 +260,34 @@ fn run(
     Ok(description.into_pipeline_data())
 }
 
-fn compact_primitive_description(mut value: Value) -> Value {
+enum Description {
-    if let Value::Record { ref mut val, .. } = value {
+    String(String),
-        if val.len() != 1 {
+    Record(Record),
-            return value;
-        }
-        if let Some(type_name) = val.to_mut().get_mut("type") {
-            return std::mem::take(type_name);
-        }
-    }
-    value
 }
 
-fn describe_value(
+impl Description {
+    fn into_value(self, span: Span) -> Value {
+        match self {
+            Description::String(ty) => Value::string(ty, span),
+            Description::Record(record) => Value::record(record, span),
+        }
+    }
+}
+
+fn describe_value(value: Value, head: Span, engine_state: Option<&EngineState>) -> Value {
+    let record = match describe_value_inner(value, head, engine_state) {
+        Description::String(ty) => record! { "type" => Value::string(ty, head) },
+        Description::Record(record) => record,
+    };
+    Value::record(record, head)
+}
+
+fn describe_value_inner(
     value: Value,
-    head: nu_protocol::Span,
+    head: Span,
     engine_state: Option<&EngineState>,
-) -> Value {
+) -> Description {
     match value {
-        Value::Custom { val, .. } => Value::record(
-            record!(
-                "type" => Value::string("custom", head),
-                "subtype" => Value::string(val.type_name(), head),
-            ),
-            head,
-        ),
         Value::Bool { .. }
         | Value::Int { .. }
         | Value::Float { .. }
@@ -287,101 +297,74 @@ fn describe_value(
         | Value::Range { .. }
         | Value::String { .. }
         | Value::Glob { .. }
-        | Value::Nothing { .. } => Value::record(
+        | Value::Nothing { .. } => Description::String(value.get_type().to_string()),
-            record!(
-                "type" => Value::string(value.get_type().to_string(), head),
-            ),
-            head,
-        ),
         Value::Record { val, .. } => {
-            let mut val = val.into_owned();
+            let mut columns = val.into_owned();
-            for (_k, v) in val.iter_mut() {
+            for (_, val) in &mut columns {
-                *v = compact_primitive_description(describe_value(
+                *val =
-                    std::mem::take(v),
+                    describe_value_inner(std::mem::take(val), head, engine_state).into_value(head);
-                    head,
-                    engine_state,
-                ));
             }
 
-            Value::record(
+            Description::Record(record! {
-                record!(
+                "type" => Value::string("record", head),
-                    "type" => Value::string("record", head),
+                "columns" => Value::record(columns, head),
-                    "columns" => Value::record(val, head),
+            })
-                ),
-                head,
-            )
         }
-        Value::List { vals, .. } => Value::record(
+        Value::List { mut vals, .. } => {
-            record!(
+            for val in &mut vals {
+                *val =
+                    describe_value_inner(std::mem::take(val), head, engine_state).into_value(head);
+            }
+
+            Description::Record(record! {
                 "type" => Value::string("list", head),
                 "length" => Value::int(vals.len() as i64, head),
-                "values" => Value::list(vals.into_iter().map(|v|
+                "values" => Value::list(vals, head),
-                    compact_primitive_description(describe_value(v, head, engine_state))
+            })
-                )
+        }
-                .collect(), head),
-            ),
-            head,
-        ),
         Value::Closure { val, .. } => {
             let block = engine_state.map(|engine_state| engine_state.get_block(val.block_id));
 
+            let mut record = record! { "type" => Value::string("closure", head) };
             if let Some(block) = block {
-                let mut record = Record::new();
-                record.push("type", Value::string("closure", head));
                 record.push(
                     "signature",
                     Value::record(
-                        record!(
+                        record! {
                             "name" => Value::string(block.signature.name.clone(), head),
                             "category" => Value::string(block.signature.category.to_string(), head),
-                        ),
+                        },
                         head,
                     ),
                 );
-                Value::record(record, head)
-            } else {
-                Value::record(
-                    record!(
-                        "type" => Value::string("closure", head),
-                    ),
-                    head,
-                )
             }
+            Description::Record(record)
         }
-
+        Value::Error { error, .. } => Description::Record(record! {
-        Value::Error { error, .. } => Value::record(
+            "type" => Value::string("error", head),
-            record!(
+            "subtype" => Value::string(error.to_string(), head),
-                "type" => Value::string("error", head),
+        }),
-                "subtype" => Value::string(error.to_string(), head),
+        Value::Binary { val, .. } => Description::Record(record! {
-            ),
+            "type" => Value::string("binary", head),
-            head,
+            "length" => Value::int(val.len() as i64, head),
-        ),
+        }),
-        Value::Binary { val, .. } => Value::record(
+        Value::CellPath { val, .. } => Description::Record(record! {
-            record!(
+            "type" => Value::string("cell-path", head),
-                "type" => Value::string("binary", head),
+            "length" => Value::int(val.members.len() as i64, head),
-                "length" => Value::int(val.len() as i64, head),
+        }),
-            ),
+        Value::Custom { val, .. } => Description::Record(record! {
-            head,
+            "type" => Value::string("custom", head),
-        ),
+            "subtype" => Value::string(val.type_name(), head),
-        Value::CellPath { val, .. } => Value::record(
+        }),
-            record!(
-                "type" => Value::string("cellpath", head),
-                "length" => Value::int(val.members.len() as i64, head),
-            ),
-            head,
-        ),
     }
 }
 
-fn metadata_to_value(metadata: Option<Box<PipelineMetadata>>, head: nu_protocol::Span) -> Value {
+fn metadata_to_value(metadata: Option<PipelineMetadata>, head: Span) -> Value {
-    match metadata {
+    if let Some(metadata) = metadata {
-        Some(metadata) => Value::record(
+        let data_source = Value::string(format!("{:?}", metadata.data_source), head);
-            record!(
+        Value::record(record! { "data_source" => data_source }, head)
-                "data_source" => Value::string(format!("{:?}", metadata.data_source), head),
+    } else {
-            ),
+        Value::nothing(head)
-            head,
-        ),
-        _ => Value::nothing(head),
     }
 }