Further improve arg errors (#3598)

* Further improve arg errors

* Fix note

* Fix note

crates/nu-command/src/commands/classified/external.rs

@@ -451,7 +451,7 @@ fn spawn(
                         }
                     }
                     let _ = stdout_read_tx.send(Ok(Value {
-                        value: UntaggedValue::Error(ShellError::external_non_zero()),
+                        value: UntaggedValue::nothing(),
                         tag: stdout_name_tag,
                     }));
                 }

crates/nu-engine/src/env/basic_host.rs

@@ -23,18 +23,17 @@ impl Host for BasicHost {
     }
 
     fn print_err(&mut self, err: ShellError, source: &Text) {
-        if let Some(diag) = err.into_diagnostic() {
+        let diag = err.into_diagnostic();
-            let source = source.to_string();
+        let source = source.to_string();
-            let mut files = codespan_reporting::files::SimpleFiles::new();
+        let mut files = codespan_reporting::files::SimpleFiles::new();
-            files.add("shell", source);
+        files.add("shell", source);
 
-            let writer = termcolor::StandardStream::stderr(termcolor::ColorChoice::Auto);
+        let writer = termcolor::StandardStream::stderr(termcolor::ColorChoice::Auto);
-            let config = codespan_reporting::term::Config::default();
+        let config = codespan_reporting::term::Config::default();
 
-            let _ = std::panic::catch_unwind(move || {
+        let _ = std::panic::catch_unwind(move || {
-                let _ = codespan_reporting::term::emit(&mut writer.lock(), &config, &files, &diag);
+            let _ = codespan_reporting::term::emit(&mut writer.lock(), &config, &files, &diag);
-            });
+        });
-        }
     }
 
     #[allow(unused_variables)]

crates/nu-engine/src/from_value.rs

@@ -37,6 +37,16 @@ impl FromValue for Tagged<num_bigint::BigInt> {
                 value: UntaggedValue::Primitive(Primitive::Duration(i)),
                 ..
             } => Ok(i.clone().tagged(tag)),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("integer", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("integer", v.spanned_type_name())),
         }
     }
@@ -57,6 +67,16 @@ impl FromValue for num_bigint::BigInt {
                 value: UntaggedValue::Primitive(Primitive::Duration(i)),
                 ..
             } => Ok(i.clone()),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("integer", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("integer", v.spanned_type_name())),
         }
     }
@@ -138,6 +158,16 @@ impl FromValue for bigdecimal::BigDecimal {
                 value: UntaggedValue::Primitive(Primitive::Int(i)),
                 ..
             } => Ok(BigDecimal::from(*i)),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("decimal", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("decimal", v.spanned_type_name())),
         }
     }
@@ -181,6 +211,16 @@ impl FromValue for String {
                 value: UntaggedValue::Primitive(Primitive::FilePath(p)),
                 ..
             } => Ok(p.to_string_lossy().to_string()),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("string", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("string", v.spanned_type_name())),
         }
     }
@@ -204,6 +244,16 @@ impl FromValue for PathBuf {
                 value: UntaggedValue::Primitive(Primitive::FilePath(p)),
                 ..
             } => Ok(p.clone()),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("filepath", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("filepath", v.spanned_type_name())),
         }
     }
@@ -220,6 +270,16 @@ impl FromValue for Tagged<PathBuf> {
                 value: UntaggedValue::Primitive(Primitive::FilePath(p)),
                 tag,
             } => Ok(p.clone().tagged(tag)),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("filepath", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("filepath", v.spanned_type_name())),
         }
     }
@@ -244,6 +304,16 @@ impl FromValue for bool {
                 value: UntaggedValue::Primitive(Primitive::Boolean(b)),
                 ..
             } => Ok(*b),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("boolean", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("boolean", v.spanned_type_name())),
         }
     }
@@ -256,6 +326,16 @@ impl FromValue for Tagged<bool> {
                 value: UntaggedValue::Primitive(Primitive::Boolean(b)),
                 tag,
             } => Ok((*b).tagged(tag)),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("boolean", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("boolean", v.spanned_type_name())),
         }
     }
@@ -268,6 +348,16 @@ impl FromValue for DateTime<FixedOffset> {
                 value: UntaggedValue::Primitive(Primitive::Date(d)),
                 ..
             } => Ok(*d),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("date", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("date", v.spanned_type_name())),
         }
     }
