merge_tools: add "$marker_length" variable

Git supports passing the conflict marker length to merge drivers using
"%L". It would be useful if we also had a way to pass the marker length
to merge tools, since it would allow Git merge drivers to be used with
`jj resolve` in more cases. Without this variable, any merge tool that
parses or generates conflict markers could fail on files which require
conflict markers longer than 7 characters.

https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver

CHANGELOG.md

@@ -75,6 +75,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   conflicts to be materialized and parsed correctly in files which already
   contain lines that look like conflict markers.
 
+* New `$marker_length` variable to allow merge tools to support longer conflict
+  markers (equivalent to "%L" for Git merge drivers).
+
 ### Fixed bugs
 
 * The `$NO_COLOR` environment variable must now be non-empty to be respected.

cli/src/merge_tools/external.rs

@@ -183,11 +183,13 @@ pub fn run_mergetool_external(
         .conflict_marker_style
         .unwrap_or(default_conflict_marker_style);
 
+    let uses_marker_length = find_all_variables(&editor.merge_args).contains(&"marker_length");
+
     // If the merge tool doesn't get conflict markers pre-populated in the output
-    // file, we should default to accepting MIN_CONFLICT_MARKER_LEN since the
-    // merge tool is unlikely to know about our rules for conflict marker length.
-    // In the future, we may want to add a "$markerLength" variable.
-    let conflict_marker_len = if editor.merge_tool_edits_conflict_markers {
+    // file and doesn't accept "$marker_length", then we should default to accepting
+    // MIN_CONFLICT_MARKER_LEN since the merge tool can't know about our rules for
+    // conflict marker length.
+    let conflict_marker_len = if editor.merge_tool_edits_conflict_markers || uses_marker_length {
         choose_materialized_conflict_marker_len(&content)
     } else {
         MIN_CONFLICT_MARKER_LEN
@@ -220,7 +222,7 @@ pub fn run_mergetool_external(
         // resolving the root path ever makes sense.
         "".to_owned()
     };
-    let paths: HashMap<&str, _> = files
+    let mut variables: HashMap<&str, _> = files
         .iter()
         .map(|(role, contents)| -> Result<_, ConflictResolveError> {
             let path = temp_dir.path().join(format!("{role}{suffix}"));
@@ -237,9 +239,10 @@ pub fn run_mergetool_external(
             ))
         })
         .try_collect()?;
+    variables.insert("marker_length", conflict_marker_len.to_string());
 
     let mut cmd = Command::new(&editor.program);
-    cmd.args(interpolate_variables(&editor.merge_args, &paths));
+    cmd.args(interpolate_variables(&editor.merge_args, &variables));
     tracing::info!(?cmd, "Invoking the external merge tool:");
     let exit_status = cmd
         .status()
@@ -260,7 +263,7 @@ pub fn run_mergetool_external(
     }
 
     let output_file_contents: Vec<u8> =
-        std::fs::read(paths.get("output").unwrap()).map_err(ExternalToolError::Io)?;
+        std::fs::read(variables.get("output").unwrap()).map_err(ExternalToolError::Io)?;
     if output_file_contents.is_empty() || output_file_contents == initial_output_content {
         return Err(ConflictResolveError::EmptyOrUnchanged);
     }

cli/testing/fake-editor.rs

@@ -27,6 +27,8 @@ use itertools::Itertools;
 struct Args {
     /// Path to the file to edit
     file: PathBuf,
+    /// Other arguments to the editor
+    other_args: Vec<String>,
 }
 
 fn main() {
@@ -64,6 +66,20 @@ fn main() {
                     exit(1)
                 }
             }
+            ["expect-arg", index] => {
+                let index = index.parse::<usize>().unwrap();
+                let Some(actual) = args.other_args.get(index) else {
+                    eprintln!("fake-editor: Missing argument at index {index}.\n");
+                    eprintln!("EXPECTED: <{payload}>");
+                    exit(1)
+                };
+
+                if actual != payload {
+                    eprintln!("fake-editor: Unexpected argument at index {index}.\n");
+                    eprintln!("EXPECTED: <{payload}>\nRECEIVED: <{actual}>");
+                    exit(1)
+                }
+            }
             ["write"] => {
                 fs::write(&args.file, payload).unwrap_or_else(|_| {
                     panic!("Failed to write file {}", args.file.to_str().unwrap())

cli/tests/test_resolve_command.rs

@@ -1096,6 +1096,265 @@ fn test_resolve_conflicts_with_executable() {
     );
 }
 
+#[test]
+fn test_resolve_long_conflict_markers() {
+    let mut test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    // Makes it easier to read the diffs between conflicts
+    test_env.add_config("ui.conflict-marker-style = 'snapshot'");
+
+    // Create a conflict which requires long conflict markers to be materialized
+    create_commit(
+        &test_env,
+        &repo_path,
+        "base",
+        &[],
+        &[("file", "======= base\n")],
+    );
+    create_commit(
+        &test_env,
+        &repo_path,
+        "a",
+        &["base"],
+        &[("file", "<<<<<<< a\n")],
+    );
+    create_commit(
+        &test_env,
+        &repo_path,
+        "b",
+        &["base"],
+        &[("file", ">>>>>>> b\n")],
+    );
+    create_commit(&test_env, &repo_path, "conflict", &["a", "b"], &[]);
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]), 
+    @"file    2-sided conflict");
+    insta::assert_snapshot!(
+        std::fs::read_to_string(repo_path.join("file")).unwrap(),
+        @r##"
+    <<<<<<<<<<< Conflict 1 of 1
+    +++++++++++ Contents of side #1
+    <<<<<<< a
+    ----------- Contents of base
+    ======= base
+    +++++++++++ Contents of side #2
+    >>>>>>> b
+    >>>>>>>>>>> Conflict 1 of 1 ends
+    "##
+    );
+    let editor_script = test_env.set_up_fake_editor();
+    // Allow signaling that conflict markers were produced even if not editing
+    // conflict markers materialized in the output file
+    test_env.add_config("merge-tools.fake-editor.merge-conflict-exit-codes = [1]");
+
+    // By default, conflict markers of length 7 or longer are parsed for
+    // compatibility with Git merge tools
+    std::fs::write(
+        &editor_script,
+        indoc! {b"
+        write
+        <<<<<<<
+        A
+        |||||||
+        BASE
+        =======
+        B
+        >>>>>>>
+        \0fail
+        "},
+    )
+    .unwrap();
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve"]);
+    insta::assert_snapshot!(stdout, @r#""#);
+    insta::assert_snapshot!(stderr, @r#"
+    Resolving conflicts in: file
+    Working copy now at: vruxwmqv 2b985546 conflict | (conflict) conflict
+    Parent commit      : zsuskuln 64177fd4 a | a
+    Parent commit      : royxmykx db442c1e b | b
+    Added 0 files, modified 1 files, removed 0 files
+    There are unresolved conflicts at these paths:
+    file    2-sided conflict
+    New conflicts appeared in these commits:
+      vruxwmqv 2b985546 conflict | (conflict) conflict
+    To resolve the conflicts, start by updating to it:
+      jj new vruxwmqv
+    Then use `jj resolve`, or edit the conflict markers in the file directly.
+    Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
+    Then run `jj squash` to move the resolution into the conflicted commit.
+    "#);
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), 
+    @r##"
+    diff --git a/file b/file
+    --- a/file
+    +++ b/file
+    @@ -1,8 +1,8 @@
+    -<<<<<<<<<<< Conflict 1 of 1
+    -+++++++++++ Contents of side #1
+    -<<<<<<< a
+    ------------ Contents of base
+    -======= base
+    -+++++++++++ Contents of side #2
+    ->>>>>>> b
+    ->>>>>>>>>>> Conflict 1 of 1 ends
+    +<<<<<<< Conflict 1 of 1
+    ++++++++ Contents of side #1
+    +A
+    +------- Contents of base
+    +BASE
+    ++++++++ Contents of side #2
+    +B
+    +>>>>>>> Conflict 1 of 1 ends
+    "##);
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
+        @"file    2-sided conflict"
+    );
+
+    // If the merge tool edits the output file with materialized markers, the
+    // markers must match the length of the materialized markers to be parsed
+    test_env.jj_cmd_ok(&repo_path, &["undo"]);
+    std::fs::write(
+        &editor_script,
+        indoc! {b"
+        dump editor
+        \0write
+        <<<<<<<<<<<
+        <<<<<<< A
+        |||||||||||
+        ======= BASE
+        ===========
+        >>>>>>> B
+        >>>>>>>>>>>
+        \0fail
+        "},
+    )
+    .unwrap();
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &[
+            "resolve",
+            "--config=merge-tools.fake-editor.merge-tool-edits-conflict-markers=true",
+        ],
+    );
+    insta::assert_snapshot!(stdout, @r#""#);
+    insta::assert_snapshot!(stderr, @r#"
+    Resolving conflicts in: file
+    Working copy now at: vruxwmqv fac9406d conflict | (conflict) conflict
+    Parent commit      : zsuskuln 64177fd4 a | a
+    Parent commit      : royxmykx db442c1e b | b
+    Added 0 files, modified 1 files, removed 0 files
+    There are unresolved conflicts at these paths:
+    file    2-sided conflict
+    New conflicts appeared in these commits:
+      vruxwmqv fac9406d conflict | (conflict) conflict
+    To resolve the conflicts, start by updating to it:
+      jj new vruxwmqv
+    Then use `jj resolve`, or edit the conflict markers in the file directly.
+    Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
+    Then run `jj squash` to move the resolution into the conflicted commit.
+    "#);
+    insta::assert_snapshot!(
+        std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r##"
+    <<<<<<<<<<< Conflict 1 of 1
+    +++++++++++ Contents of side #1
+    <<<<<<< a
+    ----------- Contents of base
+    ======= base
+    +++++++++++ Contents of side #2
+    >>>>>>> b
+    >>>>>>>>>>> Conflict 1 of 1 ends
+    "##);
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), 
+    @r##"
+    diff --git a/file b/file
+    --- a/file
+    +++ b/file
+    @@ -1,8 +1,8 @@
+     <<<<<<<<<<< Conflict 1 of 1
+     +++++++++++ Contents of side #1
+    -<<<<<<< a
+    +<<<<<<< A
+     ----------- Contents of base
+    -======= base
+    +======= BASE
+     +++++++++++ Contents of side #2
+    ->>>>>>> b
+    +>>>>>>> B
+     >>>>>>>>>>> Conflict 1 of 1 ends
+    "##);
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
+        @"file    2-sided conflict"
+    );
+
+    // If the merge tool accepts the marker length as an argument, then the conflict
+    // markers should be at least as long as "$marker_length"
+    test_env.jj_cmd_ok(&repo_path, &["undo"]);
+    std::fs::write(
+        &editor_script,
+        indoc! {b"
+        expect-arg 0
+        11\0write
+        <<<<<<<<<<<
+        <<<<<<< A
+        |||||||||||
+        ======= BASE
+        ===========
+        >>>>>>> B
+        >>>>>>>>>>>
+        \0fail
+        "},
+    )
+    .unwrap();
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &[
+            "resolve",
+            r#"--config=merge-tools.fake-editor.merge-args=["$output", "$marker_length"]"#,
+        ],
+    );
+    insta::assert_snapshot!(stdout, @r#""#);
+    insta::assert_snapshot!(stderr, @r#"
+    Resolving conflicts in: file
+    Working copy now at: vruxwmqv 1b29631a conflict | (conflict) conflict
+    Parent commit      : zsuskuln 64177fd4 a | a
+    Parent commit      : royxmykx db442c1e b | b
+    Added 0 files, modified 1 files, removed 0 files
+    There are unresolved conflicts at these paths:
+    file    2-sided conflict
+    New conflicts appeared in these commits:
+      vruxwmqv 1b29631a conflict | (conflict) conflict
+    To resolve the conflicts, start by updating to it:
+      jj new vruxwmqv
+    Then use `jj resolve`, or edit the conflict markers in the file directly.
+    Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
+    Then run `jj squash` to move the resolution into the conflicted commit.
+    "#);
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), 
+    @r##"
+    diff --git a/file b/file
+    --- a/file
+    +++ b/file
+    @@ -1,8 +1,8 @@
+     <<<<<<<<<<< Conflict 1 of 1
+     +++++++++++ Contents of side #1
+    -<<<<<<< a
+    +<<<<<<< A
+     ----------- Contents of base
+    -======= base
+    +======= BASE
+     +++++++++++ Contents of side #2
+    ->>>>>>> b
+    +>>>>>>> B
+     >>>>>>>>>>> Conflict 1 of 1 ends
+    "##);
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
+        @"file    2-sided conflict"
+    );
+}
+
 #[test]
 fn test_multiple_conflicts() {
     let mut test_env = TestEnvironment::default();

docs/config.md

@@ -860,6 +860,11 @@ merge-tool-edits-conflict-markers = true    # See below for an explanation
 - `$base` is replaced with the path to a file containing the contents of the
   conflicted file in the last common ancestor of the two sides of the conflict.
 
+- `$marker_length` is replaced with the length of the conflict markers which
+  should be used for the file. This can be useful if the merge tool parses
+  and/or generates conflict markers. Usually, `jj` uses conflict markers of
+  length 7, but they can be longer if necessary to make parsing unambiguous.
+
 ### Editing conflict markers with a tool or a text editor
 
 By default, the merge tool starts with an empty output file. If the tool puts