Skip to content

Commit

Permalink
cli: make rebase --skip-empty keep already empty commits
Browse files Browse the repository at this point in the history
I think the user usually wants to abandon only newly empty commits. I
think they should use `jj abandon` if they want to get rid of already
empty commits. By keeping already empty commits, we don't need to
special-case the working copy and merge commits.
  • Loading branch information
martinvonz committed Feb 26, 2024
1 parent 3bc3a63 commit b695fa1
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions cli/src/commands/rebase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
43 changes: 43 additions & 0 deletions cli/tests/test_rebase_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
"###);
}

0 comments on commit b695fa1

Please sign in to comment.