Remove unused Display implementations

After the previous commit, nushell uses PrettyDebug and
PrettyDebugWithSource for our pretty-printed display output.

PrettyDebug produces a structured `pretty.rs` document rather than
writing directly into a fmt::Formatter, and types that implement
`PrettyDebug` have a convenience `display` method that produces a string
(to be used in situations where `Display` is needed for compatibility
with other traits, or where simple rendering is appropriate).

src/commands/help.rs

@@ -130,7 +130,7 @@ impl PerItemCommand for Help {
                                         long_desc.push_str(&format!(
                                             "  --{} <{}> (required parameter){} {}\n",
                                             flag,
-                                            m,
+                                            m.display(),
                                             if ty.1.len() > 0 { ":" } else { "" },
                                             ty.1
                                         ));
@@ -139,7 +139,7 @@ impl PerItemCommand for Help {
                                         long_desc.push_str(&format!(
                                             "  --{} <{}>{} {}\n",
                                             flag,
-                                            o,
+                                            o.display(),
                                             if ty.1.len() > 0 { ":" } else { "" },
                                             ty.1
                                         ));

src/data/base.rs

@@ -245,7 +245,7 @@ impl Block {
             "EXPRS = {:?}",
             self.expressions
                 .iter()
-                .map(|e| format!("{}", e))
+                .map(|e| format!("{:?}", e))
                 .collect::<Vec<_>>()
         );
 

src/errors.rs

@@ -29,6 +29,15 @@ impl Description {
     }
 }
 
+impl PrettyDebug for Description {
+    fn pretty(&self) -> DebugDocBuilder {
+        match self {
+            Description::Source(s) => b::description(&s.item),
+            Description::Synthetic(s) => b::description(s),
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum ParseErrorReason {
     Eof {
@@ -100,12 +109,165 @@ pub enum ArgumentError {
     InvalidExternalWord,
 }
 
+impl PrettyDebug for ArgumentError {
+    fn pretty(&self) -> DebugDocBuilder {
+        match self {
+            ArgumentError::MissingMandatoryFlag(flag) => {
+                b::description("missing `")
+                    + b::description(flag)
+                    + b::description("` as mandatory flag")
+            }
+            ArgumentError::MissingMandatoryPositional(pos) => {
+                b::description("missing `")
+                    + b::description(pos)
+                    + b::description("` as mandatory positional argument")
+            }
+            ArgumentError::MissingValueForName(name) => {
+                b::description("missing value for flag `")
+                    + b::description(name)
+                    + b::description("`")
+            }
+            ArgumentError::InvalidExternalWord => b::description("invalid word"),
+        }
+    }
+}
+
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize, Deserialize, Hash)]
 pub struct ShellError {
     error: ProximateShellError,
     cause: Option<Box<ProximateShellError>>,
 }
 
+impl PrettyDebug for ShellError {
+    fn pretty(&self) -> DebugDocBuilder {
+        match &self.error {
+            ProximateShellError::SyntaxError { problem } => {
+                b::error("Syntax Error")
+                    + b::space()
+                    + b::delimit("(", b::description(&problem.item), ")")
+            }
+            ProximateShellError::UnexpectedEof { .. } => b::error("Unexpected end"),
+            ProximateShellError::TypeError { expected, actual } => {
+                b::error("Type Error")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("expected:")
+                            + b::space()
+                            + b::description(expected)
+                            + b::description(",")
+                            + b::space()
+                            + b::description("actual:")
+                            + b::space()
+                            + b::option(actual.item.as_ref().map(|actual| b::description(actual))),
+                        ")",
+                    )
+            }
+            ProximateShellError::MissingProperty { subpath, expr } => {
+                b::error("Missing Property")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("expr:")
+                            + b::space()
+                            + expr.pretty()
+                            + b::description(",")
+                            + b::space()
+                            + b::description("subpath:")
+                            + b::space()
+                            + subpath.pretty(),
+                        ")",
+                    )
+            }
+            ProximateShellError::InvalidIntegerIndex { subpath, .. } => {
+                b::error("Invalid integer index")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("subpath:") + b::space() + subpath.pretty(),
+                        ")",
+                    )
+            }
+            ProximateShellError::MissingValue { reason, .. } => {
+                b::error("Missing Value")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("reason:") + b::space() + b::description(reason),
+                        ")",
+                    )
+            }
+            ProximateShellError::ArgumentError { command, error } => {
+                b::error("Argument Error")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("command:")
+                            + b::space()
+                            + b::description(&command.item)
+                            + b::description(",")
+                            + b::space()
+                            + b::description("error:")
+                            + b::space()
+                            + error.pretty(),
+                        ")",
+                    )
+            }
+            ProximateShellError::RangeError {
+                kind,
+                actual_kind,
+                operation,
+            } => {
+                b::error("Range Error")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("expected:")
+                            + b::space()
+                            + kind.pretty()
+                            + b::description(",")
+                            + b::space()
+                            + b::description("actual:")
+                            + b::space()
+                            + b::description(&actual_kind.item)
+                            + b::description(",")
+                            + b::space()
+                            + b::description("operation:")
+                            + b::space()
+                            + b::description(operation),
+                        ")",
+                    )
+            }
+            ProximateShellError::Diagnostic(_) => b::error("diagnostic"),
+            ProximateShellError::CoerceError { left, right } => {
+                b::error("Coercion Error")
+                    + b::space()
+                    + b::delimit(
+                        "(",
+                        b::description("left:")
+                            + b::space()
+                            + b::description(&left.item)
+                            + b::description(",")
+                            + b::space()
+                            + b::description("right:")
+                            + b::space()
+                            + b::description(&right.item),
+                        ")",
+                    )
+            }
+            ProximateShellError::UntaggedRuntimeError { reason } => {
+                b::error("Unknown Error") + b::delimit("(", b::description(reason), ")")
+            }
+        }
+    }
+}
+
+impl std::fmt::Display for ShellError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.pretty().display())
+    }
+}
+
 impl serde::de::Error for ShellError {
     fn custom<T>(msg: T) -> Self
     where
@@ -464,6 +626,12 @@ impl From<Range<usize>> for ExpectedRange {
     }
 }
 
