From af3fcdc77f775b495e3a44415c4eb06903c20abc Mon Sep 17 00:00:00 2001 From: Matt Stark Date: Wed, 5 Jun 2024 09:34:50 +1000 Subject: [PATCH] Feat: Don't abandon empty commits on partial squash. 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 --- CHANGELOG.md | 3 +++ cli/src/commands/move.rs | 1 + cli/src/commands/squash.rs | 13 ++++++---- cli/tests/cli-reference@.md.snap | 6 ++++- cli/tests/test_squash_command.rs | 41 ++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2009f6a0c6..378fb91120 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/cli/src/commands/move.rs b/cli/src/commands/move.rs index 1bcea4670e..230c722a45 100644 --- a/cli/src/commands/move.rs +++ b/cli/src/commands/move.rs @@ -100,6 +100,7 @@ pub(crate) fn cmd_move( SquashedDescription::Combine, false, &args.paths, + false, )?; tx.finish(ui, tx_description)?; Ok(()) diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 58290fb8ff..6795884d98 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -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 @@ -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, + /// The source revision will not be abandoned + #[arg(long)] + keep_empty: bool, } #[instrument(skip_all)] @@ -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(()) @@ -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 @@ -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())?; @@ -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 diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index a2be897b00..824c207296 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -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. @@ -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 ` — Specify diff editor to be used (implies --interactive) +* `--keep-empty` — The source revision will not be abandoned + + Possible values: `true`, `false` + diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index da672d923a..f9175c5a11 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -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();