From b0c7d0a7e26209502e94ce074d8a05aeaaf48c94 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Tue, 3 Dec 2024 19:49:02 +0900
Subject: [PATCH] cli: duplicate: parse -rREV option properly

Since "jj duplicate" interface is now quite similar to "jj rebase", it's
annoying that "-r" isn't parsed properly.
---
 cli/src/commands/duplicate.rs       | 20 ++++++++++++--------
 cli/tests/cli-reference@.md.snap    |  4 +---
 cli/tests/test_duplicate_command.rs |  9 ++++++---
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/cli/src/commands/duplicate.rs b/cli/src/commands/duplicate.rs
index 3c7438716d..075d1b51f1 100644
--- a/cli/src/commands/duplicate.rs
+++ b/cli/src/commands/duplicate.rs
@@ -51,12 +51,11 @@ use crate::ui::Ui;
 /// specified commits.
 #[derive(clap::Args, Clone, Debug)]
 pub(crate) struct DuplicateArgs {
-    /// The revision(s) to duplicate
-    #[arg(default_value = "@", add = ArgValueCandidates::new(complete::all_revisions))]
-    revisions: Vec<RevisionArg>,
-    /// Ignored (but lets you pass `-r` for consistency with other commands)
-    #[arg(short = 'r', hide = true, action = clap::ArgAction::Count)]
-    unused_revision: u8,
+    /// The revision(s) to duplicate (default: @)
+    #[arg(value_name = "REVISIONS", add = ArgValueCandidates::new(complete::all_revisions))]
+    revisions_pos: Vec<RevisionArg>,
+    #[arg(short = 'r', hide = true)]
+    revisions_opt: Vec<RevisionArg>,
     /// The revision(s) to duplicate onto (can be repeated to create a merge
     /// commit)
     #[arg(long, short, add = ArgValueCandidates::new(complete::all_revisions))]
@@ -90,8 +89,13 @@ pub(crate) fn cmd_duplicate(
     args: &DuplicateArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
-    let to_duplicate: Vec<CommitId> = workspace_command
-        .parse_union_revsets(ui, &args.revisions)?
+    let to_duplicate: Vec<CommitId> =
+        if !args.revisions_pos.is_empty() || !args.revisions_opt.is_empty() {
+            workspace_command
+                .parse_union_revsets(ui, &[&*args.revisions_pos, &*args.revisions_opt].concat())?
+        } else {
+            workspace_command.parse_revset(ui, &RevisionArg::AT)?
+        }
         .evaluate_to_commit_ids()?
         .try_collect()?; // in reverse topological order
     if to_duplicate.is_empty() {
diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap
index 7c2d078112..b3083eb360 100644
--- a/cli/tests/cli-reference@.md.snap
+++ b/cli/tests/cli-reference@.md.snap
@@ -755,9 +755,7 @@ When any of the `--destination`, `--insert-after`, or `--insert-before` argument
 
 ###### **Arguments:**
 
-* `<REVISIONS>` — The revision(s) to duplicate
-
-  Default value: `@`
+* `<REVISIONS>` — The revision(s) to duplicate (default: @)
 
 ###### **Options:**
 
diff --git a/cli/tests/test_duplicate_command.rs b/cli/tests/test_duplicate_command.rs
index d1d66e68a9..819474034e 100644
--- a/cli/tests/test_duplicate_command.rs
+++ b/cli/tests/test_duplicate_command.rs
@@ -344,7 +344,8 @@ fn test_duplicate_destination() {
     test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
     // Duplicate multiple commits without a direct ancestry relationship onto a
     // single destination.
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["duplicate", "a1", "b", "-d", "c"]);
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&repo_path, &["duplicate", "-r=a1", "-r=b", "-d", "c"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r"
     Duplicated 9e85a474f005 as xlzxqlsl da0996fd a1
@@ -369,8 +370,10 @@ fn test_duplicate_destination() {
 
     // Duplicate multiple commits without a direct ancestry relationship onto
     // multiple destinations.
-    let (stdout, stderr) =
-        test_env.jj_cmd_ok(&repo_path, &["duplicate", "a1", "b", "-d", "c", "-d", "d"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["duplicate", "-r=a1", "b", "-d", "c", "-d", "d"],
+    );
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r"
     Duplicated 9e85a474f005 as oupztwtk 2f519daa a1