From 86d79b9a05b9d6554d98b3a4d4347e615a678bc8 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Mon, 11 Mar 2024 09:30:56 -0700
Subject: [PATCH] description_utils: teach `combine_messages()` to handle more
 than two sources

I plan to teach `jj squash --from` to accept a revset as input.
---
 cli/src/commands/squash.rs         |  7 +----
 cli/src/commands/unsquash.rs       |  3 +-
 cli/src/description_util.rs        | 44 ++++++++++++++++++++----------
 cli/tests/test_move_command.rs     |  2 +-
 cli/tests/test_squash_command.rs   |  2 +-
 cli/tests/test_unsquash_command.rs |  2 +-
 6 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs
index 8e9fa5173c..918be7515a 100644
--- a/cli/src/commands/squash.rs
+++ b/cli/src/commands/squash.rs
@@ -207,12 +207,7 @@ from the source will be moved into the destination.
         Some(description) => description,
         None => {
             if abandon_source {
-                combine_messages(
-                    tx.base_repo(),
-                    &source,
-                    &destination,
-                    settings,
-                )?
+                combine_messages(tx.base_repo(), &[&source], &destination, settings)?
             } else {
                 destination.description().to_owned()
             }
diff --git a/cli/src/commands/unsquash.rs b/cli/src/commands/unsquash.rs
index 0e7f79ce94..ca120d9f29 100644
--- a/cli/src/commands/unsquash.rs
+++ b/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())?;
+        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)
diff --git a/cli/src/description_util.rs b/cli/src/description_util.rs
index ee91d8d16c..f3ce542db7 100644
--- a/cli/src/description_util.rs
+++ b/cli/src/description_util.rs
@@ -40,25 +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,
 ) -> Result<String, CommandError> {
-    let description = 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)?
-    };
-    Ok(description)
+    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());
+        }
+        [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.
diff --git a/cli/tests/test_move_command.rs b/cli/tests/test_move_command.rs
index d302125765..50b3d5b9b2 100644
--- a/cli/tests/test_move_command.rs
+++ b/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.
diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs
index f32dd3e8a9..ab084e422c 100644
--- a/cli/tests/test_squash_command.rs
+++ b/cli/tests/test_squash_command.rs
@@ -646,7 +646,7 @@ fn test_squash_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.
diff --git a/cli/tests/test_unsquash_command.rs b/cli/tests/test_unsquash_command.rs
index 3026882883..c5ab18f735 100644
--- a/cli/tests/test_unsquash_command.rs
+++ b/cli/tests/test_unsquash_command.rs
@@ -296,7 +296,7 @@ fn test_unsquash_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.