From 6aed3c01f07c65f13f0d239060a579f9d6f0dcef Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sat, 16 Nov 2024 11:25:30 -0600 Subject: [PATCH] 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 | 3 +++ cli/src/config/merge_tools.toml | 3 +++ cli/src/merge_tools/external.rs | 19 +++++++++++++------ cli/src/merge_tools/mod.rs | 12 ++++++++++++ lib/src/default_index/revset_engine.rs | 2 +- 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc22b6da6d5..d8deee6d829 100644 --- a/CHANGELOG.md +++ b/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..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 diff --git a/cli/src/config/merge_tools.toml b/cli/src/config/merge_tools.toml index f560053a62f..a3eeac7ad2b 100644 --- a/cli/src/config/merge_tools.toml +++ b/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" diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index a45e81162bf..39fb7d4ab1e 100644 --- a/cli/src/merge_tools/external.rs +++ b/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, } #[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 { + let conflict_marker_style = editor + .conflict_marker_style + .unwrap_or(default_conflict_marker_style); + let initial_output_content: Vec = if editor.merge_tool_edits_conflict_markers { let mut materialized_conflict = vec![]; materialize_merge_result(&content, conflict_marker_style, &mut materialized_conflict) diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 64c40f0f3c7..420f6f0cca8 100644 --- a/cli/src/merge_tools/mod.rs +++ b/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, }, ) "###); diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 544c43fd5a4..7272d673236 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/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)?;