From fead2e9222262956c48e33ec9e7c181be18be0de Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 1 Apr 2024 16:31:29 +0900 Subject: [PATCH] cli: migrate singular parse/resolve revset argument to RevisionArg It doesn't make sense that plural versions take &[RevisionArg], whereas singular ones take &str. --- cli/src/cli_util.rs | 20 ++++++++++---------- cli/src/commands/git.rs | 14 +++++++------- cli/src/commands/log.rs | 3 ++- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index b5128ae8bf..e48c138b77 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -750,11 +750,11 @@ impl WorkspaceCommandHelper { /// Resolve a revset to a single revision. Return an error if the revset is /// empty or has multiple revisions. - pub fn resolve_single_rev(&self, revision_str: &str) -> Result { - let expression = self.parse_revset(revision_str)?; + pub fn resolve_single_rev(&self, revision_arg: &RevisionArg) -> Result { + let expression = self.parse_revset(revision_arg)?; let should_hint_about_all_prefix = false; revset_util::evaluate_revset_to_single_commit( - revision_str, + revision_arg, &expression, || self.commit_summary_template(), should_hint_about_all_prefix, @@ -771,8 +771,8 @@ impl WorkspaceCommandHelper { revision_args: &[RevisionArg], ) -> Result, CommandError> { let mut all_commits = IndexSet::new(); - for revision_str in revision_args { - let (expression, modifier) = self.parse_revset_with_modifier(revision_str)?; + for revision_arg in revision_args { + let (expression, modifier) = self.parse_revset_with_modifier(revision_arg)?; let all = match modifier { Some(RevsetModifier::All) => true, None => false, @@ -784,7 +784,7 @@ impl WorkspaceCommandHelper { } else { let should_hint_about_all_prefix = true; let commit = revset_util::evaluate_revset_to_single_commit( - revision_str, + revision_arg, &expression, || self.commit_summary_template(), should_hint_about_all_prefix, @@ -806,9 +806,9 @@ impl WorkspaceCommandHelper { pub fn parse_revset( &self, - revision_str: &str, + revision_arg: &RevisionArg, ) -> Result, CommandError> { - let expression = revset::parse(revision_str, &self.revset_parse_context())?; + let expression = revset::parse(revision_arg, &self.revset_parse_context())?; self.attach_revset_evaluator(expression) } @@ -816,10 +816,10 @@ impl WorkspaceCommandHelper { // shouldn't be allowed in aliases, though. fn parse_revset_with_modifier( &self, - revision_str: &str, + revision_arg: &RevisionArg, ) -> Result<(RevsetExpressionEvaluator<'_>, Option), CommandError> { let context = self.revset_parse_context(); - let (expression, modifier) = revset::parse_with_modifier(revision_str, &context)?; + let (expression, modifier) = revset::parse_with_modifier(revision_arg, &context)?; Ok((self.attach_revset_evaluator(expression)?, modifier)) } diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index d9853c5493..94adf08f79 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -1110,9 +1110,9 @@ fn update_change_branches( branch_prefix: &str, ) -> Result, CommandError> { let mut branch_names = Vec::new(); - for change_str in changes { + for change_arg in changes { let workspace_command = tx.base_workspace_helper(); - let commit = workspace_command.resolve_single_rev(change_str)?; + let commit = workspace_command.resolve_single_rev(change_arg)?; let mut branch_name = format!("{branch_prefix}{}", commit.change_id().hex()); let view = tx.base_repo().view(); if view.get_local_branch(&branch_name).is_absent() { @@ -1120,7 +1120,7 @@ fn update_change_branches( // short ID if it's not ambiguous (which it shouldn't be most of the time). let short_change_id = short_change_hash(commit.change_id()); if workspace_command - .resolve_single_rev(&short_change_id) + .resolve_single_rev(&RevisionArg::from(short_change_id.clone())) .is_ok() { // Short change ID is not ambiguous, so update the branch name to use it. @@ -1132,7 +1132,7 @@ fn update_change_branches( ui.status(), "Creating branch {} for revision {}", branch_name, - change_str.deref() + change_arg.deref() )?; } tx.mut_repo() @@ -1202,12 +1202,12 @@ fn find_branches_targeted_by_revisions<'a>( )?; } } - for rev_str in revisions { - let mut expression = workspace_command.parse_revset(rev_str)?; + for rev_arg in revisions { + let mut expression = workspace_command.parse_revset(rev_arg)?; 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_str; + let rev_str: &str = rev_arg; writeln!( ui.warning_default(), "No branches point to the specified revisions: {rev_str}" diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 93ca0f43e4..70d3d4c712 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -78,7 +78,8 @@ pub(crate) fn cmd_log( let revset_expression = { let mut expression = if args.revisions.is_empty() { - workspace_command.parse_revset(&command.settings().default_revset())? + workspace_command + .parse_revset(&RevisionArg::from(command.settings().default_revset()))? } else { workspace_command.parse_union_revsets(&args.revisions)? };