cli: move resolve_multiple_nonempty_revsets_default_single() to command helper

resolve_revset_default_single() will be inlined instead. Since "default_single"
evaluation can return multiple revisions, the CLI interface usually accepts
multiple arguments. This suggests that there would be no external callers of
the singular resolve_revset_default_single().
This commit is contained in:
Yuya Nishihara 2024-03-31 12:59:30 +09:00
parent 0c4f4c8767
commit a8c4e0ecbd
4 changed files with 46 additions and 44 deletions

View file

@ -753,6 +753,34 @@ impl WorkspaceCommandHelper {
self.resolve_single_rev_with_hint_about_all_prefix(revision_str, false) self.resolve_single_rev_with_hint_about_all_prefix(revision_str, false)
} }
/// Evaluates revset expressions to non-empty set of commits. The returned
/// set preserves the order of the input expressions.
///
/// If an input expression is prefixed with `all:`, it may be evaluated to
/// any number of revisions (including 0.)
pub fn resolve_some_revsets_default_single(
&self,
revision_args: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let mut all_commits = IndexSet::new();
for revision_str in revision_args {
let commits = self.resolve_revset_default_single(revision_str)?;
for commit in commits {
let commit_hash = short_commit_hash(commit.id());
if !all_commits.insert(commit) {
return Err(user_error(format!(
r#"More than one revset resolved to revision {commit_hash}"#,
)));
}
}
}
if all_commits.is_empty() {
Err(user_error("Empty revision set"))
} else {
Ok(all_commits)
}
}
/// Resolve a revset any number of revisions (including 0). /// Resolve a revset any number of revisions (including 0).
pub fn resolve_revset(&self, revision_str: &str) -> Result<Vec<Commit>, CommandError> { pub fn resolve_revset(&self, revision_str: &str) -> Result<Vec<Commit>, CommandError> {
Ok(self Ok(self
@ -764,7 +792,7 @@ impl WorkspaceCommandHelper {
/// Resolve a revset any number of revisions (including 0), but require the /// Resolve a revset any number of revisions (including 0), but require the
/// user to indicate if they allow multiple revisions by prefixing the /// user to indicate if they allow multiple revisions by prefixing the
/// expression with `all:`. /// expression with `all:`.
pub fn resolve_revset_default_single( fn resolve_revset_default_single(
&self, &self,
revision_str: &str, revision_str: &str,
) -> Result<Vec<Commit>, CommandError> { ) -> Result<Vec<Commit>, CommandError> {
@ -1598,29 +1626,6 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> {
Ok(()) Ok(())
} }
pub fn resolve_multiple_nonempty_revsets_default_single(
workspace_command: &WorkspaceCommandHelper,
revisions: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let mut all_commits = IndexSet::new();
for revision_str in revisions {
let commits = workspace_command.resolve_revset_default_single(revision_str)?;
for commit in commits {
let commit_hash = short_commit_hash(commit.id());
if !all_commits.insert(commit) {
return Err(user_error(format!(
r#"More than one revset resolved to revision {commit_hash}"#,
)));
}
}
}
if all_commits.is_empty() {
Err(user_error("Empty revision set"))
} else {
Ok(all_commits)
}
}
pub fn update_working_copy( pub fn update_working_copy(
repo: &Arc<ReadonlyRepo>, repo: &Arc<ReadonlyRepo>,
workspace: &mut Workspace, workspace: &mut Workspace,

View file

@ -22,9 +22,7 @@ use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::rewrite::{merge_commit_trees, rebase_commit}; use jj_lib::rewrite::{merge_commit_trees, rebase_commit};
use tracing::instrument; use tracing::instrument;
use crate::cli_util::{ use crate::cli_util::{short_commit_hash, CommandHelper, RevisionArg};
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper, RevisionArg,
};
use crate::command_error::{user_error, CommandError}; use crate::command_error::{user_error, CommandError};
use crate::description_util::join_message_paragraphs; use crate::description_util::join_message_paragraphs;
use crate::ui::Ui; use crate::ui::Ui;
@ -96,10 +94,10 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
)); ));
} }
let mut workspace_command = command.workspace_helper(ui)?; let mut workspace_command = command.workspace_helper(ui)?;
let target_commits = let target_commits = workspace_command
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.revisions)? .resolve_some_revsets_default_single(&args.revisions)?
.into_iter() .into_iter()
.collect_vec(); .collect_vec();
let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec(); let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec();
let mut tx = workspace_command.start_transaction(); let mut tx = workspace_command.start_transaction();
let mut num_rebased; let mut num_rebased;

View file

@ -30,8 +30,8 @@ use jj_lib::settings::UserSettings;
use tracing::instrument; use tracing::instrument;
use crate::cli_util::{ use crate::cli_util::{
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper, short_commit_hash, CommandHelper, RevisionArg, WorkspaceCommandHelper,
RevisionArg, WorkspaceCommandHelper, WorkspaceCommandTransaction, WorkspaceCommandTransaction,
}; };
use crate::command_error::{user_error, CommandError}; use crate::command_error::{user_error, CommandError};
use crate::formatter::Formatter; use crate::formatter::Formatter;
@ -193,10 +193,10 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
simplify_ancestor_merge: false, simplify_ancestor_merge: false,
}; };
let mut workspace_command = command.workspace_helper(ui)?; let mut workspace_command = command.workspace_helper(ui)?;
let new_parents = let new_parents = workspace_command
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.destination)? .resolve_some_revsets_default_single(&args.destination)?
.into_iter() .into_iter()
.collect_vec(); .collect_vec();
if let Some(rev_str) = &args.revision { if let Some(rev_str) = &args.revision {
assert_eq!( assert_eq!(
// In principle, `-r --skip-empty` could mean to abandon the `-r` // In principle, `-r --skip-empty` could mean to abandon the `-r`
@ -221,8 +221,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
rev_str, rev_str,
)?; )?;
} else if !args.source.is_empty() { } else if !args.source.is_empty() {
let source_commits = let source_commits = workspace_command.resolve_some_revsets_default_single(&args.source)?;
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.source)?;
rebase_descendants_transaction( rebase_descendants_transaction(
ui, ui,
command.settings(), command.settings(),
@ -235,7 +234,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
let branch_commits = if args.branch.is_empty() { let branch_commits = if args.branch.is_empty() {
IndexSet::from([workspace_command.resolve_single_rev("@")?]) IndexSet::from([workspace_command.resolve_single_rev("@")?])
} else { } else {
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.branch)? workspace_command.resolve_some_revsets_default_single(&args.branch)?
}; };
rebase_branch( rebase_branch(
ui, ui,

View file

@ -29,9 +29,8 @@ use jj_lib::workspace::Workspace;
use tracing::instrument; use tracing::instrument;
use crate::cli_util::{ use crate::cli_util::{
check_stale_working_copy, print_checkout_stats, check_stale_working_copy, print_checkout_stats, short_commit_hash, CommandHelper, RevisionArg,
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper, WorkingCopyFreshness, WorkspaceCommandHelper,
RevisionArg, WorkingCopyFreshness, WorkspaceCommandHelper,
}; };
use crate::command_error::{internal_error_with_message, user_error, CommandError}; use crate::command_error::{internal_error_with_message, user_error, CommandError};
use crate::ui::Ui; use crate::ui::Ui;
@ -203,7 +202,8 @@ fn cmd_workspace_add(
vec![tx.repo().store().root_commit()] vec![tx.repo().store().root_commit()]
} }
} else { } else {
resolve_multiple_nonempty_revsets_default_single(&old_workspace_command, &args.revision)? old_workspace_command
.resolve_some_revsets_default_single(&args.revision)?
.into_iter() .into_iter()
.collect_vec() .collect_vec()
}; };