merge_tools: allow setting conflict marker style per-tool

I left the "merge-tool-edits-conflict-markers" option unchanged,
since removing it would likely break some existing configurations. It
also seems like it could be useful to have a merge tool use the default
conflict markers instead of requiring the conflict marker style to
always be set for the merge tool (e.g. if a merge tool allows the user
to manually edit the conflicts).

CHANGELOG.md

@@ -49,6 +49,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   replicates Git's "diff3" conflict style, meaning it is more likely to work
   with external tools, but it doesn't support conflicts with more than 2 sides.
 
+* New `merge-tools.<TOOL>.conflict-marker-style` config option to override the
+  conflict marker style used for a specific merge tool.
+
 ### Fixed bugs
 
 ## [0.23.0] - 2024-11-06

cli/src/config/merge_tools.toml

@@ -52,10 +52,13 @@ merge-args = ["--wait", "--merge", "$left", "$right", "$base", "$output"]
 # markers. Unfortunately, it does not seem to be able to output conflict markers when
 # the user only resolves some of the conflicts.
 merge-tool-edits-conflict-markers = true
+# VS Code prefers Git-style conflict markers
+conflict-marker-style = "git-diff3"
 
 # free/libre distribution of vscode, functionally more or less the same
 [merge-tools.vscodium]
 program = "codium"
 merge-args = ["--wait", "--merge", "$left", "$right", "$base", "$output"]
 merge-tool-edits-conflict-markers = true
+conflict-marker-style = "git-diff3"
 

cli/src/merge_tools/external.rs

@@ -60,13 +60,15 @@ pub struct ExternalMergeTool {
     /// If false (default), the `$output` file starts out empty and is accepted
     /// as a full conflict resolution as-is by `jj` after the merge tool is
     /// done with it. If true, the `$output` file starts out with the
-    /// contents of the conflict, with JJ's conflict markers. After the
-    /// merge tool is done, any remaining conflict markers in the
-    /// file parsed and taken to mean that the conflict was only partially
+    /// contents of the conflict, with the configured conflict markers. After
+    /// the merge tool is done, any remaining conflict markers in the
+    /// file are parsed and taken to mean that the conflict was only partially
     /// resolved.
-    // TODO: Instead of a boolean, this could denote the flavor of conflict markers to put in
-    // the file (`jj` or `diff3` for example).
     pub merge_tool_edits_conflict_markers: bool,
+    /// If provided, overrides the normal conflict marker style setting. This is
+    /// useful if a merge tool parses conflict markers, and so it requires a
+    /// specific format.
+    pub conflict_marker_style: Option<ConflictMarkerStyle>,
 }
 
 #[derive(serde::Deserialize, Copy, Clone, Debug, Eq, PartialEq)]
@@ -91,6 +93,7 @@ impl Default for ExternalMergeTool {
             edit_args: ["$left", "$right"].map(ToOwned::to_owned).to_vec(),
             merge_args: vec![],
             merge_tool_edits_conflict_markers: false,
+            conflict_marker_style: None,
             diff_invocation_mode: DiffToolMode::Dir,
         }
     }
@@ -157,8 +160,12 @@ pub fn run_mergetool_external(
     repo_path: &RepoPath,
     conflict: MergedTreeValue,
     tree: &MergedTree,
-    conflict_marker_style: ConflictMarkerStyle,
+    default_conflict_marker_style: ConflictMarkerStyle,
 ) -> Result<MergedTreeId, ConflictResolveError> {
+    let conflict_marker_style = editor
+        .conflict_marker_style
+        .unwrap_or(default_conflict_marker_style);
+
     let initial_output_content: Vec<u8> = if editor.merge_tool_edits_conflict_markers {
         let mut materialized_conflict = vec![];
         materialize_merge_result(&content, conflict_marker_style, &mut materialized_conflict)

cli/src/merge_tools/mod.rs

@@ -385,6 +385,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -412,6 +413,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -446,6 +448,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -469,6 +472,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -491,6 +495,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -519,6 +524,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -545,6 +551,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -565,6 +572,7 @@ mod tests {
                 ],
                 merge_args: [],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -616,6 +624,7 @@ mod tests {
                     "$output",
                 ],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -662,6 +671,7 @@ mod tests {
                     "$output",
                 ],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -690,6 +700,7 @@ mod tests {
                     "$output",
                 ],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);
@@ -721,6 +732,7 @@ mod tests {
                     "$output",
                 ],
                 merge_tool_edits_conflict_markers: false,
+                conflict_marker_style: None,
             },
         )
         "###);

lib/src/default_index/revset_engine.rs

@@ -1301,7 +1301,7 @@ fn matches_diff_from_parent(
             let left_future = materialize_tree_value(store, &entry.path, left_value);
             let right_future = materialize_tree_value(store, &entry.path, right_value);
             let (left_value, right_value) = futures::try_join!(left_future, right_future)?;
-            // Use "diff" setting for now (will be updated in later commit)
+            // Ignore setting for conflict marker style to ensure consistency
             let left_content = to_file_content(&entry.path, left_value, ConflictMarkerStyle::Diff)?;
             let right_content =
                 to_file_content(&entry.path, right_value, ConflictMarkerStyle::Diff)?;