From 363b508441bd5f76622cd163ebf5ec1c81bbb941 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 1 Apr 2024 16:46:12 +0900 Subject: [PATCH] cli: ditch Deref, implement AsRef and Display for RevisionArg instead Deref has a super power, which we no longer need. --- cli/src/cli_util.rs | 23 +++++++++++++---------- cli/src/commands/bench.rs | 12 ++++-------- cli/src/commands/git.rs | 8 ++------ 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 9e57e8bba4..7b55b22064 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -19,7 +19,6 @@ use std::env::{self, ArgsOs, VarError}; use std::ffi::OsString; use std::fmt::Debug; use std::io::{self, Write as _}; -use std::ops::Deref; use std::path::{Path, PathBuf}; use std::process::ExitCode; use std::rc::Rc; @@ -754,7 +753,7 @@ impl WorkspaceCommandHelper { let expression = self.parse_revset(revision_arg)?; let should_hint_about_all_prefix = false; revset_util::evaluate_revset_to_single_commit( - revision_arg, + revision_arg.as_ref(), &expression, || self.commit_summary_template(), should_hint_about_all_prefix, @@ -784,7 +783,7 @@ impl WorkspaceCommandHelper { } else { let should_hint_about_all_prefix = true; let commit = revset_util::evaluate_revset_to_single_commit( - revision_arg, + revision_arg.as_ref(), &expression, || self.commit_summary_template(), should_hint_about_all_prefix, @@ -808,7 +807,7 @@ impl WorkspaceCommandHelper { &self, revision_arg: &RevisionArg, ) -> Result, CommandError> { - let expression = revset::parse(revision_arg, &self.revset_parse_context())?; + let expression = revset::parse(revision_arg.as_ref(), &self.revset_parse_context())?; self.attach_revset_evaluator(expression) } @@ -819,7 +818,7 @@ impl WorkspaceCommandHelper { revision_arg: &RevisionArg, ) -> Result<(RevsetExpressionEvaluator<'_>, Option), CommandError> { let context = self.revset_parse_context(); - let (expression, modifier) = revset::parse_with_modifier(revision_arg, &context)?; + let (expression, modifier) = revset::parse_with_modifier(revision_arg.as_ref(), &context)?; Ok((self.attach_revset_evaluator(expression)?, modifier)) } @@ -831,7 +830,7 @@ impl WorkspaceCommandHelper { let context = self.revset_parse_context(); let expressions: Vec<_> = revision_args .iter() - .map(|s| revset::parse(s, &context)) + .map(|arg| revset::parse(arg.as_ref(), &context)) .try_collect()?; let expression = RevsetExpression::union_all(&expressions); self.attach_revset_evaluator(expression) @@ -2120,14 +2119,18 @@ impl From for RevisionArg { } } -impl Deref for RevisionArg { - type Target = str; - - fn deref(&self) -> &Self::Target { +impl AsRef for RevisionArg { + fn as_ref(&self) -> &str { &self.0 } } +impl fmt::Display for RevisionArg { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + impl ValueParserFactory for RevisionArg { type Parser = MapValueParser RevisionArg>; diff --git a/cli/src/commands/bench.rs b/cli/src/commands/bench.rs index cb5a02cca7..6401a92133 100644 --- a/cli/src/commands/bench.rs +++ b/cli/src/commands/bench.rs @@ -142,7 +142,7 @@ pub(crate) fn cmd_bench( || index.common_ancestors(&[commit1.id().clone()], &[commit2.id().clone()]); run_bench( ui, - &format!("commonancestors-{}-{}", &*args.revision1, &*args.revision2), + &format!("commonancestors-{}-{}", args.revision1, args.revision2), &args.criterion, routine, )?; @@ -155,7 +155,7 @@ pub(crate) fn cmd_bench( let routine = || index.is_ancestor(ancestor_commit.id(), descendant_commit.id()); run_bench( ui, - &format!("isancestor-{}-{}", &*args.ancestor, &*args.descendant), + &format!("isancestor-{}-{}", args.ancestor, args.descendant), &args.criterion, routine, )?; @@ -204,11 +204,7 @@ fn bench_revset( group: &mut BenchmarkGroup, revset: &RevisionArg, ) -> Result<(), CommandError> { - writeln!( - ui.status(), - "----------Testing revset: {revset}----------", - revset = &**revset - )?; + writeln!(ui.status(), "----------Testing revset: {revset}----------")?; let expression = revset::optimize(workspace_command.parse_revset(revset)?.expression().clone()); // Time both evaluation and iteration. let routine = |workspace_command: &WorkspaceCommandHelper, expression: Rc| { @@ -231,7 +227,7 @@ fn bench_revset( )?; group.bench_with_input( - BenchmarkId::from_parameter(&**revset), + BenchmarkId::from_parameter(revset), &expression, |bencher, expression| { bencher.iter_batched( diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 94adf08f79..9122399631 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -14,7 +14,6 @@ use std::collections::HashSet; use std::io::Write; -use std::ops::Deref; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::{fmt, fs, io}; @@ -1130,9 +1129,7 @@ fn update_change_branches( if view.get_local_branch(&branch_name).is_absent() { writeln!( ui.status(), - "Creating branch {} for revision {}", - branch_name, - change_arg.deref() + "Creating branch {branch_name} for revision {change_arg}", )?; } tx.mut_repo() @@ -1207,10 +1204,9 @@ fn find_branches_targeted_by_revisions<'a>( expression.intersect_with(&RevsetExpression::branches(StringPattern::everything())); let mut commit_ids = expression.evaluate_to_commit_ids()?.peekable(); if commit_ids.peek().is_none() { - let rev_str: &str = rev_arg; writeln!( ui.warning_default(), - "No branches point to the specified revisions: {rev_str}" + "No branches point to the specified revisions: {rev_arg}" )?; } revision_commit_ids.extend(commit_ids);