Skip to content

Commit

Permalink
cli: migrate singular parse/resolve revset argument to RevisionArg
Browse files Browse the repository at this point in the history
It doesn't make sense that plural versions take &[RevisionArg], whereas
singular ones take &str.
  • Loading branch information
yuja committed Apr 3, 2024
1 parent 056b917 commit fead2e9
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 18 deletions.
20 changes: 10 additions & 10 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Commit, CommandError> {
let expression = self.parse_revset(revision_str)?;
pub fn resolve_single_rev(&self, revision_arg: &RevisionArg) -> Result<Commit, CommandError> {
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,
Expand All @@ -771,8 +771,8 @@ impl WorkspaceCommandHelper {
revision_args: &[RevisionArg],
) -> Result<IndexSet<Commit>, 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,
Expand All @@ -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,
Expand All @@ -806,20 +806,20 @@ impl WorkspaceCommandHelper {

pub fn parse_revset(
&self,
revision_str: &str,
revision_arg: &RevisionArg,
) -> Result<RevsetExpressionEvaluator<'_>, 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)
}

// TODO: maybe better to parse all: prefix even if it is the default? It
// shouldn't be allowed in aliases, though.
fn parse_revset_with_modifier(
&self,
revision_str: &str,
revision_arg: &RevisionArg,
) -> Result<(RevsetExpressionEvaluator<'_>, Option<RevsetModifier>), 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))
}

Expand Down
14 changes: 7 additions & 7 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1110,17 +1110,17 @@ fn update_change_branches(
branch_prefix: &str,
) -> Result<Vec<String>, 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() {
// A local branch with the full change ID doesn't exist already, so use the
// 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.
Expand All @@ -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()
Expand Down Expand Up @@ -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}"
Expand Down
3 changes: 2 additions & 1 deletion cli/src/commands/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
};
Expand Down

0 comments on commit fead2e9

Please sign in to comment.