cli: skip formatting instructions if not required

cli/src/cli_util.rs

@@ -2248,12 +2248,12 @@ impl DiffSelector {
         left_tree: &MergedTree,
         right_tree: &MergedTree,
         matcher: &dyn Matcher,
-        instructions: Option<&str>,
+        format_instructions: impl FnOnce() -> String,
     ) -> Result<MergedTreeId, CommandError> {
         match self {
             DiffSelector::NonInteractive => Ok(restore_tree(right_tree, left_tree, matcher)?),
             DiffSelector::Interactive(editor) => {
-                Ok(editor.edit(left_tree, right_tree, matcher, instructions)?)
+                Ok(editor.edit(left_tree, right_tree, matcher, format_instructions)?)
             }
         }
     }

cli/src/commands/commit.rs

@@ -71,21 +71,23 @@ pub(crate) fn cmd_commit(
         workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let base_tree = commit.parent_tree(tx.repo())?;
-    let instructions = format!(
-        "\
+    let format_instructions = || {
+        format!(
+            "\
 You are splitting the working-copy commit: {}
 
 The diff initially shows all changes. Adjust the right side until it shows the
 contents you want for the first commit. The remainder will be included in the
 new working-copy commit.
 ",
-        tx.format_commit_summary(&commit)
-    );
+            tx.format_commit_summary(&commit)
+        )
+    };
     let tree_id = diff_selector.select(
         &base_tree,
         &commit.tree()?,
         matcher.as_ref(),
-        Some(&instructions),
+        format_instructions,
     )?;
     let middle_tree = tx.repo().store().get_root_tree(&tree_id)?;
     if !args.paths.is_empty() && middle_tree.id() == base_tree.id() {

cli/src/commands/diffedit.rs

@@ -89,19 +89,21 @@ pub(crate) fn cmd_diffedit(
 
     let diff_editor = workspace_command.diff_editor(ui, args.tool.as_deref())?;
     let mut tx = workspace_command.start_transaction();
-    let instructions = format!(
-        "\
+    let format_instructions = || {
+        format!(
+            "\
 You are editing changes in: {}
 
 {diff_description}
 
 Adjust the right side until it shows the contents you want. If you
 don't make any changes, then the operation will be aborted.",
-        tx.format_commit_summary(&target_commit),
-    );
+            tx.format_commit_summary(&target_commit),
+        )
+    };
     let base_tree = merge_commit_trees(tx.repo(), base_commits.as_slice())?;
     let tree = target_commit.tree()?;
-    let tree_id = diff_editor.edit(&base_tree, &tree, &EverythingMatcher, Some(&instructions))?;
+    let tree_id = diff_editor.edit(&base_tree, &tree, &EverythingMatcher, format_instructions)?;
     if tree_id == *target_commit.tree_id() {
         writeln!(ui.status(), "Nothing changed.")?;
     } else {

cli/src/commands/split.rs

@@ -88,8 +88,9 @@ pub(crate) fn cmd_split(
     let mut tx = workspace_command.start_transaction();
     let end_tree = commit.tree()?;
     let base_tree = commit.parent_tree(tx.repo())?;
-    let instructions = format!(
-        "\
+    let format_instructions = || {
+        format!(
+            "\
 You are splitting a commit into two: {}
 
 The diff initially shows the changes in the commit you're splitting.
@@ -98,12 +99,13 @@ Adjust the right side until it shows the contents you want for the first commit.
 The remainder will be in the second commit. If you don't make any changes, then
 the operation will be aborted.
 ",
-        tx.format_commit_summary(&commit)
-    );
+            tx.format_commit_summary(&commit)
+        )
+    };
 
     // Prompt the user to select the changes they want for the first commit.
     let selected_tree_id =
-        diff_selector.select(&base_tree, &end_tree, matcher.as_ref(), Some(&instructions))?;
+        diff_selector.select(&base_tree, &end_tree, matcher.as_ref(), format_instructions)?;
     if &selected_tree_id == commit.tree_id() && diff_selector.is_interactive() {
         // The user selected everything from the original commit.
         writeln!(ui.status(), "Nothing changed.")?;

cli/src/commands/squash.rs

@@ -196,8 +196,9 @@ pub fn move_diff(
     for source in sources {
         let parent_tree = source.parent_tree(tx.repo())?;
         let source_tree = source.tree()?;
-        let instructions = format!(
-            "\
+        let format_instructions = || {
+            format!(
+                "\
 You are moving changes from: {}
 into commit: {}
 
@@ -209,11 +210,12 @@ 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)
-        );
+                tx.format_commit_summary(source),
+                tx.format_commit_summary(destination)
+            )
+        };
         let selected_tree_id =
-            diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?;
+            diff_selector.select(&parent_tree, &source_tree, matcher, format_instructions)?;
         let selected_tree = tx.repo().store().get_root_tree(&selected_tree_id)?;
         let abandon = !keep_emptied && selected_tree.id() == source_tree.id();
         if !abandon && selected_tree_id == parent_tree.id() {

cli/src/commands/unsquash.rs

@@ -71,8 +71,9 @@ pub(crate) fn cmd_unsquash(
     let parent_base_tree = parent.parent_tree(tx.repo())?;
     let new_parent_tree_id;
     if let Some(diff_editor) = &interactive_editor {
-        let instructions = format!(
-            "\
+        let format_instructions = || {
+            format!(
+                "\
 You are moving changes from: {}
 into its child: {}
 
@@ -83,15 +84,16 @@ the parent commit. The changes you edited out will be moved into the
 child commit. If you don't make any changes, then the operation will be
 aborted.
 ",
-            tx.format_commit_summary(&parent),
-            tx.format_commit_summary(&commit)
-        );
+                tx.format_commit_summary(&parent),
+                tx.format_commit_summary(&commit)
+            )
+        };
         let parent_tree = parent.tree()?;
         new_parent_tree_id = diff_editor.edit(
             &parent_base_tree,
             &parent_tree,
             &EverythingMatcher,
-            Some(&instructions),
+            format_instructions,
         )?;
         if new_parent_tree_id == parent_base_tree.id() {
             return Err(user_error("No changes selected"));

cli/src/merge_tools/mod.rs

@@ -219,20 +219,20 @@ impl DiffEditor {
         left_tree: &MergedTree,
         right_tree: &MergedTree,
         matcher: &dyn Matcher,
-        instructions: Option<&str>,
+        format_instructions: impl FnOnce() -> String,
     ) -> Result<MergedTreeId, DiffEditError> {
         match &self.tool {
             MergeTool::Builtin => {
                 Ok(edit_diff_builtin(left_tree, right_tree, matcher).map_err(Box::new)?)
             }
             MergeTool::External(editor) => {
-                let instructions = self.use_instructions.then_some(instructions).flatten();
+                let instructions = self.use_instructions.then(format_instructions);
                 edit_diff_external(
                     editor,
                     left_tree,
                     right_tree,
                     matcher,
-                    instructions,
+                    instructions.as_deref(),
                     self.base_ignores.clone(),
                 )
             }

cli/testing/fake-diff-editor.rs

@@ -57,7 +57,7 @@ fn files_recursively(p: &Path) -> HashSet<String> {
 fn main() {
     let args: Args = Args::parse();
     let edit_script_path = PathBuf::from(std::env::var_os("DIFF_EDIT_SCRIPT").unwrap());
-    let edit_script = String::from_utf8(std::fs::read(edit_script_path).unwrap()).unwrap();
+    let edit_script = String::from_utf8(std::fs::read(&edit_script_path).unwrap()).unwrap();
     for instruction in edit_script.split('\0') {
         let (command, payload) = instruction.split_once('\n').unwrap_or((instruction, ""));
         let parts = command.split(' ').collect_vec();
@@ -111,6 +111,10 @@ fn main() {
                     std::fs::remove_file(args.after.join(file)).unwrap();
                 }
             }
+            ["dump", file, dest] => {
+                let dest_path = edit_script_path.parent().unwrap().join(dest);
+                std::fs::copy(args.after.join(file), dest_path).unwrap();
+            }
             ["write", file] => {
                 std::fs::write(args.after.join(file), payload).unwrap();
             }

cli/tests/test_commit_command.rs

@@ -104,11 +104,21 @@ fn test_commit_interactive() {
     std::fs::write(edit_script, ["dump editor"].join("\0")).unwrap();
 
     let diff_editor = test_env.set_up_fake_diff_editor();
-    std::fs::write(diff_editor, "rm file2").unwrap();
+    let diff_script = ["rm file2", "dump JJ-INSTRUCTIONS instrs"].join("\0");
+    std::fs::write(diff_editor, diff_script).unwrap();
 
     // Create a commit interactively and select only file1
     test_env.jj_cmd_ok(&workspace_path, &["commit", "-i"]);
 
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r###"
+    You are splitting the working-copy commit: qpvuntsm 4219467e add files
+
+    The diff initially shows all changes. Adjust the right side until it shows the
+    contents you want for the first commit. The remainder will be included in the
+    new working-copy commit.
+    "###);
+
     insta::assert_snapshot!(
         std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
     add files

cli/tests/test_diffedit_command.rs

@@ -33,14 +33,28 @@ fn test_diffedit() {
     // Test the setup; nothing happens if we make no changes
     std::fs::write(
         &edit_script,
-        "files-before file1 file2\0files-after JJ-INSTRUCTIONS file2",
+        [
+            "files-before file1 file2",
+            "files-after JJ-INSTRUCTIONS file2",
+            "dump JJ-INSTRUCTIONS instrs",
+        ]
+        .join("\0"),
     )
     .unwrap();
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["diffedit"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Nothing changed.
     "###);
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r###"
+    You are editing changes in: kkmpptxz 3d4cce89 (no description set)
+
+    The diff initially shows the commit's changes.
+
+    Adjust the right side until it shows the contents you want. If you
+    don't make any changes, then the operation will be aborted.
+    "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s"]);
     insta::assert_snapshot!(stdout, @r###"
     D file1

cli/tests/test_split_command.rs

@@ -559,3 +559,51 @@ fn test_split_message_editor_avoids_unc() {
     // over 260 chars.
     assert_eq!(edited_path, dunce::simplified(&edited_path));
 }
+
+#[test]
+fn test_split_interactive() {
+    let mut test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let workspace_path = test_env.env_root().join("repo");
+
+    std::fs::write(workspace_path.join("file1"), "foo\n").unwrap();
+    std::fs::write(workspace_path.join("file2"), "bar\n").unwrap();
+    let edit_script = test_env.set_up_fake_editor();
+    std::fs::write(edit_script, ["dump editor"].join("\0")).unwrap();
+
+    let diff_editor = test_env.set_up_fake_diff_editor();
+    let diff_script = ["rm file2", "dump JJ-INSTRUCTIONS instrs"].join("\0");
+    std::fs::write(diff_editor, diff_script).unwrap();
+
+    // Split the working commit interactively and select only file1
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_path, &["split"]);
+
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r###"
+    You are splitting a commit into two: qpvuntsm 44af2155 (no description set)
+
+    The diff initially shows the changes in the commit you're splitting.
+
+    Adjust the right side until it shows the contents you want for the first commit.
+    The remainder will be in the second commit. If you don't make any changes, then
+    the operation will be aborted.
+    "###);
+
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
+    JJ: Enter a description for the first commit.
+
+    JJ: This commit contains the following changes:
+    JJ:     A file1
+
+    JJ: Lines starting with "JJ: " (like this one) will be removed.
+    "###);
+
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    First part: qpvuntsm 0e15949e (no description set)
+    Second part: rlvkpnrz 9ed12e4c (no description set)
+    Working copy now at: rlvkpnrz 9ed12e4c (no description set)
+    Parent commit      : qpvuntsm 0e15949e (no description set)
+    "###);
+}

cli/tests/test_squash_command.rs

@@ -157,13 +157,29 @@ fn test_squash_partial() {
     // If we don't make any changes in the diff-editor, the whole change is moved
     // into the parent
     let edit_script = test_env.set_up_fake_diff_editor();
+    std::fs::write(&edit_script, "dump JJ-INSTRUCTIONS instrs").unwrap();
     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 3c633226 c | (no description set)
     Parent commit      : qpvuntsm 38ffd8b9 a b | (no description set)
     "###);
+
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r###"
+    You are moving changes from: kkmpptxz d117da27 b | (no description set)
+    into commit: qpvuntsm 54d3c1c0 a | (no description set)
+
+    The left side of the diff shows the contents of the parent commit. The
+    right side initially shows the contents of the commit you're moving
+    changes from.
+
+    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.
+    "###);
+
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  3c6332267ea8 c
     ◉  38ffd8b98578 a b

cli/tests/test_unsquash_command.rs

@@ -156,13 +156,28 @@ fn test_unsquash_partial() {
     // If we don't make any changes in the diff-editor, the whole change is moved
     // from the parent
     let edit_script = test_env.set_up_fake_diff_editor();
+    std::fs::write(&edit_script, "dump JJ-INSTRUCTIONS instrs").unwrap();
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["unsquash", "-r", "b", "-i"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Rebased 1 descendant commits
     Working copy now at: mzvwutvl 8802263d c | (no description set)
     Parent commit      : kkmpptxz 5bd83140 b | (no description set)
     "###);
+
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r###"
+    You are moving changes from: qpvuntsm 54d3c1c0 a | (no description set)
+    into its child: kkmpptxz d117da27 b | (no description set)
+
+    The diff initially shows the parent commit's changes.
+
+    Adjust the right side until it shows the contents you want to keep in
+    the parent commit. The changes you edited out will be moved into the
+    child commit. If you don't make any changes, then the operation will be
+    aborted.
+    "###);
+
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  8802263dbd92 c
     ◉  5bd83140fd47 b