squash: add -A/-B/-d options

For example, with a commit tree like this one:

@  kkrvspxk (empty) (no description set)
○  lpllnppl file4
│  A file4
○  xynqvmyt file3
│  A file3
○  wkrztqwl file2
│  A file2
○  mzuowqnz file1
│  A file1
♦  zzzzzzzz root()

we can jj squash -f x:: -d m to squash xynqvmyt and all its descendant
as a new commit onto mzuowqnz.

@  youptqqn (empty) (no description set)
○  wkrztqwl file2
│  A file2
│ ○  vsonsouy file3 file4
├─╯  A file3
│    A file4
○  mzuowqnz file1
│  A file1
♦  zzzzzzzz root()

On the implementation side, I've chosen to prepare the destination
commit before hand, and keep the squash algorithm mostly unmodified.

Author: glehmann

CHANGELOG.md

@@ -74,6 +74,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   output using template expressions, similar to `jj op log`. Also added
   `--no-op-diff` flag to suppress the operation diff.
 
+* `jj squash` has gained `--insert-before`, `--insert-after`, and `--destination`
+  options.
+
 ### Fixed bugs
 
 * `jj git clone` now correctly fetches all tags, unless `--fetch-tags` is

cli/src/commands/squash.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::collections::HashMap;
+
 use clap_complete::ArgValueCandidates;
 use clap_complete::ArgValueCompleter;
 use indoc::formatdoc;
@@ -23,12 +25,15 @@ use jj_lib::object_id::ObjectId as _;
 use jj_lib::repo::Repo as _;
 use jj_lib::rewrite;
 use jj_lib::rewrite::CommitWithSelection;
+use jj_lib::rewrite::merge_commit_trees;
+use pollster::FutureExt as _;
 use tracing::instrument;
 
 use crate::cli_util::CommandHelper;
 use crate::cli_util::DiffSelector;
 use crate::cli_util::RevisionArg;
 use crate::cli_util::WorkspaceCommandTransaction;
+use crate::cli_util::compute_commit_location;
 use crate::command_error::CommandError;
 use crate::command_error::user_error;
 use crate::command_error::user_error_with_hint;
@@ -72,6 +77,7 @@ pub(crate) struct SquashArgs {
         add = ArgValueCompleter::new(complete::revset_expression_mutable),
     )]
     revision: Option<RevisionArg>,
+
     /// Revision(s) to squash from (default: @)
     #[arg(
         long, short,
@@ -80,6 +86,7 @@ pub(crate) struct SquashArgs {
         add = ArgValueCompleter::new(complete::revset_expression_mutable),
     )]
     from: Vec<RevisionArg>,
+
     /// Revision to squash into (default: @)
     #[arg(
         long, short = 't',
@@ -89,30 +96,76 @@ pub(crate) struct SquashArgs {
         add = ArgValueCompleter::new(complete::revset_expression_mutable),
     )]
     into: Option<RevisionArg>,
+
+    /// The revision(s) to use as parent for the new commit (can be repeated
+    /// to create a merge commit)
+    #[arg(
+        long,
+        short,
+        conflicts_with = "into",
+        conflicts_with = "revision",
+        value_name = "REVSETS",
+        add = ArgValueCompleter::new(complete::revset_expression_all),
+    )]
+    destination: Option<Vec<RevisionArg>>,
+
+    /// The revision(s) to insert the new commit after (can be repeated to
+    /// create a merge commit)
+    #[arg(
+        long,
+        short = 'A',
+        visible_alias = "after",
+        conflicts_with = "destination",
+        conflicts_with = "into",
+        conflicts_with = "revision",
+        value_name = "REVSETS",
+        add = ArgValueCompleter::new(complete::revset_expression_all),
+    )]
+    insert_after: Option<Vec<RevisionArg>>,
+
+    /// The revision(s) to insert the new commit before (can be repeated to
+    /// create a merge commit)
+    #[arg(
+        long,
+        short = 'B',
+        visible_alias = "before",
+        conflicts_with = "destination",
+        conflicts_with = "into",
+        conflicts_with = "revision",
+        value_name = "REVSETS",
+        add = ArgValueCompleter::new(complete::revset_expression_mutable),
+    )]
+    insert_before: Option<Vec<RevisionArg>>,
+
     /// The description to use for squashed revision (don't open editor)
     #[arg(long = "message", short, value_name = "MESSAGE")]
     message_paragraphs: Vec<String>,
