Skip to content

Commit

Permalink
cli: move resolve_destination_revs to cli_utils and rename
Browse files Browse the repository at this point in the history
Summary: This is currently used by `new.rs`, `workspace.rs`, and `rebase.rs`,
and may be useful for other commands and custom CLIs. So just go ahead and move
it into the parent module hierarchy.

Also rename the function to `resolve_all_revs`, as it isn't actually specific to
rebase at all.

Signed-off-by: Austin Seipp <[email protected]>
Change-Id: I0ea12afd8107f95a37a91340820221a0
  • Loading branch information
thoughtpolice committed Nov 4, 2023
1 parent e1193db commit 17bcac6
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 30 deletions.
17 changes: 17 additions & 0 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1907,6 +1907,23 @@ fn resolve_single_op(
Ok(operation)
}

/// Resolves revsets into revisions for use; useful for rebases or operations
/// that take multiple parents.
pub fn resolve_all_revs(
workspace_command: &WorkspaceCommandHelper,
ui: &mut Ui,
revisions: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let commits =
resolve_multiple_nonempty_revsets_default_single(workspace_command, ui, revisions)?;
let root_commit_id = workspace_command.repo().store().root_commit_id();
if commits.len() >= 2 && commits.iter().any(|c| c.id() == root_commit_id) {
Err(user_error("Cannot merge with root revision"))
} else {
Ok(commits)
}
}

fn find_all_operations(
op_store: &Arc<dyn OpStore>,
op_heads_store: &Arc<dyn OpHeadsStore>,
Expand Down
3 changes: 1 addition & 2 deletions cli/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use tracing::instrument;
use crate::cli_util::{
self, short_commit_hash, user_error, CommandError, CommandHelper, RevisionArg,
};
use crate::commands::rebase::resolve_destination_revs;
use crate::ui::Ui;

/// Create a new, empty change and edit it in the working copy
Expand Down Expand Up @@ -76,7 +75,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
!args.revisions.is_empty(),
"expected a non-empty list from clap"
);
let target_commits = resolve_destination_revs(&workspace_command, ui, &args.revisions)?
let target_commits = cli_util::resolve_all_revs(&workspace_command, ui, &args.revisions)?
.into_iter()
.collect_vec();
let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec();
Expand Down
23 changes: 3 additions & 20 deletions cli/src/commands/rebase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use jj_lib::settings::UserSettings;
use tracing::instrument;

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

Expand Down Expand Up @@ -161,7 +161,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
));
}
let mut workspace_command = command.workspace_helper(ui)?;
let new_parents = resolve_destination_revs(&workspace_command, ui, &args.destination)?
let new_parents = cli_util::resolve_all_revs(&workspace_command, ui, &args.destination)?
.into_iter()
.collect_vec();
if let Some(rev_str) = &args.revision {
Expand Down Expand Up @@ -390,20 +390,3 @@ fn check_rebase_destinations(
}
Ok(())
}

/// Resolves revsets into revisions to rebase onto. These revisions don't have
/// to be rewriteable.
pub(crate) fn resolve_destination_revs(
workspace_command: &WorkspaceCommandHelper,
ui: &mut Ui,
revisions: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let commits =
resolve_multiple_nonempty_revsets_default_single(workspace_command, ui, revisions)?;
let root_commit_id = workspace_command.repo().store().root_commit_id();
if commits.len() >= 2 && commits.iter().any(|c| c.id() == root_commit_id) {
Err(user_error("Cannot merge with root revision"))
} else {
Ok(commits)
}
}
12 changes: 4 additions & 8 deletions cli/src/commands/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use jj_lib::workspace::{default_working_copy_initializer, Workspace};
use tracing::instrument;

use crate::cli_util::{
check_stale_working_copy, print_checkout_stats, user_error, CommandError, CommandHelper,
self, check_stale_working_copy, print_checkout_stats, user_error, CommandError, CommandHelper,
RevisionArg, WorkspaceCommandHelper,
};
use crate::ui::Ui;
Expand Down Expand Up @@ -192,13 +192,9 @@ fn cmd_workspace_add(
vec![tx.repo().store().root_commit()]
}
} else {
crate::commands::rebase::resolve_destination_revs(
&old_workspace_command,
ui,
&args.revision,
)?
.into_iter()
.collect_vec()
cli_util::resolve_all_revs(&old_workspace_command, ui, &args.revision)?
.into_iter()
.collect_vec()
};

let tree = merge_commit_trees(tx.repo(), &parents)?;
Expand Down

1 comment on commit 17bcac6

@ilyagr
Copy link
Contributor

@ilyagr ilyagr commented on 17bcac6 Nov 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! I've been staring at this function and wondering what to do about it, I think this is a good way to deal with it.

Please sign in to comment.