Make jj squash --from accept multiple commits

CHANGELOG.md

@@ -24,7 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   elided.
 
 * `jj squash` now accepts `--from` and `--into` (mutually exclusive with `-r`).
-  It can thereby be for all use cases where `jj move` can be used.
+  It can thereby be for all use cases where `jj move` can be used. The `--from`
+  argument accepts a revset that resolves to move than one revision.
 
 ### Fixed bugs
 

cli/src/commands/move.rs

@@ -89,8 +89,8 @@ pub(crate) fn cmd_move(
         ui,
         &mut tx,
         command.settings(),
-        source,
-        destination,
+        &[source],
+        &destination,
         matcher.as_ref(),
         &diff_selector,
         None,

cli/src/commands/squash.rs

@@ -80,36 +80,42 @@ pub(crate) fn cmd_squash(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
 
-    let source;
+    let mut sources;
     let destination;
     if args.from.is_some() || args.into.is_some() {
-        source = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?;
+        sources = workspace_command.resolve_revset(args.from.as_deref().unwrap_or("@"))?;
         destination = workspace_command.resolve_single_rev(args.into.as_deref().unwrap_or("@"))?;
-        if source.id() == destination.id() {
+        if sources.iter().any(|source| source.id() == destination.id()) {
             return Err(user_error("Source and destination cannot be the same"));
         }
+        // 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.
+        sources.reverse();
     } else {
-        source = workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?;
+        let source =
+            workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?;
         let mut parents = source.parents();
         if parents.len() != 1 {
             return Err(user_error("Cannot squash merge commits"));
         }
+        sources = vec![source];
         destination = parents.pop().unwrap();
     }
 
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
     let diff_selector =
         workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
     let mut tx = workspace_command.start_transaction();
-    let tx_description = format!("squash commit {}", source.id().hex());
+    let tx_description = format!("squash commits into {}", destination.id().hex());
     let description = (!args.message_paragraphs.is_empty())
         .then(|| join_message_paragraphs(&args.message_paragraphs));
     move_diff(
         ui,
         &mut tx,
         command.settings(),
-        source,
-        destination,
+        &sources,
+        &destination,
         matcher.as_ref(),
         &diff_selector,
         description,
@@ -125,20 +131,24 @@ pub fn move_diff(
     ui: &mut Ui,
     tx: &mut WorkspaceCommandTransaction,
     settings: &UserSettings,
-    source: Commit,
-    mut destination: Commit,
+    sources: &[Commit],
+    destination: &Commit,
     matcher: &dyn Matcher,
     diff_selector: &DiffSelector,
     description: Option<String>,
     no_rev_arg: bool,
     path_arg: &[String],
 ) -> Result<(), CommandError> {
     tx.base_workspace_helper()
-        .check_rewritable([&source, &destination])?;
-    let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?;
-    let source_tree = source.tree()?;
-    let instructions = format!(
-        "\
+        .check_rewritable(sources.iter().chain(std::iter::once(destination)))?;
+    // Tree diffs to apply to the destination
+    let mut tree_diffs = vec![];
+    let mut abandoned_commits = vec![];
+    for source in sources {
+        let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?;
+        let source_tree = source.tree()?;
+        let instructions = format!(
+            "\
 You are moving changes from: {}
 into commit: {}
 
@@ -150,12 +160,32 @@ Adjust the right side until the diff shows the changes you want to move
 to the destination. If you don't make any changes, then all the changes
 from the source will be moved into the destination.
 ",
-        tx.format_commit_summary(&source),
-        tx.format_commit_summary(&destination)
-    );
-    let new_parent_tree_id =
-        diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?;
-    if new_parent_tree_id == parent_tree.id() {
+            tx.format_commit_summary(source),
+            tx.format_commit_summary(destination)
+        );
+        let new_parent_tree_id =
+            diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?;
+        let new_parent_tree = tx.repo().store().get_root_tree(&new_parent_tree_id)?;
+        // TODO: Do we want to optimize the case of moving to the parent commit (`jj
+        // squash -r`)? The source tree will be unchanged in that case.
+
+        // Apply the reverse of the selected changes onto the source
+        let new_source_tree = source_tree.merge(&new_parent_tree, &parent_tree)?;
+        let abandon_source = new_source_tree.id() == parent_tree.id();
+        if abandon_source {
+            abandoned_commits.push(source);
+            tx.mut_repo().record_abandoned_commit(source.id().clone());
+        } else {
+            tx.mut_repo()
+                .rewrite_commit(settings, source)
+                .set_tree_id(new_source_tree.id().clone())
+                .write()?;
+        }
+        if new_parent_tree_id != parent_tree.id() {
+            tree_diffs.push((parent_tree, new_parent_tree));
+        }
+    }
+    if tree_diffs.is_empty() {
         if diff_selector.is_interactive() {
             return Err(user_error("No changes selected"));
         }
@@ -176,47 +206,34 @@ from the source will be moved into the destination.
             }
         }
     }
-    let new_parent_tree = tx.repo().store().get_root_tree(&new_parent_tree_id)?;
-    // TODO: Do we want to optimize the case of moving to the parent commit (`jj
-    // squash -r`)? The source tree will be unchanged in that case.
-
-    // Apply the reverse of the selected changes onto the source
-    let new_source_tree = source_tree.merge(&new_parent_tree, &parent_tree)?;
-    let abandon_source = new_source_tree.id() == parent_tree.id();
-    if abandon_source {
-        tx.mut_repo().record_abandoned_commit(source.id().clone());
-    } else {
-        tx.mut_repo()
-            .rewrite_commit(settings, &source)
-            .set_tree_id(new_source_tree.id().clone())
-            .write()?;
-    }
-    if tx.repo().index().is_ancestor(source.id(), destination.id()) {
+    let mut rewritten_destination = destination.clone();
+    if sources
+        .iter()
+        .any(|source| tx.repo().index().is_ancestor(source.id(), destination.id()))
+    {
         // If we're moving changes to a descendant, first rebase descendants onto the
-        // rewritten source. Otherwise it will likely already have the content
+        // rewritten sources. Otherwise it will likely already have the content
         // changes we're moving, so applying them will have no effect and the
         // changes will disappear.
         let rebase_map = tx.mut_repo().rebase_descendants_return_map(settings)?;
         let rebased_destination_id = rebase_map.get(destination.id()).unwrap().clone();
-        destination = tx.mut_repo().store().get_commit(&rebased_destination_id)?;
+        rewritten_destination = tx.mut_repo().store().get_commit(&rebased_destination_id)?;
     }
     // Apply the selected changes onto the destination
-    let destination_tree = destination.tree()?;
-    let new_destination_tree = destination_tree.merge(&parent_tree, &new_parent_tree)?;
+    let mut destination_tree = rewritten_destination.tree()?;
+    for (tree1, tree2) in tree_diffs {
+        destination_tree = destination_tree.merge(&tree1, &tree2)?;
+    }
     let description = match description {
         Some(description) => description,
-        None => combine_messages(
-            tx.base_repo(),
-            &source,
-            &destination,
-            settings,
-            abandon_source,
-        )?,
+        None => combine_messages(tx.base_repo(), &abandoned_commits, destination, settings)?,
     };
+    let mut predecessors = vec![destination.id().clone()];
+    predecessors.extend(sources.iter().map(|source| source.id().clone()));
     tx.mut_repo()
-        .rewrite_commit(settings, &destination)
-        .set_tree_id(new_destination_tree.id().clone())
-        .set_predecessors(vec![destination.id().clone(), source.id().clone()])
+        .rewrite_commit(settings, &rewritten_destination)
+        .set_tree_id(destination_tree.id().clone())
+        .set_predecessors(predecessors)
         .set_description(description)
         .write()?;
     Ok(())

cli/src/commands/unsquash.rs

@@ -105,8 +105,7 @@ aborted.
     // case).
     if new_parent_tree_id == parent_base_tree.id() {
         tx.mut_repo().record_abandoned_commit(parent.id().clone());
-        let description =
-            combine_messages(tx.base_repo(), parent, &commit, command.settings(), true)?;
+        let description = combine_messages(tx.base_repo(), &[parent], &commit, command.settings())?;
         // Commit the new child on top of the parent's parents.
         tx.mut_repo()
             .rewrite_commit(command.settings(), &commit)

cli/src/description_util.rs

@@ -40,30 +40,41 @@ JJ: Lines starting with "JJ: " (like this one) will be removed.
     Ok(text_util::complete_newline(description.trim_matches('\n')))
 }
 
+/// Combines the descriptions from the input commits. If only one is non-empty,
+/// then that one is used. Otherwise we concatenate the messages and ask the
+/// user to edit the result in their editor.
 pub fn combine_messages(
     repo: &ReadonlyRepo,
-    source: &Commit,
+    sources: &[&Commit],
     destination: &Commit,
     settings: &UserSettings,
-    abandon_source: bool,
 ) -> Result<String, CommandError> {
-    let description = if abandon_source {
-        if source.description().is_empty() {
-            destination.description().to_string()
-        } else if destination.description().is_empty() {
-            source.description().to_string()
-        } else {
-            let combined = "JJ: Enter a description for the combined commit.\n".to_string()
-                + "JJ: Description from the destination commit:\n"
-                + destination.description()
-                + "\nJJ: Description from the source commit:\n"
-                + source.description();
-            edit_description(repo, &combined, settings)?
+    let non_empty = sources
+        .iter()
+        .chain(std::iter::once(&destination))
+        .filter(|c| !c.description().is_empty())
+        .take(2)
+        .collect_vec();
+    match *non_empty.as_slice() {
+        [] => {
+            return Ok(String::new());
         }
-    } else {
-        destination.description().to_string()
-    };
-    Ok(description)
+        [commit] => {
+            return Ok(commit.description().to_owned());
+        }
+        _ => {}
+    }
+    // Produce a combined description with instructions for the user to edit.
+    // Include empty descriptins too, so the user doesn't have to wonder why they
+    // only see 2 descriptions when they combined 3 commits.
+    let mut combined = "JJ: Enter a description for the combined commit.".to_string();
+    combined.push_str("\nJJ: 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());
+    }
+    edit_description(repo, &combined, settings)
 }
 
 /// Create a description from a list of paragraphs.

cli/tests/test_move_command.rs

@@ -415,7 +415,7 @@ fn test_move_description() {
     JJ: Description from the destination commit:
     destination
 
-    JJ: Description from the source commit:
+    JJ: Description from source commit:
     source
 
     JJ: Lines starting with "JJ: " (like this one) will be removed.