From cb4873fc0f90bc22039ce714ecb68758885c1d45 Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Fri, 3 Feb 2023 21:33:17 -0800
Subject: [PATCH] cli_util: Make `resolve_base_revs` use IndexSet

This will make deduplication easier in the next commit.
The error message becomes slightly less informative, unfortunately.
---
 src/cli_util.rs           | 14 ++++++--------
 src/commands/mod.rs       |  8 ++++++--
 tests/test_new_command.rs |  2 +-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/cli_util.rs b/src/cli_util.rs
index 1fd7dc74e7..3b5db9946c 100644
--- a/src/cli_util.rs
+++ b/src/cli_util.rs
@@ -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;
@@ -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();
diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index 4575805770..7b30c75eac 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -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);
@@ -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 {
diff --git a/tests/test_new_command.rs b/tests/test_new_command.rs
index 596da8df2b..6a9e22afbb 100644
--- a/tests/test_new_command.rs
+++ b/tests/test_new_command.rs
@@ -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