absorb: abandon source commit if it becomes discardable

I don't think we need --keep-emptied flag. IIRC, "jj squash" has that flag in
order not to squash commit description to the destination commits. Since
"jj absorb" never moves commit description, the source commit is preserved in
that situation.

Closes #5141

CHANGELOG.md

@@ -37,6 +37,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * The deprecated `[alias]` config section is no longer respected. Move command
   aliases to the `[aliases]` section.
 
+* `jj absorb` now abandons the source commit if it becomes empty and has no
+  description.
+
 ### Deprecations
 
 * `--config-toml=TOML` is deprecated in favor of `--config=NAME=VALUE` and

cli/src/commands/absorb.rs

@@ -32,6 +32,9 @@ use crate::ui::Ui;
 /// last. If the destination revision cannot be determined unambiguously, the
 /// change will be left in the source revision.
 ///
+/// The source revision will be abandoned if all changes are absorbed into the
+/// destination revisions, and if the source revision has no description.
+///
 /// The modification made by `jj absorb` can be reviewed by `jj op show -p`.
 #[derive(clap::Args, Clone, Debug)]
 pub(crate) struct AbsorbArgs {

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

@@ -231,6 +231,8 @@ Move changes from a revision into the stack of mutable revisions
 
 This command splits changes in the source revision and moves each change to the closest mutable ancestor where the corresponding lines were modified last. If the destination revision cannot be determined unambiguously, the change will be left in the source revision.
 
+The source revision will be abandoned if all changes are absorbed into the destination revisions, and if the source revision has no description.
+
 The modification made by `jj absorb` can be reviewed by `jj op show -p`.
 
 **Usage:** `jj absorb [OPTIONS] [FILESETS]...`

cli/tests/test_absorb_command.rs

@@ -31,39 +31,41 @@ fn test_absorb_simple() {
     test_env.jj_cmd_ok(&repo_path, &["new", "-m2"]);
     std::fs::write(repo_path.join("file1"), "1a\n1b\n2a\n2b\n").unwrap();
 
-    // Insert first and last lines
+    // Empty commit
     test_env.jj_cmd_ok(&repo_path, &["new"]);
+    let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb"]);
+    insta::assert_snapshot!(stderr, @"Nothing changed.");
+
+    // Insert first and last lines
     std::fs::write(repo_path.join("file1"), "1X\n1a\n1b\n2a\n2b\n2Z\n").unwrap();
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb"]);
     insta::assert_snapshot!(stderr, @r"
     Absorbed changes into these revisions:
-      zsuskuln 3c319ca8 2
-      kkmpptxz 6b722c47 1
-    Rebased 1 descendant commits.
-    Working copy now at: mzvwutvl dfb52c7a (empty) (no description set)
-    Parent commit      : zsuskuln 3c319ca8 2
+      zsuskuln 3027ca7a 2
+      kkmpptxz d0f1e8dd 1
+    Working copy now at: yqosqzyt 277bed24 (empty) (no description set)
+    Parent commit      : zsuskuln 3027ca7a 2
     ");
 
     // Modify middle line in hunk
     std::fs::write(repo_path.join("file1"), "1X\n1A\n1b\n2a\n2b\n2Z\n").unwrap();
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb"]);
     insta::assert_snapshot!(stderr, @r"
     Absorbed changes into these revisions:
-      kkmpptxz 1027ba02 1
-    Rebased 2 descendant commits.
-    Working copy now at: mzvwutvl 7e01bf33 (empty) (no description set)
-    Parent commit      : zsuskuln 669278a7 2
+      kkmpptxz d366d92c 1
+    Rebased 1 descendant commits.
+    Working copy now at: vruxwmqv 32eb72fe (empty) (no description set)
+    Parent commit      : zsuskuln 5bf0bc06 2
     ");
 
     // Remove middle line from hunk
     std::fs::write(repo_path.join("file1"), "1X\n1A\n1b\n2a\n2Z\n").unwrap();
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb"]);
     insta::assert_snapshot!(stderr, @r"
     Absorbed changes into these revisions:
-      zsuskuln 660c7cac 2
-    Rebased 1 descendant commits.
-    Working copy now at: mzvwutvl 39c8fe11 (empty) (no description set)
-    Parent commit      : zsuskuln 660c7cac 2
+      zsuskuln 6e2c4777 2
+    Working copy now at: yostqsxw 4a48490c (empty) (no description set)
+    Parent commit      : zsuskuln 6e2c4777 2
     ");
 
     // Insert ambiguous line in between
@@ -72,7 +74,7 @@ fn test_absorb_simple() {
     insta::assert_snapshot!(stderr, @"Nothing changed.");
 
     insta::assert_snapshot!(get_diffs(&test_env, &repo_path, "mutable()"), @r"
-    @  mzvwutvl a0e56cbc (no description set)
+    @  yostqsxw 80965bcc (no description set)
     │  diff --git a/file1 b/file1
     │  index 8653ca354d..88eb438902 100644
     │  --- a/file1
@@ -84,7 +86,7 @@ fn test_absorb_simple() {
     │  +Y
     │   2a
     │   2Z
-    ○  zsuskuln 660c7cac 2
+    ○  zsuskuln 6e2c4777 2
     │  diff --git a/file1 b/file1
     │  index ed237b5112..8653ca354d 100644
     │  --- a/file1
@@ -95,7 +97,7 @@ fn test_absorb_simple() {
     │   1b
     │  +2a
     │  +2Z
-    ○  kkmpptxz 1027ba02 1
+    ○  kkmpptxz d366d92c 1
     │  diff --git a/file1 b/file1
     │  index e69de29bb2..ed237b5112 100644
     │  --- a/file1
@@ -110,28 +112,26 @@ fn test_absorb_simple() {
        index 0000000000..e69de29bb2
     ");
     insta::assert_snapshot!(get_evolog(&test_env, &repo_path, "description(1)"), @r"
-    ○    kkmpptxz 1027ba02 1
+    ○    kkmpptxz d366d92c 1
     ├─╮
-    │ ○  mzvwutvl hidden 4624004f (no description set)
-    │ ○  mzvwutvl hidden dfb52c7a (empty) (no description set)
-    ○ │  kkmpptxz hidden 6b722c47 1
+    │ ○  yqosqzyt hidden c506fbc7 (no description set)
+    │ ○  yqosqzyt hidden 277bed24 (empty) (no description set)
+    ○    kkmpptxz hidden d0f1e8dd 1
     ├─╮
-    │ ○  mzvwutvl hidden 2342dbe2 (no description set)
+    │ ○  mzvwutvl hidden 8935ee61 (no description set)
     │ ○  mzvwutvl hidden 2bc3d2ce (empty) (no description set)
     ○  kkmpptxz hidden ee76d790 1
     ○  kkmpptxz hidden 677e62d5 (empty) 1
     ");
     insta::assert_snapshot!(get_evolog(&test_env, &repo_path, "description(2)"), @r"
-    ○    zsuskuln 660c7cac 2
+    ○    zsuskuln 6e2c4777 2
     ├─╮
-    │ ○  mzvwutvl hidden cb78f902 (no description set)
-    │ ○  mzvwutvl hidden 7e01bf33 (empty) (no description set)
-    │ ○  mzvwutvl hidden 4624004f (no description set)
-    │ ○  mzvwutvl hidden dfb52c7a (empty) (no description set)
-    ○ │  zsuskuln hidden 669278a7 2
-    ○ │  zsuskuln hidden 3c319ca8 2
+    │ ○  vruxwmqv hidden 7b1da5cd (no description set)
+    │ ○  vruxwmqv hidden 32eb72fe (empty) (no description set)
+    ○  zsuskuln hidden 5bf0bc06 2
+    ○    zsuskuln hidden 3027ca7a 2
     ├─╮
-    │ ○  mzvwutvl hidden 2342dbe2 (no description set)
+    │ ○  mzvwutvl hidden 8935ee61 (no description set)
     │ ○  mzvwutvl hidden 2bc3d2ce (empty) (no description set)
     ○  zsuskuln hidden cca09b4d 2
     ○  zsuskuln hidden 7b092471 (empty) 2
@@ -156,11 +156,11 @@ fn test_absorb_replace_single_line_hunk() {
     test_env.jj_cmd_ok(&repo_path, &["new"]);
     std::fs::write(repo_path.join("file1"), "2a\n1A\n2b\n").unwrap();
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb"]);
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r"
     Absorbed changes into these revisions:
       qpvuntsm 7e885236 (conflict) 1
-    Rebased 2 descendant commits.
-    Working copy now at: zsuskuln e9c3b95b (empty) (no description set)
+    Rebased 1 descendant commits.
+    Working copy now at: mzvwutvl e9c3b95b (empty) (no description set)
     Parent commit      : kkmpptxz 7c36845c 2
     New conflicts appeared in these commits:
       qpvuntsm 7e885236 (conflict) 1
@@ -169,10 +169,10 @@ fn test_absorb_replace_single_line_hunk() {
     Then use `jj resolve`, or edit the conflict markers in the file directly.
     Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
     Then run `jj squash` to move the resolution into the conflicted commit.
-    "###);
+    ");
 
-    insta::assert_snapshot!(get_diffs(&test_env, &repo_path, "mutable()"), @r###"
-    @  zsuskuln e9c3b95b (empty) (no description set)
+    insta::assert_snapshot!(get_diffs(&test_env, &repo_path, "mutable()"), @r"
+    @  mzvwutvl e9c3b95b (empty) (no description set)
     ○  kkmpptxz 7c36845c 2
     │  diff --git a/file1 b/file1
     │  index 0000000000..2f87e8e465 100644
@@ -206,7 +206,7 @@ fn test_absorb_replace_single_line_hunk() {
        +1A
        +2b
        +>>>>>>> Conflict 1 of 1 ends
-    "###);
+    ");
 }
 
 #[test]
@@ -258,13 +258,12 @@ fn test_absorb_merge() {
     insta::assert_snapshot!(stderr, @r"
     Absorbed changes into these revisions:
       mzvwutvl e93c0210 3
-    Rebased 1 descendant commits.
-    Working copy now at: yqosqzyt 1b10dfa4 (empty) (no description set)
+    Working copy now at: vruxwmqv 1b10dfa4 (empty) (no description set)
     Parent commit      : mzvwutvl e93c0210 3
     ");
 
     insta::assert_snapshot!(get_diffs(&test_env, &repo_path, "mutable()"), @r"
-    @  yqosqzyt 1b10dfa4 (empty) (no description set)
+    @  vruxwmqv 1b10dfa4 (empty) (no description set)
     ○    mzvwutvl e93c0210 3
     ├─╮  diff --git a/file2 b/file2
     │ │  new file mode 100644
@@ -302,6 +301,85 @@ fn test_absorb_merge() {
     ");
 }
 
+#[test]
+fn test_absorb_discardable_merge_with_descendant() {
+    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");
+
+    test_env.jj_cmd_ok(&repo_path, &["describe", "-m0"]);
+    std::fs::write(repo_path.join("file1"), "0a\n").unwrap();
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m1"]);
+    std::fs::write(repo_path.join("file1"), "1a\n1b\n0a\n").unwrap();
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m2", "description(0)"]);
+    std::fs::write(repo_path.join("file1"), "0a\n2a\n2b\n").unwrap();
+
+    let (_stdout, stderr) =
+        test_env.jj_cmd_ok(&repo_path, &["new", "description(1)", "description(2)"]);
+    insta::assert_snapshot!(stderr, @r"
+    Working copy now at: mzvwutvl f59b2364 (empty) (no description set)
+    Parent commit      : kkmpptxz 7e9df299 1
+    Parent commit      : zsuskuln baf056cf 2
+    Added 0 files, modified 1 files, removed 0 files
+    ");
+
+    // Modify first and last lines in the merge commit
+    std::fs::write(repo_path.join("file1"), "1A\n1b\n0a\n2a\n2B\n").unwrap();
+    // Add new commit on top
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m3"]);
+    std::fs::write(repo_path.join("file2"), "3a\n").unwrap();
+    // Then absorb the merge commit
+    let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb", "--from=@-"]);
+    insta::assert_snapshot!(stderr, @r"
+    Absorbed changes into these revisions:
+      zsuskuln 02668cf6 2
+      kkmpptxz fcabe394 1
+    Rebased 1 descendant commits.
+    Working copy now at: royxmykx f04f1247 3
+    Parent commit      : kkmpptxz fcabe394 1
+    Parent commit      : zsuskuln 02668cf6 2
+    ");
+
+    insta::assert_snapshot!(get_diffs(&test_env, &repo_path, "mutable()"), @r"
+    @    royxmykx f04f1247 3
+    ├─╮  diff --git a/file2 b/file2
+    │ │  new file mode 100644
+    │ │  index 0000000000..31cd755d20
+    │ │  --- /dev/null
+    │ │  +++ b/file2
+    │ │  @@ -0,0 +1,1 @@
+    │ │  +3a
+    │ ○  zsuskuln 02668cf6 2
+    │ │  diff --git a/file1 b/file1
+    │ │  index eb6e8821f1..4907935b9f 100644
+    │ │  --- a/file1
+    │ │  +++ b/file1
+    │ │  @@ -1,1 +1,3 @@
+    │ │   0a
+    │ │  +2a
+    │ │  +2B
+    ○ │  kkmpptxz fcabe394 1
+    ├─╯  diff --git a/file1 b/file1
+    │    index eb6e8821f1..902dd8ef13 100644
+    │    --- a/file1
+    │    +++ b/file1
+    │    @@ -1,1 +1,3 @@
+    │    +1A
+    │    +1b
+    │     0a
+    ○  qpvuntsm 3777b700 0
+    │  diff --git a/file1 b/file1
+    ~  new file mode 100644
+       index 0000000000..eb6e8821f1
+       --- /dev/null
+       +++ b/file1
+       @@ -0,0 +1,1 @@
+       +0a
+    ");
+}
+
 #[test]
 fn test_absorb_conflict() {
     let test_env = TestEnvironment::default();
@@ -456,8 +534,7 @@ fn test_absorb_from_into() {
     ");
 
     // Absorb all lines from the working-copy parent. An empty commit won't be
-    // discarded because "absorb" isn't a command to squash revisions, but to
-    // move hunks.
+    // discarded because "absorb" isn't a command to squash commit descriptions.
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["absorb", "--from=@-"]);
     insta::assert_snapshot!(stderr, @r"
     Absorbed changes into these revisions:

lib/src/absorb.rs

@@ -260,7 +260,8 @@ fn combine_texts(text1: &[u8], text2: &[u8], selected_ranges: &[SelectedRange])
     .collect()
 }
 
-/// Merges selected trees into the specified commits.
+/// Merges selected trees into the specified commits. Abandons the source commit
+/// if it becomes discardable.
 pub fn absorb_hunks(
     repo: &mut MutableRepo,
     source: &AbsorbSource,
@@ -278,9 +279,13 @@ pub fn absorb_hunks(
         |rewriter| {
             // Remove selected hunks from the source commit by reparent()
             if rewriter.old_commit().id() == source.commit.id() {
-                // TODO: should we abandon the source if it's discardable?
-                rewriter.reparent(settings).write()?;
-                num_rebased += 1;
+                let commit_builder = rewriter.reparent(settings);
+                if commit_builder.is_discardable()? {
+                    commit_builder.abandon();
+                } else {
+                    commit_builder.write()?;
+                    num_rebased += 1;
+                }
                 return Ok(());
             }
             let Some(tree_builder) = selected_trees.remove(rewriter.old_commit().id()) else {