rebase/new: make --allow-large-revset no longer also allow duplicates

The `--allow-large-revset` option for `jj rebase` and `jj new` is used
for allowing a single revset to resolve to more than one destination
commit. It also means that duplicate commits between individual
revsets are allowed (e.g. `jj rebase -d x -d 'x|y'`). I'm about to
replace the first meaning of the flag by a revset function. I don't
think it's worth keeping the flag only for the second meaning, so I'm
just removing the feature instead. We can add it back under a
different name (`--allow-duplicate-destinations`?) if people care
about it.

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * The minimum supported Rust version (MSRV) is now 1.71.0.
 
+* The `--allow-large-revsets` flag for `jj rebase` and `jj new` can no longer be
+  used for allowing duplicate destinations. Include the potential duplicates
+  in a single expression instead (e.g. `jj new --allow-large-revsets 'x|y'`).
+
 ### New features
 
 * `jj log` output is now topologically grouped.

src/cli_util.rs

@@ -1751,23 +1751,32 @@ pub fn resolve_multiple_nonempty_revsets_flag_guarded(
     revisions: &[RevisionArg],
     allow_plural_revsets: bool,
 ) -> Result<IndexSet<Commit>, CommandError> {
-    if allow_plural_revsets {
-        resolve_multiple_nonempty_revsets(revisions, workspace_command)
-    } else {
-        let mut commits = IndexSet::new();
-        for revision_str in revisions {
+    let mut all_commits = IndexSet::new();
+    for revision_str in revisions {
+        if allow_plural_revsets {
+            let commits = workspace_command.resolve_revset(revision_str)?;
+            workspace_command.check_non_empty(&commits)?;
+            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}"#,
+                    )));
+                }
+            }
+        } else {
             let commit = workspace_command
                 .resolve_single_rev(revision_str)
                 .map_err(append_large_revsets_hint_if_multiple_revisions)?;
             let commit_hash = short_commit_hash(commit.id());
-            if !commits.insert(commit) {
+            if !all_commits.insert(commit) {
                 return Err(user_error(format!(
                     r#"More than one revset resolved to revision {commit_hash}"#,
                 )));
             }
         }
-        Ok(commits)
     }
+    Ok(all_commits)
 }
 
 fn append_large_revsets_hint_if_multiple_revisions(err: CommandError) -> CommandError {

tests/test_rebase_command.rs

@@ -142,10 +142,9 @@ fn test_rebase_branch() {
     ◉
     "###);
 
-    // Same test but with more than one revision per argument and same revision
-    // repeated in more than one argument
+    // Same test but with more than one revision per argument
     test_env.jj_cmd_success(&repo_path, &["undo"]);
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-b=e|d", "-b=d", "-d=b"]);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-b=e|d", "-d=b"]);
     insta::assert_snapshot!(stderr, @r###"
     Error: Revset "e|d" resolved to more than one revision
     Hint: The revset "e|d" resolved to these revisions:
@@ -155,7 +154,7 @@ fn test_rebase_branch() {
     "###);
     let stdout = test_env.jj_cmd_success(
         &repo_path,
-        &["rebase", "-b=e|d", "-b=d", "-d=b", "--allow-large-revsets"],
+        &["rebase", "-b=e|d", "-d=b", "--allow-large-revsets"],
     );
     insta::assert_snapshot!(stdout, @r###"
     Rebased 2 commits
@@ -399,30 +398,8 @@ fn test_rebase_multiple_destinations() {
     Error: More than one revset resolved to revision d370aee184ba
     "###);
 
-    // Adding --allow-large-revsets suppresses this error in addition to the large
-    // revsets error.
-    let stdout = test_env.jj_cmd_success(
-        &repo_path,
-        &[
-            "rebase",
-            "-r",
-            "a",
-            "-d",
-            "b",
-            "-d",
-            "b",
-            "--allow-large-revsets",
-        ],
-    );
-    insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    ◉  a
-    ◉  b
-    │ @  c
-    ├─╯
-    ◉
-    "###);
-    let stdout = test_env.jj_cmd_success(
+    // Same error with --allow-large-revsets if there is overlap.
+    let stderr = test_env.jj_cmd_failure(
         &repo_path,
         &[
             "rebase",
@@ -435,14 +412,8 @@ fn test_rebase_multiple_destinations() {
             "--allow-large-revsets",
         ],
     );
-    insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    ◉    a
-    ├─╮
-    │ ◉  b
-    @ │  c
-    ├─╯
-    ◉
+    insta::assert_snapshot!(stderr, @r###"
+    Error: More than one revset resolved to revision d370aee184ba
     "###);
 
     let stderr =
@@ -536,10 +507,9 @@ fn test_rebase_with_descendants() {
     ◉
     "###);
 
-    // Same test as above, but with duplicate commits and multiple commits per
-    // argument
+    // Same test as above, but with multiple commits per argument
     test_env.jj_cmd_success(&repo_path, &["undo"]);
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-s=b|d", "-s=d", "-d=a"]);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-s=b|d", "-d=a"]);
     insta::assert_snapshot!(stderr, @r###"
     Error: Revset "b|d" resolved to more than one revision
     Hint: The revset "b|d" resolved to these revisions:
@@ -549,7 +519,7 @@ fn test_rebase_with_descendants() {
     "###);
     let stdout = test_env.jj_cmd_success(
         &repo_path,
-        &["rebase", "-s=b|d", "-s=d", "-d=a", "--allow-large-revsets"],
+        &["rebase", "-s=b|d", "-d=a", "--allow-large-revsets"],
     );
     insta::assert_snapshot!(stdout, @r###"
     Rebased 3 commits