Skip to content

Commit

Permalink
cli: ditch Deref, implement AsRef and Display for RevisionArg instead
Browse files Browse the repository at this point in the history
Deref has a super power, which we no longer need.
  • Loading branch information
yuja committed Apr 3, 2024
1 parent c596d45 commit 363b508
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 24 deletions.
23 changes: 13 additions & 10 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -808,7 +807,7 @@ impl WorkspaceCommandHelper {
&self,
revision_arg: &RevisionArg,
) -> Result<RevsetExpressionEvaluator<'_>, 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)
}

Expand All @@ -819,7 +818,7 @@ impl WorkspaceCommandHelper {
revision_arg: &RevisionArg,
) -> Result<(RevsetExpressionEvaluator<'_>, Option<RevsetModifier>), 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))
}

Expand All @@ -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)
Expand Down Expand Up @@ -2120,14 +2119,18 @@ impl From<String> for RevisionArg {
}
}

impl Deref for RevisionArg {
type Target = str;

fn deref(&self) -> &Self::Target {
impl AsRef<str> 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<NonEmptyStringValueParser, fn(String) -> RevisionArg>;

Expand Down
12 changes: 4 additions & 8 deletions cli/src/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)?;
Expand All @@ -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,
)?;
Expand Down Expand Up @@ -204,11 +204,7 @@ fn bench_revset<M: Measurement>(
group: &mut BenchmarkGroup<M>,
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<RevsetExpression>| {
Expand All @@ -231,7 +227,7 @@ fn bench_revset<M: Measurement>(
)?;

group.bench_with_input(
BenchmarkId::from_parameter(&**revset),
BenchmarkId::from_parameter(revset),
&expression,
|bencher, expression| {
bencher.iter_batched(
Expand Down
8 changes: 2 additions & 6 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 363b508

Please sign in to comment.