merge_tools: extract MergeToolFile struct

I'm going to change the merge tools to accept multiple files, and this
will make it easier to pass all the required data about each file.

cli/src/merge_tools/builtin.rs

@@ -2,7 +2,6 @@ use std::borrow::Cow;
 use std::path::Path;
 use std::sync::Arc;
 
-use bstr::BString;
 use futures::StreamExt;
 use futures::TryFutureExt;
 use futures::TryStreamExt;
@@ -32,6 +31,8 @@ use jj_lib::store::Store;
 use pollster::FutureExt;
 use thiserror::Error;
 
+use super::MergeToolFile;
+
 #[derive(Debug, Error)]
 pub enum BuiltinToolError {
     #[error("Failed to record changes")]
@@ -637,33 +638,48 @@ fn make_merge_sections(
     Ok(sections)
 }
 
+fn make_merge_file(
+    merge_tool_file: &MergeToolFile,
+) -> Result<scm_record::File<'static>, BuiltinToolError> {
+    let merge_result = files::merge(&merge_tool_file.content);
+    let sections = make_merge_sections(merge_result)?;
+    Ok(scm_record::File {
+        old_path: None,
+        // Path for displaying purposes, not for file access.
+        path: Cow::Owned(
+            merge_tool_file
+                .repo_path
+                .to_fs_path_unchecked(Path::new("")),
+        ),
+        file_mode: None,
+        sections,
+    })
+}
+
 pub fn edit_merge_builtin(
     tree: &MergedTree,
-    path: &RepoPath,
-    content: Merge<BString>,
+    merge_tool_file: &MergeToolFile,
 ) -> Result<MergedTreeId, BuiltinToolError> {
-    let merge_result = files::merge(&content);
-    let sections = make_merge_sections(merge_result)?;
     let mut input = scm_record::helpers::CrosstermInput;
     let recorder = scm_record::Recorder::new(
         scm_record::RecordState {
             is_read_only: false,
-            files: vec![scm_record::File {
-                old_path: None,
-                // Path for displaying purposes, not for file access.
-                path: Cow::Owned(path.to_fs_path_unchecked(Path::new(""))),
-                file_mode: None,
-                sections,
-            }],
+            files: vec![make_merge_file(merge_tool_file)?],
             commits: Default::default(),
         },
         &mut input,
     );
     let state = recorder.run()?;
 
     let file = state.files.into_iter().exactly_one().unwrap();
-    apply_diff_builtin(tree.store(), tree, tree, vec![path.to_owned()], &[file])
-        .map_err(BuiltinToolError::BackendError)
+    apply_diff_builtin(
+        tree.store(),
+        tree,
+        tree,
+        vec![merge_tool_file.repo_path.clone()],
+        &[file],
+    )
+    .map_err(BuiltinToolError::BackendError)
 }
 
 #[cfg(test)]

cli/src/merge_tools/external.rs

@@ -8,7 +8,6 @@ use std::sync::Arc;
 
 use bstr::BString;
 use itertools::Itertools;
-use jj_lib::backend::FileId;
 use jj_lib::backend::MergedTreeId;
 use jj_lib::backend::TreeValue;
 use jj_lib::conflicts;
@@ -17,10 +16,8 @@ use jj_lib::conflicts::ConflictMarkerStyle;
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::matchers::Matcher;
 use jj_lib::merge::Merge;
-use jj_lib::merge::MergedTreeValue;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::merged_tree::MergedTreeBuilder;
-use jj_lib::repo_path::RepoPath;
 use jj_lib::working_copy::CheckoutOptions;
 use pollster::FutureExt;
 use thiserror::Error;
@@ -33,6 +30,7 @@ use super::diff_working_copies::DiffSide;
 use super::ConflictResolveError;
 use super::DiffEditError;
 use super::DiffGenerateError;
+use super::MergeToolFile;
 use crate::config::find_all_variables;
 use crate::config::interpolate_variables;
 use crate::config::CommandNameAndArgs;
@@ -170,19 +168,23 @@ pub enum ExternalToolError {
 
 pub fn run_mergetool_external(
     editor: &ExternalMergeTool,
-    file_merge: Merge<Option<FileId>>,
-    content: Merge<BString>,
-    repo_path: &RepoPath,
-    conflict: MergedTreeValue,
     tree: &MergedTree,
+    merge_tool_file: &MergeToolFile,
     default_conflict_marker_style: ConflictMarkerStyle,
 ) -> Result<MergedTreeId, ConflictResolveError> {
+    let MergeToolFile {
+        repo_path,
+        conflict,
+        file_merge,
+        content,
+    } = merge_tool_file;
+
     let conflict_marker_style = editor
         .conflict_marker_style
         .unwrap_or(default_conflict_marker_style);
 
     let initial_output_content = if editor.merge_tool_edits_conflict_markers {
-        materialize_merge_result_to_bytes(&content, conflict_marker_style)
+        materialize_merge_result_to_bytes(content, conflict_marker_style)
     } else {
         BString::default()
     };
@@ -252,7 +254,7 @@ pub fn run_mergetool_external(
 
     let new_file_ids = if editor.merge_tool_edits_conflict_markers || exit_status_implies_conflict {
         conflicts::update_from_content(
-            &file_merge,
+            file_merge,
             tree.store(),
             repo_path,
             output_file_contents.as_slice(),

cli/src/merge_tools/mod.rs

@@ -18,6 +18,8 @@ mod external;
 
 use std::sync::Arc;
 
+use bstr::BString;
+use jj_lib::backend::FileId;
 use jj_lib::backend::MergedTreeId;
 use jj_lib::config::ConfigGetError;
 use jj_lib::config::ConfigGetResultExt as _;
@@ -26,6 +28,8 @@ use jj_lib::conflicts::extract_as_single_hunk;
 use jj_lib::conflicts::ConflictMarkerStyle;
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::matchers::Matcher;
+use jj_lib::merge::Merge;
+use jj_lib::merge::MergedTreeValue;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::repo_path::InvalidRepoPathError;
 use jj_lib::repo_path::RepoPath;
@@ -257,6 +261,14 @@ impl DiffEditor {
     }
 }
 
+/// A file to be merged by a merge tool.
+struct MergeToolFile {
+    repo_path: RepoPathBuf,
+    conflict: MergedTreeValue,
+    file_merge: Merge<Option<FileId>>,
+    content: Merge<BString>,
+}
+
 /// Configured 3-way merge editor.
 #[derive(Clone, Debug)]
 pub struct MergeEditor {
@@ -334,19 +346,22 @@ impl MergeEditor {
         };
         let content =
             extract_as_single_hunk(&simplified_file_merge, tree.store(), repo_path).block_on()?;
+        let merge_tool_file = MergeToolFile {
+            repo_path: repo_path.to_owned(),
+            conflict,
+            file_merge,
+            content,
+        };
 
         match &self.tool {
             MergeTool::Builtin => {
-                let tree_id = edit_merge_builtin(tree, repo_path, content).map_err(Box::new)?;
+                let tree_id = edit_merge_builtin(tree, &merge_tool_file).map_err(Box::new)?;
                 Ok(tree_id)
             }
             MergeTool::External(editor) => external::run_mergetool_external(
                 editor,
-                file_merge,
-                content,
-                repo_path,
-                conflict,
                 tree,
+                &merge_tool_file,
                 self.conflict_marker_style,
             ),
         }