conflicts: escape conflict markers by making them longer

CHANGELOG.md

@@ -57,6 +57,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * A new Email template type is added. `Signature.email()` now returns an Email
   template type instead of a String.
 
+* Conflict markers are now allowed to be longer than 7 characters, allowing
+  conflicts to be materialized and parsed correctly in files which already
+  contain lines which look like conflict markers.
+
 ### Fixed bugs
 
 * The `$NO_COLOR` environment variable must now be non-empty to be respected.

cli/src/commands/debug/local_working_copy.rs

@@ -40,10 +40,11 @@ pub fn cmd_debug_local_working_copy(
     for (file, state) in wc.file_states()? {
         writeln!(
             ui.stdout(),
-            "{:?} {:13?} {:10?} {:?}",
+            "{:?} {:13?} {:10?} {:?} {:?}",
             state.file_type,
             state.size,
             state.mtime.0,
+            state.materialized_conflict_data,
             file
         )?;
     }

cli/src/merge_tools/external.rs

@@ -12,8 +12,10 @@ use jj_lib::backend::FileId;
 use jj_lib::backend::MergedTreeId;
 use jj_lib::backend::TreeValue;
 use jj_lib::conflicts;
-use jj_lib::conflicts::materialize_merge_result_to_bytes;
+use jj_lib::conflicts::choose_materialized_conflict_marker_len;
+use jj_lib::conflicts::materialize_merge_result_to_bytes_with_marker_len;
 use jj_lib::conflicts::ConflictMarkerStyle;
+use jj_lib::conflicts::MIN_CONFLICT_MARKER_LEN;
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::matchers::Matcher;
 use jj_lib::merge::Merge;
@@ -181,8 +183,21 @@ pub fn run_mergetool_external(
         .conflict_marker_style
         .unwrap_or(default_conflict_marker_style);
 
+    // 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 {
+        choose_materialized_conflict_marker_len(&content)
+    } else {
+        MIN_CONFLICT_MARKER_LEN
+    };
     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_with_marker_len(
+            &content,
+            conflict_marker_style,
+            conflict_marker_len,
+        )
     } else {
         BString::default()
     };