+
     /// Use the description of the destination revision and discard the
     /// description(s) of the source revision(s)
     #[arg(long, short, conflicts_with = "message_paragraphs")]
     use_destination_message: bool,
+
     /// Interactively choose which parts to squash
     #[arg(long, short)]
     interactive: bool,
+
     /// Specify diff editor to be used (implies --interactive)
     #[arg(
         long,
         value_name = "NAME",
         add = ArgValueCandidates::new(complete::diff_editors),
     )]
     tool: Option<String>,
+
     /// Move only changes to these paths (instead of all paths)
     #[arg(
         value_name = "FILESETS",
         value_hint = clap::ValueHint::AnyPath,
         add = ArgValueCompleter::new(complete::squash_revision_files),
     )]
     paths: Vec<String>,
+
     /// The source revision will not be abandoned
     #[arg(long, short)]
     keep_emptied: bool,
@@ -124,22 +177,31 @@ pub(crate) fn cmd_squash(
     command: &CommandHelper,
     args: &SquashArgs,
 ) -> Result<(), CommandError> {
+    let insert_destination_commit =
+        args.destination.is_some() || args.insert_after.is_some() || args.insert_before.is_some();
+
     let mut workspace_command = command.workspace_helper(ui)?;
 
     let mut sources: Vec<Commit>;
-    let destination;
-    if !args.from.is_empty() || args.into.is_some() {
+    let pre_existing_destination;
+
+    if !args.from.is_empty() || args.into.is_some() || insert_destination_commit {
         sources = if args.from.is_empty() {
             workspace_command.parse_revset(ui, &RevisionArg::AT)?
         } else {
             workspace_command.parse_union_revsets(ui, &args.from)?
         }
         .evaluate_to_commits()?
         .try_collect()?;
-        destination = workspace_command
-            .resolve_single_rev(ui, args.into.as_ref().unwrap_or(&RevisionArg::AT))?;
-        // remove the destination from the sources
-        sources.retain(|source| source.id() != destination.id());
+        if insert_destination_commit {
+            pre_existing_destination = None;
+        } else {
+            let destination = workspace_command
+                .resolve_single_rev(ui, args.into.as_ref().unwrap_or(&RevisionArg::AT))?;
+            // remove the destination from the sources
+            sources.retain(|source| source.id() != destination.id());
+            pre_existing_destination = Some(destination);
+        }
         // Reverse the set so we apply the oldest commits first. It shouldn't affect the
         // result, but it avoids creating transient conflicts and is therefore probably
         // a little faster.
@@ -155,21 +217,83 @@ pub(crate) fn cmd_squash(
             ));
         }
         sources = vec![source];
-        destination = parents.pop().unwrap();
-    }
+        pre_existing_destination = Some(parents.pop().unwrap());
+    };
 