+impl PrettyDebug for ExpectedRange {
+    fn pretty(&self) -> DebugDocBuilder {
+        b::description(self.desc())
+    }
+}
+
 impl ExpectedRange {
     fn desc(&self) -> String {
         match self {
@@ -540,23 +708,6 @@ impl ProximateShellError {
             error: self,
         }
     }
-
-    // pub(crate) fn tag(&self) -> Option<Tag> {
-    //     Some(match self {
-    //         ProximateShellError::SyntaxError { problem } => problem.tag(),
-    //         ProximateShellError::UnexpectedEof { tag, .. } => tag.clone(),
-    //         ProximateShellError::InvalidCommand { command } => command.clone(),
-    //         ProximateShellError::TypeError { actual, .. } => actual.tag.clone(),
-    //         ProximateShellError::MissingProperty { tag, .. } => tag.clone(),
-    //         ProximateShellError::MissingValue { tag, .. } => return tag.clone(),
-    //         ProximateShellError::ArgumentError { tag, .. } => tag.clone(),
-    //         ProximateShellError::RangeError { actual_kind, .. } => actual_kind.tag.clone(),
-    //         ProximateShellError::InvalidIntegerIndex { integer, .. } => integer.into(),
-    //         ProximateShellError::Diagnostic(..) => return None,
-    //         ProximateShellError::UntaggedRuntimeError { .. } => return None,
-    //         ProximateShellError::CoerceError { left, right } => left.tag.until(&right.tag),
-    //     })
-    // }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -607,24 +758,6 @@ pub struct StringError {
     error: String,
 }
 
-impl std::fmt::Display for ShellError {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        match &self.error {
-            ProximateShellError::MissingValue { .. } => write!(f, "MissingValue"),
-            ProximateShellError::TypeError { .. } => write!(f, "TypeError"),
-            ProximateShellError::UnexpectedEof { .. } => write!(f, "UnexpectedEof"),
-            ProximateShellError::RangeError { .. } => write!(f, "RangeError"),
-            ProximateShellError::InvalidIntegerIndex { .. } => write!(f, "InvalidIntegerIndex"),
-            ProximateShellError::SyntaxError { .. } => write!(f, "SyntaxError"),
-            ProximateShellError::MissingProperty { .. } => write!(f, "MissingProperty"),
-            ProximateShellError::ArgumentError { .. } => write!(f, "ArgumentError"),
-            ProximateShellError::Diagnostic(_) => write!(f, "<diagnostic>"),
-            ProximateShellError::CoerceError { .. } => write!(f, "CoerceError"),
-            ProximateShellError::UntaggedRuntimeError { .. } => write!(f, "UntaggedRuntimeError"),
-        }
-    }
-}
-
 impl std::error::Error for ShellError {}
 
 impl std::convert::From<Box<dyn std::error::Error>> for ShellError {

src/evaluate/evaluator.rs

@@ -10,8 +10,8 @@ use crate::TaggedDictBuilder;
 use indexmap::IndexMap;
 use log::trace;
 use nu_source::Text;
-use std::fmt;
 
+#[derive(Debug)]
 pub struct Scope {
     it: Value,
     vars: IndexMap<String, Value>,
@@ -26,15 +26,6 @@ impl Scope {
     }
 }
 
-impl fmt::Display for Scope {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_map()
-            .entry(&"$it", &format!("{:?}", self.it.value))
-            .entries(self.vars.iter().map(|(k, v)| (k, &v.value)))
-            .finish()
-    }
-}
-
 impl Scope {
     pub(crate) fn empty() -> Scope {
         Scope {
@@ -177,7 +168,7 @@ fn evaluate_reference(
     source: &Text,
     tag: Tag,
 ) -> Result<Value, ShellError> {
-    trace!("Evaluating {} with Scope {}", name, scope);
+    trace!("Evaluating {:?} with Scope {:?}", name, scope);
     match name {
         hir::Variable::It(_) => Ok(scope.it.value.clone().into_value(tag)),
         hir::Variable::Other(inner) => match inner.slice(source) {

src/parser/hir.rs

@@ -195,39 +195,6 @@ impl PrettyDebugWithSource for Expression {
     }
 }
 
-impl std::fmt::Display for Expression {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let span = self.span;
-
-        match &self.expr {
-            RawExpression::Literal(literal) => write!(f, "{:?}", literal),
-            RawExpression::Synthetic(Synthetic::String(s)) => write!(f, "{}", s),
-            RawExpression::Command(_) => write!(f, "Command{{ {}..{} }}", span.start(), span.end()),
-            RawExpression::ExternalWord => {
-                write!(f, "ExternalWord{{ {}..{} }}", span.start(), span.end())
-            }
-            RawExpression::FilePath(file) => write!(f, "Path{{ {} }}", file.display()),
-            RawExpression::Variable(variable) => write!(f, "{}", variable),
-            RawExpression::List(list) => f
-                .debug_list()
-                .entries(list.iter().map(|e| format!("{}", e)))
-                .finish(),
-            RawExpression::Binary(binary) => write!(f, "{}", binary),
-            RawExpression::Block(items) => {
-                write!(f, "Block")?;
-                f.debug_set()
-                    .entries(items.iter().map(|i| format!("{}", i)))
-                    .finish()
-            }
-            RawExpression::Path(path) => write!(f, "{}", path),
-            RawExpression::Boolean(b) => write!(f, "${}", b),
-            RawExpression::ExternalCommand(..) => {
-                write!(f, "ExternalComment{{ {}..{} }}", span.start(), span.end())
-            }
-        }
-    }
-}
-
 impl Expression {
     pub(crate) fn number(i: impl Into<Number>, span: impl Into<Span>) -> Expression {
         let span = span.into();
@@ -405,12 +372,3 @@ pub enum Variable {
     It(Span),
     Other(Span),
 }
-
-impl std::fmt::Display for Variable {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Variable::It(_) => write!(f, "$it"),
-            Variable::Other(span) => write!(f, "${{ {}..{} }}", span.start(), span.end()),
-        }
-    }
-}

src/parser/hir/binary.rs

@@ -5,7 +5,6 @@ use derive_new::new;
 use getset::Getters;
 use nu_source::Spanned;
 use serde::{Deserialize, Serialize};
-use std::fmt;
 
 #[derive(
     Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Getters, Serialize, Deserialize, new,
@@ -31,9 +30,3 @@ impl PrettyDebugWithSource for Binary {
         .group()
     }
 }
-
-impl fmt::Display for Binary {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "({} {} {})", self.op.as_str(), self.left, self.right)
-    }
-}

src/parser/hir/path.rs

@@ -4,7 +4,6 @@ use derive_new::new;
 use getset::{Getters, MutGetters};
 use nu_source::{b, span_for_spanned_list, PrettyDebug};
 use serde::{Deserialize, Serialize};
-use std::fmt;
 
 #[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 pub enum UnspannedPathMember {
@@ -78,15 +77,6 @@ impl HasFallibleSpan for ColumnPath {
     }
 }
 
