local_working_copy: store materialized conflict marker length

Storing the conflict marker length in the working copy makes conflict
parsing more consistent, and it allows us to parse valid conflict hunks
even if the user left some invalid conflict markers in the file while
resolving the conflicts.

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;
 
@@ -253,3 +254,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("da133d2605b63b84c53b512007b32bd5822e4821d7f8ca69b03a0bbd702cd61fad7857e430e911011aaecf3bf6942e81a95180792c7e0056af18bc956ee834a4")
+    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 b7dadc87 (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("65b1b6a0da226e45694fda78d85efa5397176204b166f107b10c8ac0dcecfcfa9346b59317d8b572711666a3e5f168bcb561c278095a83363885911e246b2230")
+    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("6dc38b23e076d05a7c80327559e6de48d2fbc0811b06e9319bdbbff392bc991385e1ecbc378613101ba862e07dad1e6703247c5239a5a672a4761411815fe9fa")
+    Current tree: Merge(Resolved(TreeId("6120567b3cb2472d549753ed3e4b84183d52a650")))
+    Normal { <executable> }           130 <timestamp> None "file"
+    "#);
+}

lib/src/conflicts.rs

@@ -344,7 +344,7 @@ pub fn materialize_merge_result<T: AsRef<[u8]>>(
     }
 }
 
-fn materialize_merge_result_with_marker_len<T: AsRef<[u8]>>(
+pub fn materialize_merge_result_with_marker_len<T: AsRef<[u8]>>(
     single_hunk: &Merge<T>,
     conflict_marker_style: ConflictMarkerStyle,
     conflict_marker_len: usize,
@@ -381,6 +381,28 @@ pub fn materialize_merge_result_to_bytes<T: AsRef<[u8]>>(
     }
 }
 
+pub fn materialize_merge_result_to_bytes_with_marker_len<T: AsRef<[u8]>>(
+    single_hunk: &Merge<T>,
+    conflict_marker_style: ConflictMarkerStyle,
+    conflict_marker_len: usize,
+) -> BString {
+    let merge_result = files::merge(single_hunk);
+    match merge_result {
+        MergeResult::Resolved(content) => content,
+        MergeResult::Conflict(hunks) => {
+            let mut output = Vec::new();
+            materialize_conflict_hunks(
+                &hunks,
+                conflict_marker_style,
+                conflict_marker_len,
+                &mut output,
+            )
+            .expect("writing to an in-memory buffer should never fail");
+            output.into()
+        }
+    }
+}
+
 fn materialize_conflict_hunks(
     hunks: &[Merge<BString>],
     conflict_marker_style: ConflictMarkerStyle,
@@ -824,6 +846,7 @@ pub async fn update_from_content(
     path: &RepoPath,
     content: &[u8],
     conflict_marker_style: ConflictMarkerStyle,
+    conflict_marker_len: usize,
 ) -> BackendResult<Merge<Option<FileId>>> {
     let simplified_file_ids = file_ids.clone().simplify();
     let simplified_file_ids = &simplified_file_ids;
@@ -835,7 +858,6 @@ pub async fn update_from_content(
     // copy.
     let mut old_content = Vec::with_capacity(content.len());
     let merge_hunk = extract_as_single_hunk(simplified_file_ids, store, path).await?;
-    let conflict_marker_len = choose_materialized_conflict_marker_len(&merge_hunk);
     materialize_merge_result_with_marker_len(
         &merge_hunk,
         conflict_marker_style,