-    let matcher = workspace_command
+    workspace_command.check_rewritable(sources.iter().chain(&pre_existing_destination).ids())?;
+
+    // prepare the tx description before possibly rebasing the source commits
+    let source_ids: Vec<_> = sources.iter().ids().collect();
+    let tx_description = if let Some(destination) = &pre_existing_destination {
+        format!("squash commits into {}", destination.id().hex())
+    } else {
+        match &source_ids[..] {
+            [] => format!("squash {} commits", source_ids.len()),
+            [id] => format!("squash commit {}", id.hex()),
+            [first, others @ ..] => {
+                format!("squash commit {} and {} more", first.hex(), others.len())
+            }
+        }
+    };
+
+    let mut tx = workspace_command.start_transaction();
+    let mut num_rebased = 0;
+    let destination = if let Some(commit) = pre_existing_destination {
+        commit
+    } else {
+        // create the new destination commit
+        let (parent_ids, child_ids) = compute_commit_location(
+            ui,
+            tx.base_workspace_helper(),
+            args.destination.as_deref(),
+            args.insert_after.as_deref(),
+            args.insert_before.as_deref(),
+            "squashed commit",
+        )?;
+        let parent_commits: Vec<_> = parent_ids
+            .iter()
+            .map(|commit_id| {
+                tx.base_workspace_helper()
+                    .repo()
+                    .store()
+                    .get_commit(commit_id)
+            })
+            .try_collect()?;
+        let merged_tree = merge_commit_trees(tx.repo(), &parent_commits).block_on()?;
+        let commit = tx
+            .repo_mut()
+            .new_commit(parent_ids.clone(), merged_tree.id())
+            .write()?;
+        let mut rewritten = HashMap::new();
+        tx.repo_mut()
+            .transform_descendants(child_ids, async |mut rewriter| {
+                let old_commit_id = rewriter.old_commit().id().clone();
+                for parent_id in &parent_ids {
+                    rewriter.replace_parent(parent_id, [commit.id()]);
+                }
+                let new_commit = rewriter.rebase().await?.write()?;
+                rewritten.insert(old_commit_id, new_commit);
+                num_rebased += 1;
+                Ok(())
+            })?;
+        for source in &mut *sources {
+            if let Some(rewritten_source) = rewritten.remove(source.id()) {
+                *source = rewritten_source;
+            }
+        }
+        commit
+    };
+
+    let matcher = tx
+        .base_workspace_helper()
         .parse_file_patterns(ui, &args.paths)?
         .to_matcher();
     let diff_selector =
-        workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
-    let text_editor = workspace_command.text_editor()?;
+        tx.base_workspace_helper()
+            .diff_selector(ui, args.tool.as_deref(), args.interactive)?;
+    let text_editor = tx.base_workspace_helper().text_editor()?;
     let description = SquashedDescription::from_args(args);
-    workspace_command
-        .check_rewritable(sources.iter().chain(std::iter::once(&destination)).ids())?;
 
-    let mut tx = workspace_command.start_transaction();
-    let tx_description = format!("squash commits into {}", destination.id().hex());
     let source_commits = select_diff(&tx, &sources, &destination, &matcher, &diff_selector)?;
     if let Some(squashed) = rewrite::squash_commits(
         tx.repo_mut(),
@@ -209,7 +333,7 @@ pub(crate) fn cmd_squash(
                         ui,
                         &tx,
                         abandoned_commits,
-                        &destination,
+                        (!insert_destination_commit).then_some(&destination),
                         &commit_builder,
                     )?;
                     // It's weird that commit.description() contains "JJ: " lines, but works.
@@ -222,12 +346,45 @@ pub(crate) fn cmd_squash(
             }
         };
         commit_builder.set_description(new_description);