-impl fmt::Display for UnspannedPathMember {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            UnspannedPathMember::String(string) => write!(f, "{}", string),
-            UnspannedPathMember::Int(int) => write!(f, "{}", int),
-        }
-    }
-}
-
 impl PathMember {
     pub fn string(string: impl Into<String>, span: impl Into<Span>) -> PathMember {
         UnspannedPathMember::String(string.into()).into_path_member(span)
@@ -126,18 +116,6 @@ impl PrettyDebugWithSource for Path {
     }
 }
 
-impl fmt::Display for Path {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.head)?;
-
-        for entry in &self.tail {
-            write!(f, ".{}", entry.unspanned)?;
-        }
-
-        Ok(())
-    }
-}
-
 impl Path {
     pub(crate) fn parts(self) -> (Expression, Vec<PathMember>) {
         (self.head, self.tail)

src/parser/hir/syntax_shape.rs

@@ -200,22 +200,6 @@ impl ExpandExpression for SyntaxShape {
     }
 }
 
-impl std::fmt::Display for SyntaxShape {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        match self {
-            SyntaxShape::Any => write!(f, "Any"),
-            SyntaxShape::String => write!(f, "String"),
-            SyntaxShape::Int => write!(f, "Integer"),
-            SyntaxShape::Member => write!(f, "Member"),
-            SyntaxShape::ColumnPath => write!(f, "ColumnPath"),
-            SyntaxShape::Number => write!(f, "Number"),
-            SyntaxShape::Path => write!(f, "Path"),
-            SyntaxShape::Pattern => write!(f, "Pattern"),
-            SyntaxShape::Block => write!(f, "Block"),
-        }
-    }
-}
-
 #[derive(Getters, new)]
 pub struct ExpandContext<'context> {
     #[get = "pub(crate)"]

