From 9f0be3b010e51b91ce7b596de6ee46f8582810a8 Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sun, 24 Nov 2024 20:46:56 -0600 Subject: [PATCH] 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 | 39 +++++++++++++++++++++++-------------- lib/tests/test_conflicts.rs | 25 +++++++++++++++--------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index e47eed543d..b792f31627 100644 --- a/lib/src/conflicts.rs +++ b/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 { 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,20 +450,19 @@ 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)?; @@ -461,14 +470,14 @@ fn materialize_git_style_conflict( 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)"), )?; diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index e1b82b202b..d6cd5ba35d 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -1839,15 +1839,17 @@ 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( @@ -1855,9 +1857,11 @@ fn test_update_conflict_from_content_with_long_markers() { 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 @@ -1865,16 +1869,18 @@ 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 <<<<<<<<<<<<<<<< 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