Make custom value type handling more consistent (#12230)

[Context on
Discord](https://discord.com/channels/601130461678272522/855947301380947968/1219425984990806207)

# Description

- Rename `CustomValue::value_string()` to `type_name()` to reflect its
usage better.
- Change print behavior to always call `to_base_value()` first, to give
the custom value better control over the output.
- Change `describe --detailed` to show the type name as the subtype,
rather than trying to describe the base value.
- Change custom `Type` to use `type_name()` rather than `typetag_name()`
to make things like `PluginCustomValue` more transparent

One question: should `describe --detailed` still include a description
of the base value somewhere? I'm torn on it, it seems possibly useful
for some things (maybe sqlite databases?), but having `describe -d` not
include the custom type name anywhere felt weird. Another option would
be to add another method to `CustomValue` for info to be displayed in
`describe`, so that it can be more type-specific?

# User-Facing Changes
Everything above has implications for printing and `describe` on custom
values

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

crates/nu-cmd-dataframe/src/dataframe/values/nu_dataframe/custom_value.rs

@@ -20,7 +20,7 @@ impl CustomValue for NuDataFrame {
         Value::custom_value(Box::new(cloned), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

crates/nu-cmd-dataframe/src/dataframe/values/nu_expression/custom_value.rs

@@ -23,7 +23,7 @@ impl CustomValue for NuExpression {
         Value::custom_value(Box::new(cloned), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

crates/nu-cmd-dataframe/src/dataframe/values/nu_lazyframe/custom_value.rs

@@ -21,7 +21,7 @@ impl CustomValue for NuLazyFrame {
         Value::custom_value(Box::new(cloned), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

crates/nu-cmd-dataframe/src/dataframe/values/nu_lazygroupby/custom_value.rs

@@ -21,7 +21,7 @@ impl CustomValue for NuLazyGroupBy {
         Value::custom_value(Box::new(cloned), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

crates/nu-cmd-dataframe/src/dataframe/values/nu_when/custom_value.rs

@@ -17,7 +17,7 @@ impl CustomValue for NuWhen {
         Value::custom_value(Box::new(cloned), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

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

@@ -235,7 +235,7 @@ fn run(
                            if options.no_collect {
                             Value::string("any", head)
                            } else {
-                            describe_value(input.into_value(head), head, engine_state, call, options)?
+                            describe_value(input.into_value(head), head, engine_state, options)?
                            }
                         },
                         "metadata" => metadata_to_value(metadata, head),
@@ -246,10 +246,7 @@ fn run(
                 Value::string("stream", head)
             } else {
                 let value = input.into_value(head);
-                let base_description = match value {
-                    Value::CustomValue { val, .. } => val.value_string(),
-                    _ => value.get_type().to_string(),
-                };
+                let base_description = value.get_type().to_string();
 
                 Value::string(format!("{} (stream)", base_description), head)
             }
@@ -257,12 +254,9 @@ fn run(
         _ => {
             let value = input.into_value(head);
             if !options.detailed {
-                match value {
-                    Value::CustomValue { val, .. } => Value::string(val.value_string(), head),
-                    _ => Value::string(value.get_type().to_string(), head),
-                }
+                Value::string(value.get_type().to_string(), head)
             } else {
-                describe_value(value, head, engine_state, call, options)?
+                describe_value(value, head, engine_state, options)?
             }
         }
     };
@@ -286,14 +280,13 @@ fn describe_value(
     value: Value,
     head: nu_protocol::Span,
     engine_state: Option<&EngineState>,
-    call: &Call,
     options: Options,
 ) -> Result<Value, ShellError> {
     Ok(match value {
-        Value::CustomValue { val, internal_span } => Value::record(
+        Value::CustomValue { val, .. } => Value::record(
             record!(
                 "type" => Value::string("custom", head),
-                "subtype" => run(engine_state,call, val.to_base_value(internal_span)?.into_pipeline_data(), options)?.into_value(head),
+                "subtype" => Value::string(val.type_name(), head),
             ),
             head,
         ),
@@ -318,7 +311,6 @@ fn describe_value(
                     std::mem::take(v),
                     head,
                     engine_state,
-                    call,
                     options,
                 )?);
             }
@@ -338,7 +330,7 @@ fn describe_value(
                 "length" => Value::int(vals.len() as i64, head),
                 "values" => Value::list(vals.into_iter().map(|v|
                     Ok(compact_primitive_description(
-                        describe_value(v, head, engine_state, call, options)?
+                        describe_value(v, head, engine_state, options)?
                     ))
                 )
                 .collect::<Result<Vec<Value>, ShellError>>()?, head),
@@ -406,7 +398,7 @@ fn describe_value(
             if options.collect_lazyrecords {
                 let collected = val.collect()?;
                 if let Value::Record { mut val, .. } =
-                    describe_value(collected, head, engine_state, call, options)?
+                    describe_value(collected, head, engine_state, options)?
                 {
                     record.push("length", Value::int(val.len() as i64, head));
                     for (_k, v) in val.iter_mut() {
@@ -414,7 +406,6 @@ fn describe_value(
                             std::mem::take(v),
                             head,
                             engine_state,
-                            call,
                             options,
                         )?);
                     }

crates/nu-command/src/database/values/sqlite.rs

@@ -353,7 +353,7 @@ impl CustomValue for SQLiteDatabase {
         Value::custom_value(Box::new(cloned), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

crates/nu-command/src/debug/explain.rs

@@ -294,6 +294,11 @@ pub fn debug_string_without_formatting(value: &Value) -> String {
         Value::Error { error, .. } => format!("{error:?}"),
         Value::Binary { val, .. } => format!("{val:?}"),
         Value::CellPath { val, .. } => val.to_string(),
-        Value::CustomValue { val, .. } => val.value_string(),
+        // If we fail to collapse the custom value, just print <{type_name}> - failure is not
+        // that critical here
+        Value::CustomValue { val, .. } => val
+            .to_base_value(value.span())
+            .map(|val| debug_string_without_formatting(&val))
+            .unwrap_or_else(|_| format!("<{}>", val.type_name())),
     }
 }

crates/nu-command/src/debug/inspect.rs

@@ -2,7 +2,7 @@ use super::inspect_table;
 use nu_protocol::{
     ast::Call,
     engine::{Command, EngineState, Stack},
-    Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Type, Value,
+    Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Type,
 };
 use terminal_size::{terminal_size, Height, Width};
 
@@ -40,10 +40,7 @@ impl Command for Inspect {
             });
         }
         let original_input = input_val.clone();
-        let description = match input_val {
-            Value::CustomValue { ref val, .. } => val.value_string(),
-            _ => input_val.get_type().to_string(),
-        };
+        let description = input_val.get_type().to_string();
 
         let (cols, _rows) = match terminal_size() {
             Some((w, h)) => (Width(w.0), Height(h.0)),

crates/nu-command/src/formats/to/text.rs

@@ -110,6 +110,7 @@ impl Iterator for ListStreamIterator {
 }
 
 fn local_into_string(value: Value, separator: &str, config: &Config) -> String {
+    let span = value.span();
     match value {
         Value::Bool { val, .. } => val.to_string(),
         Value::Int { val, .. } => val.to_string(),
@@ -148,7 +149,12 @@ fn local_into_string(value: Value, separator: &str, config: &Config) -> String {
         Value::Error { error, .. } => format!("{error:?}"),
         Value::Binary { val, .. } => format!("{val:?}"),
         Value::CellPath { val, .. } => val.to_string(),
-        Value::CustomValue { val, .. } => val.value_string(),
+        // If we fail to collapse the custom value, just print <{type_name}> - failure is not
+        // that critical here
+        Value::CustomValue { val, .. } => val
+            .to_base_value(span)
+            .map(|val| local_into_string(val, separator, config))
+            .unwrap_or_else(|_| format!("<{}>", val.type_name())),
     }
 }
 

crates/nu-plugin/src/plugin/interface/engine/tests.rs

@@ -1138,7 +1138,7 @@ impl CustomValue for CantSerialize {
         Value::custom_value(Box::new(self.clone()), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         "CantSerialize".into()
     }
 

crates/nu-plugin/src/protocol/plugin_custom_value.rs

@@ -55,7 +55,7 @@ impl CustomValue for PluginCustomValue {
         Value::custom_value(Box::new(self.clone()), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.name().to_owned()
     }
 
@@ -212,7 +212,7 @@ impl PluginCustomValue {
         custom_value: &dyn CustomValue,
         span: Span,
     ) -> Result<PluginCustomValue, ShellError> {
-        let name = custom_value.value_string();
+        let name = custom_value.type_name();
         let notify_on_drop = custom_value.notify_plugin_on_drop();
         bincode::serialize(custom_value)
             .map(|data| PluginCustomValue::new(name, data, notify_on_drop, None))
@@ -297,7 +297,7 @@ impl PluginCustomValue {
                     } else {
                         // Only PluginCustomValues can be sent
                         Err(ShellError::CustomValueIncorrectForPlugin {
-                            name: val.value_string(),
+                            name: val.type_name(),
                             span,
                             dest_plugin: source.name().to_owned(),
                             src_plugin: None,

crates/nu-plugin/src/protocol/plugin_custom_value/tests.rs

@@ -19,7 +19,7 @@ fn serialize_deserialize() -> Result<(), ShellError> {
     let original_value = TestCustomValue(32);
     let span = Span::test_data();
     let serialized = PluginCustomValue::serialize_from_custom_value(&original_value, span)?;
-    assert_eq!(original_value.value_string(), serialized.name());
+    assert_eq!(original_value.type_name(), serialized.name());
     assert!(serialized.source.is_none());
     let deserialized = serialized.deserialize_to_custom_value(span)?;
     let downcasted = deserialized

crates/nu-plugin/src/protocol/test_util.rs

@@ -14,7 +14,7 @@ impl CustomValue for TestCustomValue {
         Value::custom_value(Box::new(self.clone()), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         "TestCustomValue".into()
     }
 

crates/nu-protocol/src/value/custom_value.rs

@@ -13,8 +13,10 @@ pub trait CustomValue: fmt::Debug + Send + Sync {
 
     //fn category(&self) -> Category;
 
-    /// Define string representation of the custom value
-    fn value_string(&self) -> String;
+    /// The friendly type name to show for the custom value, e.g. in `describe` and in error
+    /// messages. This does not have to be the same as the name of the struct or enum, but
+    /// conventionally often is.
+    fn type_name(&self) -> String;
 
     /// Converts the custom value to a base nushell value.
     ///
@@ -34,7 +36,7 @@ pub trait CustomValue: fmt::Debug + Send + Sync {
     ) -> Result<Value, ShellError> {
         let _ = (self_span, index);
         Err(ShellError::IncompatiblePathAccess {
-            type_name: self.value_string(),
+            type_name: self.type_name(),
             span: path_span,
         })
     }
@@ -48,7 +50,7 @@ pub trait CustomValue: fmt::Debug + Send + Sync {
     ) -> Result<Value, ShellError> {
         let _ = (self_span, column_name);
         Err(ShellError::IncompatiblePathAccess {
-            type_name: self.value_string(),
+            type_name: self.type_name(),
             span: path_span,
         })
     }

crates/nu-protocol/src/value/mod.rs

@@ -831,7 +831,7 @@ impl Value {
             Value::Error { .. } => Type::Error,
             Value::Binary { .. } => Type::Binary,
             Value::CellPath { .. } => Type::CellPath,
-            Value::CustomValue { val, .. } => Type::Custom(val.typetag_name().into()),
+            Value::CustomValue { val, .. } => Type::Custom(val.type_name()),
         }
     }
 
@@ -947,7 +947,12 @@ impl Value {
             Value::Error { error, .. } => format!("{error:?}"),
             Value::Binary { val, .. } => format!("{val:?}"),
             Value::CellPath { val, .. } => val.to_string(),
-            Value::CustomValue { val, .. } => val.value_string(),
+            // If we fail to collapse the custom value, just print <{type_name}> - failure is not
+            // that critical here
+            Value::CustomValue { val, .. } => val
+                .to_base_value(span)
+                .map(|val| val.to_expanded_string(separator, config))
+                .unwrap_or_else(|_| format!("<{}>", val.type_name())),
         }
     }
 

crates/nu_plugin_custom_values/src/cool_custom_value.rs

@@ -50,7 +50,7 @@ impl CustomValue for CoolCustomValue {
         Value::custom_value(Box::new(self.clone()), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }
 

crates/nu_plugin_custom_values/src/drop_check.rs

@@ -31,7 +31,7 @@ impl CustomValue for DropCheckValue {
         self.clone().into_value(span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         "DropCheckValue".into()
     }
 

crates/nu_plugin_custom_values/src/second_custom_value.rs

@@ -45,7 +45,7 @@ impl CustomValue for SecondCustomValue {
         Value::custom_value(Box::new(self.clone()), span)
     }
 
-    fn value_string(&self) -> String {
+    fn type_name(&self) -> String {
         self.typetag_name().to_string()
     }