Skip to content

Commit

Permalink
Feat: Don't abandon empty commits on partial squash.
Browse files Browse the repository at this point in the history
If the user wrote, for example, `jj squash foo.rs`, it'd be a little wierd if we then moved a branch pointer to the parent commit.

Fixes #3815
  • Loading branch information
matts1 committed Jun 11, 2024
1 parent 3050685 commit af3fcdc
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#3800](https://github.com/martinvonz/jj/issues/3800) and
[#3801](https://github.com/martinvonz/jj/issues/3801)).

*
* `jj squash` now accepts a `--keep-empty` option to keep the source commit.

### Fixed bugs

* Previously, `jj git push` only made sure that the branch is in the expected
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/move.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ pub(crate) fn cmd_move(
SquashedDescription::Combine,
false,
&args.paths,
false,
)?;
tx.finish(ui, tx_description)?;
Ok(())
Expand Down
13 changes: 9 additions & 4 deletions cli/src/commands/squash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ use crate::ui::Ui;
/// commit to the grandparent.
///
/// If, after moving changes out, the source revision is empty compared to its
/// parent(s), it will be abandoned. Without `--interactive`, the source
/// revision will always be empty.
/// parent(s), and `--keep-empty` is not set, it will be abandoned. Without
/// `--interactive` or paths, the source revision will always be empty.
///
/// If the source became empty and both the source and destination had a
/// non-empty description, you will be asked for the combined description. If
Expand Down Expand Up @@ -74,6 +74,9 @@ pub(crate) struct SquashArgs {
/// Move only changes to these paths (instead of all paths)
#[arg(conflicts_with_all = ["interactive", "tool"], value_hint = clap::ValueHint::AnyPath)]
paths: Vec<String>,
/// The source revision will not be abandoned
#[arg(long)]
keep_empty: bool,
}

#[instrument(skip_all)]
Expand Down Expand Up @@ -132,6 +135,7 @@ pub(crate) fn cmd_squash(
SquashedDescription::from_args(args),
args.revision.is_none() && args.from.is_empty() && args.into.is_none(),
&args.paths,
args.keep_empty,
)?;
tx.finish(ui, tx_description)?;
Ok(())
Expand All @@ -157,7 +161,7 @@ impl SquashedDescription {
if !args.message_paragraphs.is_empty() {
let desc = join_message_paragraphs(&args.message_paragraphs);
SquashedDescription::Exact(desc)
} else if args.use_destination_message {
} else if args.use_destination_message || args.keep_empty {
SquashedDescription::UseDestination
} else {
SquashedDescription::Combine
Expand All @@ -177,6 +181,7 @@ pub fn move_diff(
description: SquashedDescription,
no_rev_arg: bool,
path_arg: &[String],
keep_empty: bool,
) -> Result<(), CommandError> {
tx.base_workspace_helper()
.check_rewritable(sources.iter().chain(std::iter::once(destination)).ids())?;
Expand Down Expand Up @@ -210,7 +215,7 @@ from the source will be moved into the destination.
let selected_tree_id =
diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?;
let selected_tree = tx.repo().store().get_root_tree(&selected_tree_id)?;
let abandon = selected_tree.id() == source_tree.id();
let abandon = !keep_empty && selected_tree.id() == source_tree.id();
if !abandon && selected_tree_id == parent_tree.id() {
// Nothing selected from this commit. If it's abandoned (i.e. already empty), we
// still include it so `jj squash` can be used for abandoning an empty commit in
Expand Down
6 changes: 5 additions & 1 deletion cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -1781,7 +1781,7 @@ With the `-r` option, moves the changes from the specified revision to the paren
With the `--from` and/or `--into` options, moves changes from/to the given revisions. If either is left out, it defaults to the working-copy commit. For example, `jj squash --into @--` moves changes from the working-copy commit to the grandparent.
If, after moving changes out, the source revision is empty compared to its parent(s), it will be abandoned. Without `--interactive`, the source revision will always be empty.
If, after moving changes out, the source revision is empty compared to its parent(s), and `--keep-empty` is not set, it will be abandoned. Without `--interactive` or paths, the source revision will always be empty.
If the source became empty and both the source and destination had a non-empty description, you will be asked for the combined description. If either was empty, then the other one will be used.
Expand All @@ -1808,6 +1808,10 @@ If a working-copy commit gets abandoned, it will be given a new, empty commit. T
Possible values: `true`, `false`
* `--tool <NAME>` — Specify diff editor to be used (implies --interactive)
* `--keep-empty` — The source revision will not be abandoned
Possible values: `true`, `false`
Expand Down
41 changes: 41 additions & 0 deletions cli/tests/test_squash_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,47 @@ fn test_squash_partial() {
insta::assert_snapshot!(stdout, @"");
}

#[test]
fn test_squash_keep_empty() {
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, &["branch", "create", "a"]);
std::fs::write(repo_path.join("file1"), "a\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]);
std::fs::write(repo_path.join("file1"), "b\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "c"]);
std::fs::write(repo_path.join("file1"), "c\n").unwrap();
// Test the setup
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 90fe0a96fc90 c
◉ fa5efbdf533c b
◉ 90aeefd03044 a
◉ 000000000000
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "-r", "b", "--keep-empty"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 2 descendant commits
Working copy now at: mzvwutvl 8d072ade c | (no description set)
Parent commit : kkmpptxz 1ffda862 b | (empty) (no description set)
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 8d072adecae0 c
◉ 1ffda862e07e b
◉ 297fde616b71 a
◉ 000000000000
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file1", "-r", "a"]);
insta::assert_snapshot!(stdout, @r###"
b
"###);
}

#[test]
fn test_squash_from_to() {
let test_env = TestEnvironment::default();
Expand Down

0 comments on commit af3fcdc

Please sign in to comment.