diff --git a/CHANGELOG.md b/CHANGELOG.md index 2009f6a0c6..6b5191e47d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### New features +* `jj squash` now accepts a `--keep-empty` option to keep empty commits. + This option is implied by `--interactive` and specifying paths. + * `jj branch list`/`tag list` now accept `-T`/`--template` option. The tag list prints commit summary along with the tag name by default. 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..5d8e40e212 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -37,13 +37,13 @@ use crate::ui::Ui; /// 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, `--interactive` is not set, no paths are provided, and `--keep-empty` +/// is not provided, after moving changes out, the original revision will be +/// abandoned. /// -/// 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. +/// If the original revision was abandoned 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. /// /// If a working-copy commit gets abandoned, it will be given a new, empty /// commit. This is true in general; it is not specific to this command. @@ -65,15 +65,20 @@ pub(crate) struct SquashArgs { /// description(s) of the source revision(s) #[arg(long, short, conflicts_with = "message_paragraphs")] use_destination_message: bool, - /// Interactively choose which parts to squash + /// Interactively choose which parts to squash. + /// Implies --keep-empty. #[arg(long, short)] interactive: bool, /// Specify diff editor to be used (implies --interactive) #[arg(long, value_name = "NAME")] tool: Option, - /// Move only changes to these paths (instead of all paths) + /// Move only changes to these paths (instead of all paths). + /// Implies --keep-empty. #[arg(conflicts_with_all = ["interactive", "tool"], value_hint = clap::ValueHint::AnyPath)] paths: Vec, + /// If true, then empty revisions will be kept. + #[arg(long)] + keep_empty: bool, } #[instrument(skip_all)] @@ -83,7 +88,6 @@ pub(crate) fn cmd_squash( args: &SquashArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let mut sources: Vec; let destination; if !args.from.is_empty() || args.into.is_some() { @@ -119,6 +123,15 @@ pub(crate) fn cmd_squash( .to_matcher(); let diff_selector = workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?; + + // If the user provided --interactive or files, the intent of the user was + // clearly to work on files rather than on commits, - it would be strange + // if the user wrote jj squash foo.rs and found that the branch pointer had + // moved to the parent, for example. + let keep_empty = args.keep_empty + || matches!(diff_selector, DiffSelector::Interactive(_)) + || !args.paths.is_empty(); + let mut tx = workspace_command.start_transaction(); let tx_description = format!("squash commits into {}", destination.id().hex()); move_diff( @@ -132,6 +145,7 @@ pub(crate) fn cmd_squash( SquashedDescription::from_args(args), args.revision.is_none() && args.from.is_empty() && args.into.is_none(), &args.paths, + keep_empty, )?; tx.finish(ui, tx_description)?; Ok(()) @@ -177,6 +191,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 +225,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 = selected_tree.id() == source_tree.id() && !keep_empty; 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..764bc93bf5 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1781,9 +1781,9 @@ 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, `--interactive` is not set, no paths are provided, and `--keep-empty` is not provided, after moving changes out, the original revision will be abandoned. -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. +If the original revision was abandoned 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. If a working-copy commit gets abandoned, it will be given a new, empty commit. This is true in general; it is not specific to this command. @@ -1791,7 +1791,7 @@ If a working-copy commit gets abandoned, it will be given a new, empty commit. T ###### **Arguments:** -* `` — Move only changes to these paths (instead of all paths) +* `` — Move only changes to these paths (instead of all paths). Implies --keep-empty ###### **Options:** @@ -1803,11 +1803,15 @@ If a working-copy commit gets abandoned, it will be given a new, empty commit. T Possible values: `true`, `false` -* `-i`, `--interactive` — Interactively choose which parts to squash +* `-i`, `--interactive` — Interactively choose which parts to squash. Implies --keep-empty Possible values: `true`, `false` * `--tool ` — Specify diff editor to be used (implies --interactive) +* `--keep-empty` — If true, then empty revisions will be kept + + Possible values: `true`, `false` + diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index da672d923a..427237d292 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -160,13 +160,14 @@ fn test_squash_partial() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "-r", "b", "-i"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Rebased 1 descendant commits - Working copy now at: mzvwutvl f03d5ce4 c | (no description set) - Parent commit : qpvuntsm c9f931cd a b | (no description set) + Rebased 2 descendant commits + Working copy now at: mzvwutvl 1fc0b9c1 c | (no description set) + Parent commit : kkmpptxz a54b88b5 b | (empty) (no description set) "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ f03d5ce4a973 c - ◉ c9f931cd78af a b + @ 1fc0b9c19783 c + ◉ a54b88b5e186 b + ◉ c9f931cd78af a ◉ 000000000000 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file1", "-r", "a"]); @@ -445,7 +446,8 @@ fn test_squash_from_to_partial() { "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" @ 71b69e433fbc d - │ ◉ 55171e33db26 b c + │ ◉ 9f21d0f664f3 c + │ ◉ 55171e33db26 b ├─╯ ◉ 3db0a2f5b535 a ◉ 000000000000 @@ -885,10 +887,8 @@ fn test_squash_from_multiple_partial_no_op() { "###); // Source commits that didn't match the paths are not rewritten - let (stdout, stderr) = test_env.jj_cmd_ok( - &repo_path, - &["squash", "--from=@-+ ~ @", "--into=@", "-m=d", "b"], - ); + let (stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["squash", "--from=@-+ ~ @", "--into=@", "b"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Working copy now at: mzvwutvl 9227d0d7 d @@ -897,6 +897,8 @@ fn test_squash_from_multiple_partial_no_op() { "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" @ 9227d0d780fa d + │ ◉ 8a4c9234c310 b + ├─╯ │ ◉ 5ad3ca4090a7 c ├─╯ ◉ 3df52ee1f8a9 a