From 5fbea31d151572334008553a3ce43c64dea9e6c7 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Mon, 25 Nov 2019 10:07:20 -0800 Subject: [PATCH] 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 | 4 +- src/data/base.rs | 2 +- src/errors.rs | 203 +++++++++++++++++++++++++++------ src/evaluate/evaluator.rs | 13 +-- src/parser/hir.rs | 42 ------- src/parser/hir/binary.rs | 7 -- src/parser/hir/path.rs | 22 ---- src/parser/hir/syntax_shape.rs | 16 --- src/parser/parse/parser.rs | 9 -- src/utils.rs | 26 ++--- tests/helpers/mod.rs | 3 +- 11 files changed, 182 insertions(+), 165 deletions(-) diff --git a/src/commands/help.rs b/src/commands/help.rs index 1082b9b35e..3958f558b4 100644 --- a/src/commands/help.rs +++ b/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 )); diff --git a/src/data/base.rs b/src/data/base.rs index 857416cad8..605e2fc216 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -245,7 +245,7 @@ impl Block { "EXPRS = {:?}", self.expressions .iter() - .map(|e| format!("{}", e)) + .map(|e| format!("{:?}", e)) .collect::>() ); diff --git a/src/errors.rs b/src/errors.rs index ad2d199f5b..036240a6a1 100644 --- a/src/errors.rs +++ b/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>, } +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(msg: T) -> Self where @@ -464,6 +626,12 @@ impl From> 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 { - // 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, ""), - ProximateShellError::CoerceError { .. } => write!(f, "CoerceError"), - ProximateShellError::UntaggedRuntimeError { .. } => write!(f, "UntaggedRuntimeError"), - } - } -} - impl std::error::Error for ShellError {} impl std::convert::From> for ShellError { diff --git a/src/evaluate/evaluator.rs b/src/evaluate/evaluator.rs index af7cc5350f..fc9c702811 100644 --- a/src/evaluate/evaluator.rs +++ b/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, @@ -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 { - 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) { diff --git a/src/parser/hir.rs b/src/parser/hir.rs index ace4713358..d5ec134f6a 100644 --- a/src/parser/hir.rs +++ b/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, span: impl Into) -> 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()), - } - } -} diff --git a/src/parser/hir/binary.rs b/src/parser/hir/binary.rs index 84f99c0290..4a119cb4f1 100644 --- a/src/parser/hir/binary.rs +++ b/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) - } -} diff --git a/src/parser/hir/path.rs b/src/parser/hir/path.rs index aea856165d..2e2e6f4248 100644 --- a/src/parser/hir/path.rs +++ b/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, span: impl Into) -> 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) { (self.head, self.tail) diff --git a/src/parser/hir/syntax_shape.rs b/src/parser/hir/syntax_shape.rs index 6fa3968997..8d78c87160 100644 --- a/src/parser/hir/syntax_shape.rs +++ b/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)"] diff --git a/src/parser/parse/parser.rs b/src/parser/parse/parser.rs index f4fd2e2f06..0f5be81aa7 100644 --- a/src/parser/parse/parser.rs +++ b/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)*) => { $( diff --git a/src/utils.rs b/src/utils.rs index a786fcc866..75004aa915 100644 --- a/src/utils.rs +++ b/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 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) -> 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> Div 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>), diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index 86c8a10e7f..0172fee71b 100644 --- a/tests/helpers/mod.rs +++ b/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() } }