@@ -280,6 +370,16 @@ impl FromValue for Range {
                 value: UntaggedValue::Primitive(Primitive::Range(r)),
                 ..
             } => Ok((**r).clone()),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("range", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("range", v.spanned_type_name())),
         }
     }
@@ -293,6 +393,16 @@ impl FromValue for Tagged<Range> {
                 value: UntaggedValue::Primitive(Primitive::Range(ref range)),
                 ..
             } => Ok((*range.clone()).tagged(tag)),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("range", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("range", v.spanned_type_name())),
         }
     }
@@ -309,6 +419,16 @@ impl FromValue for Vec<u8> {
                 value: UntaggedValue::Primitive(Primitive::String(s)),
                 ..
             } => Ok(s.bytes().collect()),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("binary data", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("binary data", v.spanned_type_name())),
         }
     }
@@ -333,6 +453,16 @@ impl FromValue for CapturedBlock {
                 value: UntaggedValue::Block(b),
                 ..
             } => Ok((**b).clone()),
+            Value {
+                value: UntaggedValue::Row(_),
+                ..
+            } => {
+                let mut shell_error = ShellError::type_error("block", v.spanned_type_name());
+                shell_error.notes.push(
+                    "Note: you can access columns using dot. eg) $it.column or (ls).column".into(),
+                );
+                Err(shell_error)
+            }
             v => Err(ShellError::type_error("block", v.spanned_type_name())),
         }
     }

crates/nu-errors/src/lib.rs

@@ -202,7 +202,7 @@ impl PrettyDebug for ArgumentError {
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize, Deserialize, Hash)]
 pub struct ShellError {
     pub error: ProximateShellError,
-    pub cause: Option<Box<ShellError>>,
+    pub notes: Vec<String>,
 }
 
 /// `PrettyDebug` is for internal debugging. For user-facing debugging, [into_diagnostic](ShellError::into_diagnostic)
@@ -336,9 +336,6 @@ impl PrettyDebug for ShellError {
                 DbgDocBldr::error("Unimplemented")
                     + DbgDocBldr::delimit("(", DbgDocBldr::description(reason), ")")
             }
-            ProximateShellError::ExternalPlaceholderError => {
-                DbgDocBldr::error("non-zero external exit code")
-            }
         }
     }
 }
@@ -459,12 +456,8 @@ impl ShellError {
         ProximateShellError::Diagnostic(ShellDiagnostic { diagnostic }).start()
     }
 
