From 93096a07aa380b080ec7d5a3841a0cf869a8c0d7 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Fri, 10 Nov 2023 20:12:51 +0000 Subject: [PATCH] Implement `Display` for `CellPath` (#11023) # Description Because `CellPath::into_string` takes a borrowed `self`, I renamed it to `to_string` to follow Rust [API guidelines](https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv). This then triggered the clippy lint [inherent_to_string](https://rust-lang.github.io/rust-clippy/master/index.html#/inherent_to_string), which is... correct! The current `CellPath::into_string` is being used as if it were the `Display` implementation for `CellPath`. # User-Facing Changes Breaking API change for `nu-protocol`, since `CellPath::into_string` was removed. --- crates/nu-cmd-lang/src/example_support.rs | 2 +- .../src/database/commands/into_sqlite.rs | 2 +- crates/nu-command/src/debug/explain.rs | 2 +- crates/nu-command/src/filters/flatten.rs | 2 +- crates/nu-command/src/filters/select.rs | 6 ++-- crates/nu-command/src/formats/to/text.rs | 2 +- crates/nu-protocol/src/ast/cell_path.rs | 35 +++++++++---------- crates/nu-protocol/src/value/from_value.rs | 4 +-- crates/nu-protocol/src/value/mod.rs | 6 ++-- 9 files changed, 29 insertions(+), 32 deletions(-) diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index 44efa911da..e713db3f50 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -270,7 +270,7 @@ impl<'a> std::fmt::Debug for DebuggableValue<'a> { write!(f, "Binary({:?})", val) } Value::CellPath { val, .. } => { - write!(f, "CellPath({:?})", val.into_string()) + write!(f, "CellPath({:?})", val.to_string()) } Value::CustomValue { val, .. } => { write!(f, "CustomValue({:?})", val) diff --git a/crates/nu-command/src/database/commands/into_sqlite.rs b/crates/nu-command/src/database/commands/into_sqlite.rs index 3dacc338ea..5fe5c76cef 100644 --- a/crates/nu-command/src/database/commands/into_sqlite.rs +++ b/crates/nu-command/src/database/commands/into_sqlite.rs @@ -262,7 +262,7 @@ fn nu_value_to_string(value: Value, separator: &str) -> String { Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::CustomValue { val, .. } => val.value_string(), Value::MatchPattern { val, .. } => format!("{:?}", val), } diff --git a/crates/nu-command/src/debug/explain.rs b/crates/nu-command/src/debug/explain.rs index e3de8c9fd4..5fb4972649 100644 --- a/crates/nu-command/src/debug/explain.rs +++ b/crates/nu-command/src/debug/explain.rs @@ -257,7 +257,7 @@ pub fn debug_string_without_formatting(value: &Value) -> String { Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::CustomValue { val, .. } => val.value_string(), Value::MatchPattern { val, .. } => format!("{:?}", val), } diff --git a/crates/nu-command/src/filters/flatten.rs b/crates/nu-command/src/filters/flatten.rs index 59cfb8a444..e20a2d0faa 100644 --- a/crates/nu-command/src/filters/flatten.rs +++ b/crates/nu-command/src/filters/flatten.rs @@ -163,7 +163,7 @@ fn flat_value(columns: &[CellPath], item: Value, all: bool) -> Vec { let mut inner_table = None; for (column_index, (column, value)) in val.into_iter().enumerate() { - let column_requested = columns.iter().find(|c| c.into_string() == column); + let column_requested = columns.iter().find(|c| c.to_string() == column); let need_flatten = { columns.is_empty() || column_requested.is_some() }; let span = value.span(); diff --git a/crates/nu-command/src/filters/select.rs b/crates/nu-command/src/filters/select.rs index f5160f7306..dc6abea582 100644 --- a/crates/nu-command/src/filters/select.rs +++ b/crates/nu-command/src/filters/select.rs @@ -254,7 +254,7 @@ fn select( //FIXME: improve implementation to not clone match input_val.clone().follow_cell_path(&path.members, false) { Ok(fetcher) => { - record.push(path.into_string().replace('.', "_"), fetcher); + record.push(path.to_string().replace('.', "_"), fetcher); if !columns_with_value.contains(&path) { columns_with_value.push(path); } @@ -284,7 +284,7 @@ fn select( // FIXME: remove clone match v.clone().follow_cell_path(&cell_path.members, false) { Ok(result) => { - record.push(cell_path.into_string().replace('.', "_"), result); + record.push(cell_path.to_string().replace('.', "_"), result); } Err(e) => return Err(e), } @@ -309,7 +309,7 @@ fn select( //FIXME: improve implementation to not clone match x.clone().follow_cell_path(&path.members, false) { Ok(value) => { - record.push(path.into_string().replace('.', "_"), value); + record.push(path.to_string().replace('.', "_"), value); } Err(e) => return Err(e), } diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 0161d7ed26..a2e5e09e31 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -146,7 +146,7 @@ fn local_into_string(value: Value, separator: &str, config: &Config) -> String { Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::CustomValue { val, .. } => val.value_string(), Value::MatchPattern { val, .. } => format!("{:?}", val), } diff --git a/crates/nu-protocol/src/ast/cell_path.rs b/crates/nu-protocol/src/ast/cell_path.rs index 8892207ad3..e9047db9dd 100644 --- a/crates/nu-protocol/src/ast/cell_path.rs +++ b/crates/nu-protocol/src/ast/cell_path.rs @@ -1,7 +1,7 @@ use super::Expression; use crate::Span; use serde::{Deserialize, Serialize}; -use std::fmt::Write; +use std::fmt::Display; #[derive(Debug, Clone, PartialOrd, Serialize, Deserialize)] pub enum PathMember { @@ -55,24 +55,6 @@ pub struct CellPath { } impl CellPath { - pub fn into_string(&self) -> String { - let mut output = String::new(); - - for (idx, elem) in self.members.iter().enumerate() { - if idx > 0 { - output.push('.'); - } - match elem { - PathMember::Int { val, .. } => { - let _ = write!(output, "{val}"); - } - PathMember::String { val, .. } => output.push_str(val), - } - } - - output - } - pub fn make_optional(&mut self) { for member in &mut self.members { match member { @@ -87,6 +69,21 @@ impl CellPath { } } +impl Display for CellPath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for (idx, elem) in self.members.iter().enumerate() { + if idx > 0 { + write!(f, ".")?; + } + match elem { + PathMember::Int { val, .. } => write!(f, "{val}")?, + PathMember::String { val, .. } => write!(f, "{val}")?, + } + } + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct FullCellPath { pub head: Expression, diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index c40f496aae..7e6dcba786 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -170,7 +170,7 @@ impl FromValue for String { fn from_value(v: Value) -> Result { // FIXME: we may want to fail a little nicer here match v { - Value::CellPath { val, .. } => Ok(val.into_string()), + Value::CellPath { val, .. } => Ok(val.to_string()), Value::String { val, .. } => Ok(val), v => Err(ShellError::CantConvert { to_type: "string".into(), @@ -187,7 +187,7 @@ impl FromValue for Spanned { let span = v.span(); Ok(Spanned { item: match v { - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::String { val, .. } => val, v => { return Err(ShellError::CantConvert { diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 7576c0a0af..ff0c6665ff 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -725,7 +725,7 @@ impl Value { Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::CustomValue { val, .. } => val.value_string(), Value::MatchPattern { val, .. } => format!("", val), } @@ -780,7 +780,7 @@ impl Value { Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::CustomValue { val, .. } => val.value_string(), Value::MatchPattern { .. } => "".into(), } @@ -888,7 +888,7 @@ impl Value { Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), - Value::CellPath { val, .. } => val.into_string(), + Value::CellPath { val, .. } => val.to_string(), Value::CustomValue { val, .. } => val.value_string(), Value::MatchPattern { val, .. } => format!("", val), }