From 31ac0d7e1fefdbe9d61ceeecfbceff4a93f2a9dc Mon Sep 17 00:00:00 2001
From: Matt Stark <msta@google.com>
Date: Wed, 3 Jul 2024 11:54:36 +1000
Subject: [PATCH] feat(rebase): Rename --skip-empty to --skip-emptied.

This is based on @martinvonz's comment in #3830 about the inconsistency between squash --keep-emptied and rebase --skip-empty.
---
 CHANGELOG.md                     |  2 ++
 cli/src/commands/rebase.rs       | 16 +++++++++++++---
 cli/tests/cli-reference@.md.snap |  2 +-
 cli/tests/test_rebase_command.rs |  4 ++--
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f3d256d47..e63e68172 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   move` to ensure that the target branch already exists.
   [#3584](https://github.com/martinvonz/jj/issues/3584)
 
+* `jj rebase --skip-empty` has been renamed to `jj rebase --skip-emptied`
+
 ### Deprecations
 
 * Replacing `-l` shorthand for `--limit` with `-n` in `jj log`, `jj op log`
diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs
index 06d63d6ac..03fe19517 100644
--- a/cli/src/commands/rebase.rs
+++ b/cli/src/commands/rebase.rs
@@ -35,7 +35,7 @@ use crate::cli_util::{
     short_commit_hash, CommandHelper, RevisionArg, WorkspaceCommandHelper,
     WorkspaceCommandTransaction,
 };
-use crate::command_error::{user_error, CommandError};
+use crate::command_error::{cli_error, user_error, CommandError};
 use crate::ui::Ui;
 
 /// Move revisions to different parent(s)
@@ -188,12 +188,16 @@ pub(crate) struct RebaseArgs {
     )]
     insert_before: Vec<RevisionArg>,
 
+    /// Deprecated. Use --skip-emptied instead.
+    #[arg(long, conflicts_with = "revisions", hide = true)]
+    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,
 }
 
 #[instrument(skip_all)]
@@ -202,8 +206,14 @@ pub(crate) fn cmd_rebase(
     command: &CommandHelper,
     args: &RebaseArgs,
 ) -> Result<(), CommandError> {
+    if args.skip_empty {
+        return Err(cli_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,
         },
diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap
index 6d23499cc..f3427a208 100644
--- a/cli/tests/cli-reference@.md.snap
+++ b/cli/tests/cli-reference@.md.snap
@@ -1540,7 +1540,7 @@ 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-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
 
 
 
diff --git a/cli/tests/test_rebase_command.rs b/cli/tests/test_rebase_command.rs
index b7ae47fbc..8e4ca1c85 100644
--- a/cli/tests/test_rebase_command.rs
+++ b/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