Skip to content

Commit

Permalink
cli_util: Make resolve_base_revs use IndexSet
Browse files Browse the repository at this point in the history
This will make deduplication easier in the next commit.
The error message becomes slightly less informative, unfortunately.
  • Loading branch information
ilyagr committed Feb 4, 2023
1 parent 241e185 commit cb4873f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 11 deletions.
14 changes: 6 additions & 8 deletions src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use std::sync::Arc;
use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory};
use clap::{Arg, ArgAction, ArgMatches, Command, FromArgMatches};
use git2::{Oid, Repository};
use indexmap::IndexSet;
use itertools::Itertools;
use jujutsu_lib::backend::{BackendError, ChangeId, CommitId, ObjectId, TreeId};
use jujutsu_lib::commit::Commit;
Expand Down Expand Up @@ -1385,19 +1386,16 @@ fn load_revset_aliases(
pub fn resolve_base_revs(
workspace_command: &WorkspaceCommandHelper,
revisions: &[RevisionArg],
) -> Result<Vec<Commit>, CommandError> {
let mut commits = vec![];
) -> Result<IndexSet<Commit>, CommandError> {
let mut commits = IndexSet::new();
for revision_str in revisions {
let commit = workspace_command.resolve_single_rev(revision_str)?;
if let Some(i) = commits.iter().position(|c| c == &commit) {
let commit_hash = short_commit_hash(commit.id());
if !commits.insert(commit) {
return Err(user_error(format!(
r#"Revset "{}" and "{}" resolved to the same revision {}"#,
&revisions[i].0,
&revision_str.0,
short_commit_hash(commit.id()),
r#"More than one revset resolved to revision {commit_hash}"#,
)));
}
commits.push(commit);
}

let root_commit_id = workspace_command.repo().store().root_commit_id();
Expand Down
8 changes: 6 additions & 2 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1994,7 +1994,9 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
!args.revisions.is_empty(),
"expected a non-empty list from clap"
);
let commits = resolve_base_revs(&workspace_command, &args.revisions)?;
let commits = resolve_base_revs(&workspace_command, &args.revisions)?
.into_iter()
.collect_vec();
let parent_ids = commits.iter().map(|c| c.id().clone()).collect();
let mut tx = workspace_command.start_transaction("new empty commit");
let merged_tree = merge_commit_trees(tx.base_repo().as_repo_ref(), &commits);
Expand Down Expand Up @@ -2683,7 +2685,9 @@ fn cmd_merge(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(),

fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let new_parents = resolve_base_revs(&workspace_command, &args.destination)?;
let new_parents = resolve_base_revs(&workspace_command, &args.destination)?
.into_iter()
.collect_vec();
if let Some(rev_str) = &args.revision {
rebase_revision(ui, command, &mut workspace_command, &new_parents, rev_str)?;
} else if let Some(source_str) = &args.source {
Expand Down
2 changes: 1 addition & 1 deletion tests/test_new_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ fn test_new_merge() {
// merge with non-unique revisions
let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "@", "200e"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revset "@" and "200e" resolved to the same revision 200ed1a14c8a
Error: More than one revset resolved to revision 200ed1a14c8a
"###);

// merge with root
Expand Down

0 comments on commit cb4873f

Please sign in to comment.