cli: split: edit both descriptions at once

closes #5432

Since #3828, it's been possible to describe multiple commits in the same
editor. This makes that the default method for `jj split`.

CHANGELOG.md

@@ -23,6 +23,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   `ellipsis` parameter; passing this prepends or appends the ellipsis to the
   content if it is truncated to fit the maximum width.
 
+* `jj split` now edits both commit descriptions at the same time.
+
 ### Fixed bugs
 
 * `jj status` now shows untracked files under untracked directories.

cli/src/commands/describe.rs

@@ -25,14 +25,12 @@ use tracing::instrument;
 
 use crate::cli_util::CommandHelper;
 use crate::cli_util::RevisionArg;
-use crate::command_error::user_error;
 use crate::command_error::CommandError;
 use crate::complete;
 use crate::description_util::description_template;
 use crate::description_util::edit_description;
 use crate::description_util::edit_multiple_descriptions;
 use crate::description_util::join_message_paragraphs;
-use crate::description_util::ParsedBulkEditMessage;
 use crate::text_util::parse_author;
 use crate::ui::Ui;
 
@@ -199,32 +197,13 @@ pub(crate) fn cmd_describe(
             let description = edit_description(&text_editor, &template)?;
             vec![(&commits[0], description)]
         } else {
-            let ParsedBulkEditMessage {
-                descriptions,
-                missing,
-                duplicates,
-                unexpected,
-            } = edit_multiple_descriptions(ui, &text_editor, &tx, &temp_commits)?;
-            if !missing.is_empty() {
-                return Err(user_error(format!(
-                    "The description for the following commits were not found in the edited \
-                     message: {}",
-                    missing.join(", ")
-                )));
-            }
-            if !duplicates.is_empty() {
-                return Err(user_error(format!(
-                    "The following commits were found in the edited message multiple times: {}",
-                    duplicates.join(", ")
-                )));
-            }
-            if !unexpected.is_empty() {
-                return Err(user_error(format!(
-                    "The following commits were not being edited, but were found in the edited \
-                     message: {}",
-                    unexpected.join(", ")
-                )));
-            }
+            let descriptions = edit_multiple_descriptions(
+                ui,
+                &text_editor,
+                &tx,
+                temp_commits.iter().map(|(id, commit)| (*id, commit, "")),
+            )?
+            .assert_complete()?;
 
             let commit_descriptions = commits
                 .iter()

cli/src/commands/split.rs

@@ -24,8 +24,7 @@ use crate::cli_util::RevisionArg;
 use crate::command_error::user_error_with_hint;
 use crate::command_error::CommandError;
 use crate::complete;
-use crate::description_util::description_template;
-use crate::description_util::edit_description;
+use crate::description_util::edit_multiple_descriptions;
 use crate::ui::Ui;
 
 /// Split a revision in two
@@ -137,27 +136,18 @@ The remainder will be in the second commit.
 
     // Create the first commit, which includes the changes selected by the user.
     let selected_tree = tx.repo().store().get_root_tree(&selected_tree_id)?;
-    let first_commit = {
+    let (first_commit, mut first_builder) = {
         let mut commit_builder = tx.repo_mut().rewrite_commit(&commit).detach();
         commit_builder.set_tree_id(selected_tree_id);
         if commit_builder.description().is_empty() {
             commit_builder.set_description(tx.settings().get_string("ui.default-description")?);
         }
-        let temp_commit = commit_builder.write_hidden()?;
-        let template = description_template(
-            ui,
-            &tx,
-            "Enter a description for the first commit.",
-            &temp_commit,
-        )?;
-        let description = edit_description(&text_editor, &template)?;
-        commit_builder.set_description(description);
-        commit_builder.write(tx.repo_mut())?
+        (commit_builder.write_hidden()?, commit_builder)
     };
 
     // Create the second commit, which includes everything the user didn't
     // select.
-    let second_commit = {
+    let (second_commit, mut second_builder) = {
         let new_tree = if args.parallel {
             // Merge the original commit tree with its parent using the tree
             // containing the user selected changes as the base for the merge.
@@ -178,22 +168,33 @@ The remainder will be in the second commit.
             // Generate a new change id so that the commit being split doesn't
             // become divergent.
             .generate_new_change_id();
-        let description = if commit.description().is_empty() {
-            // If there was no description before, don't ask for one for the
-            // second commit.
-            "".to_string()
-        } else {
-            let temp_commit = commit_builder.write_hidden()?;
-            let template = description_template(
-                ui,
-                &tx,
-                "Enter a description for the second commit.",
-                &temp_commit,
-            )?;
-            edit_description(&text_editor, &template)?
+        (commit_builder.write_hidden()?, commit_builder)
+    };
+
+    let commits = [
+        (
+            first_commit.id(),
+            &first_commit,
+            "Enter a description for the first commit.",
+        ),
+        (
+            second_commit.id(),
+            &second_commit,
+            "Enter a description for the second commit.",
+        ),
+    ];
+    let descriptions =
+        edit_multiple_descriptions(ui, &text_editor, &tx, commits)?.assert_complete()?;
+
+    first_builder.set_description(&descriptions[first_commit.id()]);
+    let first_commit = first_builder.write(tx.repo_mut())?;
+
+    let second_commit = {
+        if !args.parallel {
+            second_builder.set_parents(vec![first_commit.id().clone()]);
         };
-        commit_builder.set_description(description);
-        commit_builder.write(tx.repo_mut())?
+        second_builder.set_description(&descriptions[second_commit.id()]);
+        second_builder.write(tx.repo_mut())?
     };
 
     // Mark the commit being split as rewritten to the second commit. As a

cli/src/description_util.rs

@@ -21,6 +21,7 @@ use thiserror::Error;
 
 use crate::cli_util::short_commit_hash;
 use crate::cli_util::WorkspaceCommandTransaction;
+use crate::command_error::user_error;
 use crate::command_error::CommandError;
 use crate::config::CommandNameAndArgs;
 use crate::formatter::PlainTextFormatter;
@@ -169,11 +170,11 @@ JJ: Lines starting with "JJ:" (like this one) will be removed.
 }
 
 /// Edits the descriptions of the given commits in a single editor session.
-pub fn edit_multiple_descriptions(
+pub fn edit_multiple_descriptions<'a>(
     ui: &Ui,
     editor: &TextEditor,
     tx: &WorkspaceCommandTransaction,
-    commits: &[(&CommitId, Commit)],
+    commits: impl IntoIterator<Item = (&'a CommitId, &'a Commit, &'a str)>,
 ) -> Result<ParsedBulkEditMessage<CommitId>, CommandError> {
     let mut commits_map = IndexMap::new();
     let mut bulk_message = String::new();
@@ -185,13 +186,13 @@ pub fn edit_multiple_descriptions(
         JJ: - The syntax of the separator lines may change in the future.
 
     "#});
-    for (commit_id, temp_commit) in commits {
+    for (commit_id, temp_commit, intro) in commits {
         let commit_hash = short_commit_hash(commit_id);
         bulk_message.push_str("JJ: describe ");
         bulk_message.push_str(&commit_hash);
         bulk_message.push_str(" -------\n");
-        commits_map.insert(commit_hash, *commit_id);
-        let template = description_template(ui, tx, "", temp_commit)?;
+        commits_map.insert(commit_hash, commit_id);
+        let template = description_template(ui, tx, intro, temp_commit)?;
         bulk_message.push_str(&template);
         bulk_message.push('\n');
     }
@@ -219,6 +220,32 @@ pub struct ParsedBulkEditMessage<T> {
     pub unexpected: Vec<String>,
 }
 
+impl<T> ParsedBulkEditMessage<T> {
+    pub fn assert_complete(self) -> Result<HashMap<T, String>, CommandError> {
+        if !self.missing.is_empty() {
+            return Err(user_error(format!(
+                "The description for the following commits were not found in the edited message: \
+                 {}",
+                self.missing.join(", ")
+            )));
+        }
+        if !self.duplicates.is_empty() {
+            return Err(user_error(format!(
+                "The following commits were found in the edited message multiple times: {}",
+                self.duplicates.join(", ")
+            )));
+        }
+        if !self.unexpected.is_empty() {
+            return Err(user_error(format!(
+                "The following commits were not being edited, but were found in the edited \
+                 message: {}",
+                self.unexpected.join(", ")
+            )));
+        }
+        Ok(self.descriptions)
+    }
+}
+
 #[derive(Debug, Error, PartialEq)]
 pub enum ParseBulkEditMessageError {
     #[error(r#"Found the following line without a commit header: "{0}""#)]