Skip to content

Commit

Permalink
cli: move resolve_multiple_nonempty_revsets_default_single() to comma…
Browse files Browse the repository at this point in the history
…nd 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().
  • Loading branch information
yuja committed Apr 1, 2024
1 parent f11d429 commit 72b6b17
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 46 deletions.
53 changes: 29 additions & 24 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,34 @@ impl WorkspaceCommandHelper {
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).
pub fn resolve_revset(&self, revision_str: &str) -> Result<Vec<Commit>, CommandError> {
Ok(self
Expand All @@ -764,7 +792,7 @@ impl WorkspaceCommandHelper {
/// Resolve a revset any number of revisions (including 0), but require the
/// user to indicate if they allow multiple revisions by prefixing the
/// expression with `all:`.
pub fn resolve_revset_default_single(
fn resolve_revset_default_single(
&self,
revision_str: &str,
) -> Result<Vec<Commit>, CommandError> {
Expand Down Expand Up @@ -1589,29 +1617,6 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> {
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(
repo: &Arc<ReadonlyRepo>,
workspace: &mut Workspace,
Expand Down
12 changes: 5 additions & 7 deletions cli/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::rewrite::{merge_commit_trees, rebase_commit};
use tracing::instrument;

use crate::cli_util::{
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper, RevisionArg,
};
use crate::cli_util::{short_commit_hash, CommandHelper, RevisionArg};
use crate::command_error::{user_error, CommandError};
use crate::description_util::join_message_paragraphs;
use crate::ui::Ui;
Expand Down Expand Up @@ -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 target_commits =
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.revisions)?
.into_iter()
.collect_vec();
let target_commits = workspace_command
.resolve_some_revsets_default_single(&args.revisions)?
.into_iter()
.collect_vec();
let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec();
let mut tx = workspace_command.start_transaction();
let mut num_rebased;
Expand Down
18 changes: 7 additions & 11 deletions cli/src/commands/rebase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ use jj_lib::rewrite::{rebase_commit, rebase_commit_with_options, EmptyBehaviour,
use jj_lib::settings::UserSettings;
use tracing::instrument;

use crate::cli_util::{
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper,
RevisionArg, WorkspaceCommandHelper,
};
use crate::cli_util::{short_commit_hash, CommandHelper, RevisionArg, WorkspaceCommandHelper};
use crate::command_error::{user_error, CommandError};
use crate::ui::Ui;

Expand Down Expand Up @@ -191,10 +188,10 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
simplify_ancestor_merge: false,
};
let mut workspace_command = command.workspace_helper(ui)?;
let new_parents =
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.destination)?
.into_iter()
.collect_vec();
let new_parents = workspace_command
.resolve_some_revsets_default_single(&args.destination)?
.into_iter()
.collect_vec();
if let Some(rev_str) = &args.revision {
assert_eq!(
// In principle, `-r --skip-empty` could mean to abandon the `-r`
Expand All @@ -219,8 +216,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
rev_str,
)?;
} else if !args.source.is_empty() {
let source_commits =
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.source)?;
let source_commits = workspace_command.resolve_some_revsets_default_single(&args.source)?;
rebase_descendants(
ui,
command.settings(),
Expand All @@ -233,7 +229,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
let branch_commits = if args.branch.is_empty() {
IndexSet::from([workspace_command.resolve_single_rev("@")?])
} else {
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.branch)?
workspace_command.resolve_some_revsets_default_single(&args.branch)?
};
rebase_branch(
ui,
Expand Down
8 changes: 4 additions & 4 deletions cli/src/commands/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ use jj_lib::workspace::Workspace;
use tracing::instrument;

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

0 comments on commit 72b6b17

Please sign in to comment.