@@ -257,6 +272,7 @@ pub fn run_mergetool_external(
             repo_path,
             output_file_contents.as_slice(),
             conflict_marker_style,
+            conflict_marker_len,
         )
         .block_on()?
     } else {

cli/tests/test_working_copy.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use indoc::indoc;
+use regex::Regex;
 
 use crate::common::TestEnvironment;
 
@@ -249,3 +250,182 @@ fn test_snapshot_invalid_ignore_pattern() {
     2: invalid utf-8 sequence of 1 bytes from index 0
     "##);
 }
+
+#[test]
+fn test_conflict_marker_length_stored_in_working_copy() {
+    let 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");
+
+    // Create a conflict in the working copy with long markers on one side
+    let conflict_file = repo_path.join("file");
+    std::fs::write(
+        &conflict_file,
+        indoc! {"
+            line 1
+            line 2
+            line 3
+        "},
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "base"]);
+    std::fs::write(
+        &conflict_file,
+        indoc! {"
+            line 1
+            line 2 - left
+            line 3 - left
+        "},
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "side-a"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "description(base)", "-m", "side-b"]);
+    std::fs::write(
+        &conflict_file,
+        indoc! {"
+            line 1
+            ======= fake marker
+            line 2 - right
+            ======= fake marker
+            line 3
+        "},
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &["new", "description(side-a)", "description(side-b)"],
+    );
+
+    // File should be materialized with long conflict markers
+    insta::assert_snapshot!(std::fs::read_to_string(&conflict_file).unwrap(), @r##"
+    line 1
+    <<<<<<<<<<< Conflict 1 of 1
+    %%%%%%%%%%% Changes from base to side #1
+    -line 2
+    -line 3
+    +line 2 - left
+    +line 3 - left
+    +++++++++++ Contents of side #2
+    ======= fake marker
+    line 2 - right
+    ======= fake marker
+    line 3
+    >>>>>>>>>>> Conflict 1 of 1 ends
+    "##);
+
+    // The timestamps in the `jj debug local-working-copy` output change, so we want
+    // to remove them before asserting the snapshot
+    let timestamp_regex = Regex::new(r"\b\d{10,}\b").unwrap();
+    // On Windows, executable is always `()`, but on Unix-like systems, it's `true`
+    // or `false`, so we want to remove it from the output as well
+    let executable_regex = Regex::new("executable: [^ ]+").unwrap();
+
+    let redact_output = |output: &str| {
+        let output = timestamp_regex.replace_all(output, "<timestamp>");
+        let output = executable_regex.replace_all(&output, "<executable>");
+        output.into_owned()
+    };
+
+    // Working copy should contain conflict marker length
+    let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "local-working-copy"]);
+    insta::assert_snapshot!(redact_output(&stdout), @r#"
+    Current operation: OperationId("6feb53603f9f7324085d2d89dca19a6dac93fef6795cfd5d57090ff803d404ab1196b45d5b97faa641f6a78302ac0fbd149f5e5a880d1fd64d6520c31beab213")
+    Current tree: Merge(Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("f56b8223da0dab22b03b8323ced4946329aeb4e0")]))
+    Normal { <executable> }           249 <timestamp> Some(MaterializedConflictData { conflict_marker_len: 11 }) "file"
+    "#);
+
+    // Update the conflict with more fake markers, and it should still parse
+    // correctly (the markers should be ignored)
+    std::fs::write(
+        &conflict_file,
+        indoc! {"
+            line 1
+            <<<<<<<<<<< Conflict 1 of 1
+            %%%%%%%%%%% Changes from base to side #1
+            -line 2
+            -line 3
+            +line 2 - left
+            +line 3 - left
+            +++++++++++ Contents of side #2
+            <<<<<<< fake marker
+            ||||||| fake marker
+            line 2 - right
+            ======= fake marker
+            line 3
+            >>>>>>> fake marker
+            >>>>>>>>>>> Conflict 1 of 1 ends
+        "},
+    )
+    .unwrap();
+
+    // The file should still be conflicted, and the new content should be saved
+    let stdout = test_env.jj_cmd_success(&repo_path, &["st"]);
+    insta::assert_snapshot!(stdout, @r#"
+    Working copy changes:
+    M file
+    There are unresolved conflicts at these paths:
+    file    2-sided conflict
+    Working copy : mzvwutvl 3a981880 (conflict) (no description set)
+    Parent commit: rlvkpnrz ce613b49 side-a
+    Parent commit: zsuskuln 7b2b03ab side-b
+    "#);
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r##"
+    diff --git a/file b/file
+    --- a/file
+    +++ b/file
+    @@ -6,8 +6,10 @@
+     +line 2 - left
+     +line 3 - left
+     +++++++++++ Contents of side #2
+    -======= fake marker
+    +<<<<<<< fake marker
+    +||||||| fake marker
+     line 2 - right
+     ======= fake marker
+     line 3
+    +>>>>>>> fake marker
+     >>>>>>>>>>> Conflict 1 of 1 ends
+    "##);
+
+    // Working copy should still contain conflict marker length
+    let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "local-working-copy"]);
+    insta::assert_snapshot!(redact_output(&stdout), @r#"
+    Current operation: OperationId("205bc702428a522e0b175938a51c51b59741c854a609ba63c89de76ffda6e5eff6fcc00725328b1a91f448401769773cefcff01fac3448190d2cea4e137d2166")
+    Current tree: Merge(Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("3329c18c95f7b7a55c278c2259e9c4ce711fae59")]))
+    Normal { <executable> }           289 <timestamp> Some(MaterializedConflictData { conflict_marker_len: 11 }) "file"
+    "#);
+
+    // Resolve the conflict
+    std::fs::write(
+        &conflict_file,
+        indoc! {"
+            line 1
+            <<<<<<< fake marker
+            ||||||| fake marker
+            line 2 - left
+            line 2 - right
+            ======= fake marker
+            line 3 - left
+            >>>>>>> fake marker
+        "},
+    )
+    .unwrap();
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["st"]);
+    insta::assert_snapshot!(stdout, @r#"
+    Working copy changes:
+    M file
+    Working copy : mzvwutvl 1aefd866 (no description set)
+    Parent commit: rlvkpnrz ce613b49 side-a
+    Parent commit: zsuskuln 7b2b03ab side-b
+    "#);
+
+    // When the file is resolved, the conflict marker length is removed from the
+    // working copy
+    let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "local-working-copy"]);
+    insta::assert_snapshot!(redact_output(&stdout), @r#"
+    Current operation: OperationId("2206ce3c108b1573df0841138c226bba1ab3cff900a5899ed31ac69162c7d6f30d37fb5ab43da60dba88047b8ab22d453887fff688f26dfcf04f2c99420a5563")
+    Current tree: Merge(Resolved(TreeId("6120567b3cb2472d549753ed3e4b84183d52a650")))
+    Normal { <executable> }           130 <timestamp> None "file"
+    "#);
+}