conflicts: avoid long markers for Git-style conflicts

Git materializes conflicts with 7-character markers even if the file
already contains conflict markers, so it doesn't seem necessary to
support generating Git-style conflicts with markers longer than 7
characters. This also means that files which use "=======" as a header
(e.g. some titles in Markdown) won't require long conflict markers.

lib/src/conflicts.rs

@@ -263,6 +263,14 @@ impl ConflictMarkerKind {
             _ => None,
         }
     }
+
+    /// Returns true if the conflict marker is allowed to be longer than
+    /// MIN_CONFLICT_MARKER_LEN. We don't allow Git-style conflict markers to be
+    /// long, which reduces the number of situations where long conflict markers
+    /// need to be used (since "=======" is a common separator line in files).
+    fn allows_long_markers(&self) -> bool {
+        !matches!(self, Self::GitAncestor | Self::GitSeparator)
+    }
 }
 
 /// Represents a conflict marker parsed from the file. Conflict markers consist
@@ -280,6 +288,7 @@ fn write_conflict_marker(
     suffix_text: &str,
 ) -> io::Result<()> {
     debug_assert!(len >= MIN_CONFLICT_MARKER_LEN);
+    debug_assert!(kind.allows_long_markers() || len == MIN_CONFLICT_MARKER_LEN);
 
     let conflict_marker: BString = iter::repeat(kind.to_byte()).take(len).collect();
 
@@ -297,6 +306,11 @@ fn parse_conflict_marker_any_len(line: &[u8]) -> Option<ConflictMarker> {
     let kind = ConflictMarkerKind::parse_byte(first_byte)?;
     let len = line.iter().take_while(|&&b| b == first_byte).count();
 
+    // Some conflict markers must be not be longer than MIN_CONFLICT_MARKER_LEN
+    if len > MIN_CONFLICT_MARKER_LEN && !kind.allows_long_markers() {
+        return None;
+    }
+
     if let Some(next_byte) = line.get(len) {
         // If there is a character after the marker, it must be ASCII whitespace
         if !next_byte.is_ascii_whitespace() {
@@ -409,16 +423,12 @@ fn materialize_conflict_hunks(
             let conflict_info = format!("Conflict {conflict_index} of {num_conflicts}");
 
             match (conflict_marker_style, hunk.as_slice()) {
-                // 2-sided conflicts can use Git-style conflict markers
-                (ConflictMarkerStyle::Git, [left, base, right]) => {
-                    materialize_git_style_conflict(
-                        left,
-                        base,
-                        right,
-                        &conflict_info,
-                        conflict_marker_len,
-                        output,
-                    )?;
+                // 2-sided conflicts can use Git-style conflict markers, but the markers cannot be
+                // longer than MIN_CONFLICT_MARKER_LEN
+                (ConflictMarkerStyle::Git, [left, base, right])
+                    if conflict_marker_len == MIN_CONFLICT_MARKER_LEN =>
+                {
+                    materialize_git_style_conflict(left, base, right, &conflict_info, output)?;
                 }
                 _ => {
                     materialize_jj_style_conflict(
@@ -440,35 +450,34 @@ fn materialize_git_style_conflict(
     base: &[u8],
     right: &[u8],
     conflict_info: &str,
-    conflict_marker_len: usize,
     output: &mut dyn Write,
 ) -> io::Result<()> {
     write_conflict_marker(
         output,
         ConflictMarkerKind::ConflictStart,
-        conflict_marker_len,
+        MIN_CONFLICT_MARKER_LEN,
         &format!("Side #1 ({conflict_info})"),
     )?;
     output.write_all(left)?;
     write_conflict_marker(
         output,
         ConflictMarkerKind::GitAncestor,
-        conflict_marker_len,
+        MIN_CONFLICT_MARKER_LEN,
         "Base",
     )?;
     output.write_all(base)?;
     // VS Code doesn't seem to support any trailing text on the separator line
     write_conflict_marker(
         output,
         ConflictMarkerKind::GitSeparator,
-        conflict_marker_len,
+        MIN_CONFLICT_MARKER_LEN,
         "",
     )?;
     output.write_all(right)?;
     write_conflict_marker(
         output,
         ConflictMarkerKind::ConflictEnd,
-        conflict_marker_len,
+        MIN_CONFLICT_MARKER_LEN,
         &format!("Side #2 ({conflict_info} ends)"),
     )?;
 

lib/tests/test_conflicts.rs

@@ -1839,42 +1839,48 @@ fn test_update_conflict_from_content_with_long_markers() {
             <<<< left 1
             line 2
             <<<<<<<<<<<< left 3
+            ======================== git conflict markers can't be long
         "},
     );
     let right_file_id = testutils::write_file(
         store,
         path,
         indoc! {"
-            >>>>>>> right 1
+            ======= right 1
             line 2
             >>>>>>>>>>>> right 3
+            |||||||||||||||||||||||| git conflict markers can't be long
         "},
     );
     let conflict = Merge::from_removes_adds(
         vec![Some(base_file_id.clone())],
         vec![Some(left_file_id.clone()), Some(right_file_id.clone())],
     );
 
-    // The conflict should be materialized using long conflict markers
+    // The conflict should be materialized using long conflict markers ("git"
+    // conflict markers should fall back to "snapshot", since "git" doesn't support
+    // long conflict markers)
     let materialized =
-        materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Snapshot);
+        materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Git);
     insta::assert_snapshot!(materialized, @r##"
     <<<<<<<<<<<<<<<< Conflict 1 of 2
     ++++++++++++++++ Contents of side #1
     <<<< left 1
     ---------------- Contents of base
     line 1
     ++++++++++++++++ Contents of side #2
-    >>>>>>> right 1
+    ======= right 1
     >>>>>>>>>>>>>>>> Conflict 1 of 2 ends
     line 2
     <<<<<<<<<<<<<<<< Conflict 2 of 2
     ++++++++++++++++ Contents of side #1
     <<<<<<<<<<<< left 3
+    ======================== git conflict markers can't be long
     ---------------- Contents of base
     line 3
     ++++++++++++++++ Contents of side #2
     >>>>>>>>>>>> right 3
+    |||||||||||||||||||||||| git conflict markers can't be long
     >>>>>>>>>>>>>>>> Conflict 2 of 2 ends
     "## );
 
@@ -1914,7 +1920,7 @@ fn test_update_conflict_from_content_with_long_markers() {
         ---------------- Contents of base
         line 1
         ++++++++++++++++ Contents of side #2
-        >>>>>>> right 1
+        ======= right 1
         >>>>>>>>>>>>>>>> Conflict 1 of 2 ends
         line 2
         line 3
@@ -1933,7 +1939,7 @@ fn test_update_conflict_from_content_with_long_markers() {
         store,
         path,
         indoc! {"
-            >>>>>>> right 1
+            ======= right 1
             line 2
             line 3
         "},
@@ -1961,8 +1967,9 @@ fn test_update_conflict_from_content_with_long_markers() {
     // still, since they aren't the longest ones in the file).
     assert_eq!(parse(&new_conflict, materialized.as_bytes()), conflict);
 
-    // If the new conflict is materialized again, it should have shorter
-    // conflict markers now
+    // If the new conflict is materialized again, it should have shorter conflict
+    // markers. The conflict markers still need to be longer than normal though,
+    // since "=======" is a valid Git-style conflict marker of length 7.
     insta::assert_snapshot!(
         materialize_conflict_string(store, path, &new_conflict, ConflictMarkerStyle::Snapshot),
         @r##"
@@ -1972,7 +1979,7 @@ fn test_update_conflict_from_content_with_long_markers() {
     ----------- Contents of base
     line 1
     +++++++++++ Contents of side #2
-    >>>>>>> right 1
+    ======= right 1
     >>>>>>>>>>> Conflict 1 of 1 ends
     line 2
     line 3