feat(rebase): Rename --skip-empty to --skip-emptied.

This is based on @martinvonz's comment in jj-vcs#3830 about the inconsistency between squash --keep-emptied and rebase --skip-empty.

cli/src/commands/rebase.rs

@@ -125,8 +125,8 @@ use crate::ui::Ui;
 /// commit. This is true in general; it is not specific to this command.
 #[derive(clap::Args, Clone, Debug)]
 #[command(verbatim_doc_comment)]
-#[command(group(ArgGroup::new("to_rebase").args(&["branch", "source", "revisions"])))]
-#[command(group(ArgGroup::new("target").args(&["destination", "insert_after", "insert_before"]).multiple(true).required(true)))]
+#[command(group(ArgGroup::new("to_rebase").args(& ["branch", "source", "revisions"])))]
+#[command(group(ArgGroup::new("target").args(& ["destination", "insert_after", "insert_before"]).multiple(true).required(true)))]
 pub(crate) struct RebaseArgs {
     /// Rebase the whole branch relative to destination's ancestors (can be
     /// repeated)
@@ -188,12 +188,16 @@ pub(crate) struct RebaseArgs {
     )]
     insert_before: Vec<RevisionArg>,
 
+    /// Deprecated. Use --skip-emptied instead.
+    #[arg(long, conflicts_with = "revisions")]
+    skip_empty: bool,
+
     /// If true, when rebasing would produce an empty commit, the commit is
     /// abandoned. It will not be abandoned if it was already empty before the
     /// rebase. Will never skip merge commits with multiple non-empty
     /// parents.
     #[arg(long, conflicts_with = "revisions")]
-    skip_empty: bool,
+    skip_emptied: bool,
 
     /// Deprecated. Please prefix the revset with `all:` instead.
     #[arg(long, short = 'L', hide = true)]
@@ -213,8 +217,14 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
         ));
     }
 
+    if args.skip_empty {
+        return Err(user_error(
+            "--skip-empty is deprecated, and has been renamed to --skip-emptied.",
+        ));
+    }
+
     let rebase_options = RebaseOptions {
-        empty: match args.skip_empty {
+        empty: match args.skip_emptied {
             true => EmptyBehaviour::AbandonNewlyEmpty,
             false => EmptyBehaviour::Keep,
         },
@@ -869,42 +879,42 @@ fn move_commits(
         .iter()
         .map(|(commit_id, commit)| {
             let new_parents =
-            // New child of the rebased target commits.
-            if let Some(new_child_parents) = new_children_parents.get(commit_id) {
-                new_child_parents.clone()
-            }
-            // Commits in the target set should persist only rebased parents from the target
-            // sets.
-            else if let Some(target_commit_parents) =
-                target_commits_internal_parents.get(commit_id)
-            {
-                // If the commit does not have any parents in the target set, it is one of the
-                // commits in the root set, and should be rebased onto the new destination.
-                if target_commit_parents.is_empty() {
-                    new_parent_ids.clone()
-                } else {
-                    target_commit_parents.clone()
+                // New child of the rebased target commits.
+                if let Some(new_child_parents) = new_children_parents.get(commit_id) {
+                    new_child_parents.clone()
                 }
-            }
-            // Commits outside the target set should have references to commits inside the set
-            // replaced.
-            else if commit
-                .parent_ids()
-                .iter()
-                .any(|id| target_commits_external_parents.contains_key(id))
-            {
-                let mut new_parents = vec![];
-                for parent in commit.parent_ids() {
-                    if let Some(parents) = target_commits_external_parents.get(parent) {
-                        new_parents.extend(parents.iter().cloned());
+                // Commits in the target set should persist only rebased parents from the target
+                // sets.
+                else if let Some(target_commit_parents) =
+                    target_commits_internal_parents.get(commit_id)
+                {
+                    // If the commit does not have any parents in the target set, it is one of the
+                    // commits in the root set, and should be rebased onto the new destination.
+                    if target_commit_parents.is_empty() {
+                        new_parent_ids.clone()
                     } else {
-                        new_parents.push(parent.clone());
+                        target_commit_parents.clone()
                     }
                 }
-                new_parents
-            } else {
-                commit.parent_ids().iter().cloned().collect_vec()
-            };
+                // Commits outside the target set should have references to commits inside the set
+                // replaced.
+                else if commit
+                    .parent_ids()
+                    .iter()
+                    .any(|id| target_commits_external_parents.contains_key(id))
+                {
+                    let mut new_parents = vec![];
+                    for parent in commit.parent_ids() {
+                        if let Some(parents) = target_commits_external_parents.get(parent) {
+                            new_parents.extend(parents.iter().cloned());
+                        } else {
+                            new_parents.push(parent.clone());
+                        }
+                    }
+                    new_parents
+                } else {
+                    commit.parent_ids().iter().cloned().collect_vec()
+                };
 
             (commit_id.clone(), new_parents)
         })

cli/tests/cli-reference@.md.snap

@@ -1540,7 +1540,8 @@ commit. This is true in general; it is not specific to this command.
 * `-B`, `--insert-before <INSERT_BEFORE>` — The revision(s) to insert before (can be repeated to create a merge commit)
 
    Only works with `-r`.
-* `--skip-empty` — If true, when rebasing would produce an empty commit, the commit is abandoned. It will not be abandoned if it was already empty before the rebase. Will never skip merge commits with multiple non-empty parents
+* `--skip-empty` — Deprecated. Use --skip-emptied instead
+* `--skip-emptied` — If true, when rebasing would produce an empty commit, the commit is abandoned. It will not be abandoned if it was already empty before the rebase. Will never skip merge commits with multiple non-empty parents
 
 
 

cli/tests/test_rebase_command.rs

@@ -2236,7 +2236,7 @@ fn test_rebase_revisions_after_before() {
 }
 
 #[test]
-fn test_rebase_skip_empty() {
+fn test_rebase_skip_emptied() {
     let test_env = TestEnvironment::default();
     test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
     let repo_path = test_env.env_root().join("repo");
@@ -2259,7 +2259,7 @@ fn test_rebase_skip_empty() {
     ◉
     "###);
 
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=b", "--skip-empty"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=b", "--skip-emptied"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Rebased 3 commits