src/parser/parse/parser.rs

@@ -70,15 +70,6 @@ impl PrettyDebug for Number {
     }
 }
 
-impl std::fmt::Display for Number {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Number::Int(int) => write!(f, "{}", int),
-            Number::Decimal(decimal) => write!(f, "{}", decimal),
-        }
-    }
-}
-
 macro_rules! primitive_int {
     ($($ty:ty)*) => {
         $(

src/utils.rs

@@ -1,7 +1,7 @@
 use crate::data::base::{UntaggedValue, Value};
 use crate::errors::ShellError;
 use crate::{PathMember, UnspannedPathMember};
-use std::fmt;
+use nu_source::{b, DebugDocBuilder, PrettyDebug};
 use std::ops::Div;
 use std::path::{Component, Path, PathBuf};
 
@@ -69,16 +69,16 @@ impl From<AbsoluteFile> for PathBuf {
     }
 }
 
-impl fmt::Display for AbsoluteFile {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.inner.display())
-    }
-}
-
 pub struct AbsolutePath {
     inner: PathBuf,
 }
 
+impl PrettyDebug for AbsolutePath {
+    fn pretty(&self) -> DebugDocBuilder {
+        b::primitive(self.inner.display())
+    }
+}
+
 impl AbsolutePath {
     pub fn new(path: impl AsRef<Path>) -> AbsolutePath {
         let path = path.as_ref();
@@ -93,12 +93,6 @@ impl AbsolutePath {
     }
 }
 
-impl fmt::Display for AbsolutePath {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.inner.display())
-    }
-}
-
 impl Div<&str> for &AbsolutePath {
     type Output = AbsolutePath;
 
@@ -151,12 +145,6 @@ impl<T: AsRef<str>> Div<T> for &RelativePath {
     }
 }
 
-impl fmt::Display for RelativePath {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.inner.display())
-    }
-}
-
 pub enum TaggedValueIter<'a> {
     Empty,
     List(indexmap::map::Iter<'a, String, Value>),

tests/helpers/mod.rs

@@ -6,6 +6,7 @@ pub use std::path::PathBuf;
 
 use app_dirs::{get_app_root, AppDataType};
 use getset::Getters;
+use nu_source::PrettyDebug;
 use std::io::Read;
 use tempfile::{tempdir, TempDir};
 
@@ -45,7 +46,7 @@ impl DisplayPath for &String {
 
 impl DisplayPath for nu::AbsolutePath {
     fn display_path(&self) -> String {
-        self.to_string()
+        self.display()
     }
 }