Make jj rebase --skip-empty keep already empty commits

CHANGELOG.md

@@ -28,6 +28,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * Dropped support for the deprecated `:` revset operator. Use `::` instead.
 
+* `jj rebase --skip-empty` no longer abandons commits that were already empty
+  before the rebase.
+
 ### New features
 
 * Added support for commit signing and verification. This comes with 

cli/src/commands/rebase.rs

@@ -155,9 +155,9 @@ pub(crate) struct RebaseArgs {
     destination: Vec<RevisionArg>,
 
     /// If true, when rebasing would produce an empty commit, the commit is
-    /// skipped.
-    /// Will never skip merge commits with multiple non-empty parents.
-    /// Will never skip the working commit.
+    /// 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 = "revision")]
     skip_empty: bool,
 
@@ -181,7 +181,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
 
     let rebase_options = RebaseOptions {
         empty: match args.skip_empty {
-            true => EmptyBehaviour::AbandonAllEmpty,
+            true => EmptyBehaviour::AbandonNewlyEmpty,
             false => EmptyBehaviour::Keep,
         },
         simplify_ancestor_merge: false,

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

@@ -1461,7 +1461,7 @@ J          J
 * `-s`, `--source <SOURCE>` — Rebase specified revision(s) together their tree of descendants (can be repeated)
 * `-r`, `--revision <REVISION>` — Rebase only this revision, rebasing descendants onto this revision's parent(s)
 * `-d`, `--destination <DESTINATION>` — The revision(s) to rebase onto (can be repeated to create a merge commit)
-* `--skip-empty` — If true, when rebasing would produce an empty commit, the commit is skipped. Will never skip merge commits with multiple non-empty parents. Will never skip the working commit
+* `--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
 
   Possible values: `true`, `false`
 

cli/tests/test_rebase_command.rs

@@ -1038,3 +1038,46 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
     ◉
     "###);
 }
+
+#[test]
+fn test_rebase_skip_empty() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    create_commit(&test_env, &repo_path, "a", &[]);
+    create_commit(&test_env, &repo_path, "b", &["a"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "a", "-m", "will become empty"]);
+    test_env.jj_cmd_ok(&repo_path, &["restore", "--from=b"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m", "already empty"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m", "also already empty"]);
+
+    // Test the setup
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["log", "-T", "description"]), @r###"
+    @  also already empty
+    ◉  already empty
+    ◉  will become empty
+    │ ◉  b
+    ├─╯
+    ◉  a
+    ◉
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=b", "--skip-empty"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 3 commits
+    Working copy now at: yostqsxw 6b74c840 (empty) also already empty
+    Parent commit      : vruxwmqv 48a31526 (empty) already empty
+    "###);
+
+    // The parent commit became empty and was dropped, but the already empty commits
+    // were kept
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["log", "-T", "description"]), @r###"
+    @  also already empty
+    ◉  already empty
+    ◉  b
+    ◉  a
+    ◉
+    "###);
+}