-        commit_builder.write(tx.repo_mut())?;
+        if insert_destination_commit {
+            // forget about the intermediate commit
+            commit_builder.set_predecessors(
+                commit_builder
+                    .predecessors()
+                    .iter()
+                    .filter(|p| p != &destination.id())
+                    .cloned()
+                    .collect(),
+            );
+        }
+        let commit = commit_builder.write(tx.repo_mut())?;
+        let num_rebased = tx.repo_mut().rebase_descendants()?;
+        if let Some(mut formatter) = ui.status_formatter() {
+            if insert_destination_commit {
+                write!(formatter, "Created new commit ")?;
+                tx.write_commit_summary(formatter.as_mut(), &commit)?;
+                writeln!(formatter)?;
+            }
+            if num_rebased > 0 {
+                writeln!(formatter, "Rebased {num_rebased} descendant commits")?;
+            }
+        }
     } else {
         if diff_selector.is_interactive() {
             return Err(user_error("No changes selected"));
         }
 
+        if let Some(mut formatter) = ui.status_formatter() {
+            if insert_destination_commit {
+                write!(formatter, "Created new commit ")?;
+                tx.write_commit_summary(formatter.as_mut(), &destination)?;
+                writeln!(formatter)?;
+            }
+            if num_rebased > 0 {
+                writeln!(formatter, "Rebased {num_rebased} descendant commits")?;
+            }
+        }
+
         if let [only_path] = &*args.paths {
             let no_rev_arg = args.revision.is_none() && args.from.is_empty() && args.into.is_none();
             if no_rev_arg

cli/src/description_util.rs

@@ -323,12 +323,14 @@ pub fn combine_messages_for_editing(
     ui: &Ui,
     tx: &WorkspaceCommandTransaction,
     sources: &[Commit],
-    destination: &Commit,
+    destination: Option<&Commit>,
     commit_builder: &DetachedCommitBuilder,
 ) -> Result<String, CommandError> {
     let mut combined = String::new();
-    combined.push_str("JJ: Description from the destination commit:\n");
-    combined.push_str(destination.description());
+    if let Some(destination) = destination {
+        combined.push_str("JJ: Description from the destination commit:\n");
+        combined.push_str(destination.description());
+    }
     for commit in sources {
         combined.push_str("\nJJ: Description from source commit:\n");
         combined.push_str(commit.description());
@@ -338,7 +340,7 @@ pub fn combine_messages_for_editing(
         // show the user only trailers that were not in one of the squashed commits
         let old_trailers: Vec<_> = sources
             .iter()
-            .chain(std::iter::once(destination))
+            .chain(destination)
             .flat_map(|commit| parse_description_trailers(commit.description()))
             .collect();
         let commit = commit_builder.write_hidden()?;

cli/tests/[email protected]

@@ -2588,6 +2588,9 @@ If a working-copy commit gets abandoned, it will be given a new, empty commit. T
 * `-r`, `--revision <REVSET>` — Revision to squash into its parent (default: @)
 * `-f`, `--from <REVSETS>` — Revision(s) to squash from (default: @)
 * `-t`, `--into <REVSET>` [alias: `to`] — Revision to squash into (default: @)
+* `-d`, `--destination <REVSETS>` — The revision(s) to use as parent for the new commit (can be repeated to create a merge commit)
+* `-A`, `--insert-after <REVSETS>` [alias: `after`] — The revision(s) to insert the new commit after (can be repeated to create a merge commit)
+* `-B`, `--insert-before <REVSETS>` [alias: `before`] — The revision(s) to insert the new commit before (can be repeated to create a merge commit)
 * `-m`, `--message <MESSAGE>` — The description to use for squashed revision (don't open editor)
 * `-u`, `--use-destination-message` — Use the description of the destination revision and discard the description(s) of the source revision(s)
 * `-i`, `--interactive` — Interactively choose which parts to squash

cli/tests/test_immutable_commits.rs

@@ -538,6 +538,34 @@ fn test_rewrite_immutable_commands() {
     [EOF]
     [exit status: 1]
     "#);
+    // squash --after
+    let output = work_dir.run_jj(["squash", "--after=main-"]);
+    insta::assert_snapshot!(output, @r#"
+    ------- stderr -------
+    Error: Commit 4397373a0991 is immutable
+    Hint: Could not modify commit: mzvwutvl 4397373a main | (conflict) merge
+    Hint: Immutable commits are used to protect shared history.
+    Hint: For more information, see:
+          - https://jj-vcs.github.io/jj/latest/config/#set-of-immutable-commits
+          - `jj help -k config`, "Set of immutable commits"
+    Hint: This operation would rewrite 1 immutable commits.
+    [EOF]
+    [exit status: 1]
+    "#);
+    // squash --before
+    let output = work_dir.run_jj(["squash", "--before=main"]);
+    insta::assert_snapshot!(output, @r#"
+    ------- stderr -------
+    Error: Commit 4397373a0991 is immutable
+    Hint: Could not modify commit: mzvwutvl 4397373a main | (conflict) merge
+    Hint: Immutable commits are used to protect shared history.
+    Hint: For more information, see:
+          - https://jj-vcs.github.io/jj/latest/config/#set-of-immutable-commits
+          - `jj help -k config`, "Set of immutable commits"
+    Hint: This operation would rewrite 1 immutable commits.
+    [EOF]
+    [exit status: 1]
+    "#);
     // sign
     let output = work_dir.run_jj(["sign", "-r=main", "--config=signing.backend=test"]);
     insta::assert_snapshot!(output, @r#"