-    pub fn external_non_zero() -> ShellError {
+    pub fn into_diagnostic(self) -> Diagnostic<usize> {
-        ProximateShellError::ExternalPlaceholderError.start()
+        let d = match self.error {
-    }
-
-    pub fn into_diagnostic(self) -> Option<Diagnostic<usize>> {
-        match self.error {
             ProximateShellError::MissingValue { span, reason } => {
                 let mut d = Diagnostic::bug().with_message(format!("Internal Error (missing value) :: {}", reason));
 
@@ -472,12 +465,12 @@ impl ShellError {
                     d = d.with_labels(vec![Label::primary(0, span)]);
                 }
 
-                Some(d)
+                d
             }
             ProximateShellError::ArgumentError {
                 command,
                 error,
-            } => Some(match error {
+            } => match error {
                 ArgumentError::InvalidExternalWord => Diagnostic::error().with_message("Invalid bare word for Nu command (did you intend to invoke an external command?)")
                 .with_labels(vec![Label::primary(0, command.span)]),
                 ArgumentError::UnexpectedArgument(argument) => Diagnostic::error().with_message(
@@ -530,7 +523,7 @@ impl ShellError {
                 )
                 .with_labels(vec![Label::primary(0, command.span)]),
                 ArgumentError::BadValue(msg) => Diagnostic::error().with_message(msg.clone()).with_labels(vec![Label::primary(0, command.span).with_message(msg)])
-            }),
+            },
             ProximateShellError::TypeError {
                 expected,
                 actual:
@@ -538,9 +531,9 @@ impl ShellError {
                         item: Some(actual),
                         span,
                     },
-            } => Some(Diagnostic::error().with_message("Type Error").with_labels(
+            } => Diagnostic::error().with_message("Type Error").with_labels(
                 vec![Label::primary(0, span)
-                    .with_message(format!("Expected {}, found {}", expected, actual))]),
+                    .with_message(format!("Expected {}, found {}", expected, actual))],
             ),
             ProximateShellError::TypeError {
                 expected,
@@ -549,13 +542,13 @@ impl ShellError {
                         item: None,
                         span
                     },
-            } => Some(Diagnostic::error().with_message("Type Error")
+            } => Diagnostic::error().with_message("Type Error")
-                .with_labels(vec![Label::primary(0, span).with_message(expected)])),
+                .with_labels(vec![Label::primary(0, span).with_message(expected)]),
 
             ProximateShellError::UnexpectedEof {
                 expected, span
-            } => Some(Diagnostic::error().with_message("Unexpected end of input")
+            } => Diagnostic::error().with_message("Unexpected end of input")
-                .with_labels(vec![Label::primary(0, span).with_message(format!("Expected {}", expected))])),
+                .with_labels(vec![Label::primary(0, span).with_message(format!("Expected {}", expected))]),
 
             ProximateShellError::RangeError {
                 kind,
@@ -565,13 +558,13 @@ impl ShellError {
                         item,
                         span
                     },
-            } => Some(Diagnostic::error().with_message("Range Error").with_labels(
+            } => Diagnostic::error().with_message("Range Error").with_labels(
                 vec![Label::primary(0, span).with_message(format!(
                     "Expected to convert {} to {} while {}, but it was out of range",
                     item,
                     kind.display(),
                     operation
-                ))]),
+                ))],
             ),
 
             ProximateShellError::SyntaxError {
@@ -580,8 +573,8 @@ impl ShellError {
                         span,
                         item
                     },
-            } => Some(Diagnostic::error().with_message("Syntax Error")
+            } => Diagnostic::error().with_message("Syntax Error")
-                .with_labels(vec![Label::primary(0, span).with_message(item)])),
+                .with_labels(vec![Label::primary(0, span).with_message(item)]),
 
             ProximateShellError::MissingProperty { subpath, expr, .. } => {
 
@@ -600,7 +593,7 @@ impl ShellError {
                     diag = diag.with_labels(labels);
                 }
 
-                Some(diag)
+                diag
             }
 
             ProximateShellError::InvalidIntegerIndex { subpath,integer } => {
@@ -616,20 +609,23 @@ impl ShellError {
                 labels.push(Label::secondary(0, integer).with_message("integer"));
                 diag = diag.with_labels(labels);
 
-                Some(diag)
+                diag
             }
 
-            ProximateShellError::Diagnostic(diag) => Some(diag.diagnostic),
+            ProximateShellError::Diagnostic(diag) => diag.diagnostic,
             ProximateShellError::CoerceError { left, right } => {
-                Some(Diagnostic::error().with_message("Coercion error")
+                Diagnostic::error().with_message("Coercion error")
                     .with_labels(vec![Label::primary(0, left.span).with_message(left.item),
-                    Label::secondary(0, right.span).with_message(right.item)]))
+                    Label::secondary(0, right.span).with_message(right.item)])
             }
 
-            ProximateShellError::UntaggedRuntimeError { reason } => Some(Diagnostic::error().with_message(format!("Error: {}", reason))),
+            ProximateShellError::UntaggedRuntimeError { reason } => Diagnostic::error().with_message(format!("Error: {}", reason)),
-            ProximateShellError::Unimplemented { reason } => Some(Diagnostic::error().with_message(format!("Inimplemented: {}", reason))),
+            ProximateShellError::Unimplemented { reason } => Diagnostic::error().with_message(format!("Inimplemented: {}", reason)),
-            ProximateShellError::ExternalPlaceholderError => None,
+
-        }
+        };
+
+        let notes = self.notes.clone();
+        d.with_notes(notes)
     }
 
     pub fn labeled_error(
@@ -782,14 +778,13 @@ pub enum ProximateShellError {
     Unimplemented {
         reason: String,
     },
-    ExternalPlaceholderError,
 }
 
 impl ProximateShellError {
     fn start(self) -> ShellError {
         ShellError {
-            cause: None,
             error: self,
+            notes: vec![],
         }
     }
 }
@@ -815,7 +810,6 @@ impl HasFallibleSpan for ProximateShellError {
             ProximateShellError::CoerceError { left, right } => left.span.until(right.span),
             ProximateShellError::UntaggedRuntimeError { .. } => return None,
             ProximateShellError::Unimplemented { .. } => return None,
-            ProximateShellError::ExternalPlaceholderError => return None,
         })
     }
 }