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.
This commit is contained in:
Ian Manske 2023-11-10 20:12:51 +00:00 committed by GitHub
parent 523d0bca16
commit 93096a07aa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 29 additions and 32 deletions

View file

@ -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)

View file

@ -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),
}

View file

@ -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),
}

View file

@ -163,7 +163,7 @@ fn flat_value(columns: &[CellPath], item: Value, all: bool) -> Vec<Value> {
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();

View file

@ -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),
}

View file

@ -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),
}

View file

@ -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,

View file

@ -170,7 +170,7 @@ impl FromValue for String {
fn from_value(v: Value) -> Result<Self, ShellError> {
// 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<String> {
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 {

View file

@ -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!("<Pattern: {:?}>", 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 { .. } => "<Pattern>".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!("<Pattern {:?}>", val),
}