From 392188db90ada75ddf53277de4a5e7a6393d31fb Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sun, 24 Nov 2024 15:20:18 -0600 Subject: [PATCH] conflicts: escape conflict markers by making them longer If a file contains lines which look like conflict markers, then we need to make the real conflict markers longer so that the materialized conflicts can be parsed unambiguously. When parsing the conflict, we require that the conflict markers in the file are at least as long as the materialized conflict markers, and we only parse the longest conflict markers in the file. For instance, if we have a file explaining the differences between Jujutsu's conflict markers and Git's conflict markers, it could produce a conflict with long markers like this: ``` <<<<<<<<<<< Conflict 1 of 1 %%%%%%%%%%% Changes from base to side #1 Jujutsu uses different conflict markers than Git, which just shows the -sides of a conflict without a diff. +sides of a conflict without a diff: + +<<<<<<< +left +||||||| +base +======= +right +>>>>>>> +++++++++++ Contents of side #2 Jujutsu uses different conflict markers than Git: <<<<<<< %%%%%%% -base +left +++++++ right >>>>>>> >>>>>>>>>>> Conflict 1 of 1 ends ``` --- CHANGELOG.md | 4 + lib/src/conflicts.rs | 231 +++++++++++++++++++++++-------- lib/tests/test_conflicts.rs | 262 +++++++++++++++++++++++++++++------- 3 files changed, 398 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3892782529..aa14182a5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `op diff` - `restore` +* 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. + * New `ui.conflict-marker-style` config option to change how conflicts are materialized in the working copy. The default option ("diff") renders conflicts as a snapshot with a list of diffs to apply to the snapshot. diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 75c38dc75c..e47eed543d 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -50,8 +50,14 @@ use crate::merge::MergedTreeValue; use crate::repo_path::RepoPath; use crate::store::Store; -/// Length of conflict markers. -pub const CONFLICT_MARKER_LEN: usize = 7; +/// Minimum length of conflict markers. +pub const MIN_CONFLICT_MARKER_LEN: usize = 7; + +/// If a file already contains lines which look like conflict markers of length +/// N, then the conflict markers we add will be of length (N + increment). This +/// number is chosen to make the conflict markers noticeably longer than the +/// existing markers. +pub const CONFLICT_MARKER_LEN_INCREMENT: usize = 4; fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> io::Result<()> { for hunk in hunks { @@ -259,15 +265,23 @@ impl ConflictMarkerKind { } } +/// Represents a conflict marker parsed from the file. Conflict markers consist +/// of a single ASCII character repeated for a certain length. +struct ConflictMarker { + kind: ConflictMarkerKind, + len: usize, +} + /// Write a conflict marker to an output file. fn write_conflict_marker( output: &mut dyn Write, kind: ConflictMarkerKind, + len: usize, suffix_text: &str, ) -> io::Result<()> { - let conflict_marker: BString = iter::repeat(kind.to_byte()) - .take(CONFLICT_MARKER_LEN) - .collect(); + debug_assert!(len >= MIN_CONFLICT_MARKER_LEN); + + let conflict_marker: BString = iter::repeat(kind.to_byte()).take(len).collect(); if suffix_text.is_empty() { writeln!(output, "{conflict_marker}") @@ -276,17 +290,13 @@ fn write_conflict_marker( } } -/// Parse a conflict marker from a line of a file. The conflict marker must have -/// the correct length (CONFLICT_MARKER_LEN). -fn parse_conflict_marker(line: &[u8]) -> Option { +/// Parse a conflict marker from a line of a file. The conflict marker may have +/// any length (even less than MIN_CONFLICT_MARKER_LEN). +fn parse_conflict_marker_any_len(line: &[u8]) -> Option { let first_byte = *line.first()?; let kind = ConflictMarkerKind::parse_byte(first_byte)?; let len = line.iter().take_while(|&&b| b == first_byte).count(); - if len != CONFLICT_MARKER_LEN { - 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() { @@ -294,19 +304,66 @@ fn parse_conflict_marker(line: &[u8]) -> Option { } } - Some(kind) + Some(ConflictMarker { kind, len }) +} + +/// Parse a conflict marker, expecting it to have a certain length. Any conflict +/// markers which have a different length are ignored. +fn parse_conflict_marker(line: &[u8], expected_len: usize) -> Option { + debug_assert!(expected_len >= MIN_CONFLICT_MARKER_LEN); + + parse_conflict_marker_any_len(line) + .filter(|marker| marker.len == expected_len) + .map(|marker| marker.kind) +} + +/// Find the longest conflict marker in a file. +pub fn find_max_conflict_marker_len(file: &[u8]) -> usize { + file.lines_with_terminator() + .filter_map(parse_conflict_marker_any_len) + .map(|marker| marker.len) + .max() + .unwrap_or_default() } +/// Choose an appropriate conflict marker length to use for a file. The conflict +/// marker length will be at least MIN_CONFLICT_MARKER_LENGTH, and is guaranteed +/// to be longer than any existing markers in the file. +pub fn choose_conflict_marker_len>(single_hunk: &Merge) -> usize { + let max_existing_marker_len = single_hunk + .iter() + .map(|term| find_max_conflict_marker_len(term.as_ref())) + .max() + .unwrap_or_default(); + + if max_existing_marker_len < MIN_CONFLICT_MARKER_LEN { + // Use the normal conflict marker length if it's unambiguous. + MIN_CONFLICT_MARKER_LEN + } else { + // If there are existing markers in the file, we want to make our conflict + // markers longer than them. We add CONFLICT_MARKER_LEN_INCREMENT to ensure + // they're longer by a visually noticeable amount. + max_existing_marker_len.saturating_add(CONFLICT_MARKER_LEN_INCREMENT) + } +} + +// Returns conflict marker length used for materializing conflicts, if there +// were any conflicts to materialize. pub fn materialize_merge_result>( single_hunk: &Merge, conflict_marker_style: ConflictMarkerStyle, output: &mut dyn Write, -) -> io::Result<()> { +) -> io::Result> { let merge_result = files::merge(single_hunk); match &merge_result { - MergeResult::Resolved(content) => output.write_all(content), + MergeResult::Resolved(content) => { + output.write_all(content)?; + Ok(None) + } MergeResult::Conflict(hunks) => { - materialize_conflict_hunks(hunks, conflict_marker_style, output) + let conflict_marker_len = choose_conflict_marker_len(single_hunk); + materialize_conflict_hunks(hunks, conflict_marker_style, conflict_marker_len, output)?; + Ok(Some(conflict_marker_len)) } } } @@ -319,9 +376,15 @@ pub fn materialize_merge_result_to_bytes>( match merge_result { MergeResult::Resolved(content) => content, MergeResult::Conflict(hunks) => { + let conflict_marker_len = choose_conflict_marker_len(single_hunk); let mut output = Vec::new(); - materialize_conflict_hunks(&hunks, conflict_marker_style, &mut output) - .expect("writing to an in-memory buffer should never fail"); + materialize_conflict_hunks( + &hunks, + conflict_marker_style, + conflict_marker_len, + &mut output, + ) + .expect("writing to an in-memory buffer should never fail"); output.into() } } @@ -330,6 +393,7 @@ pub fn materialize_merge_result_to_bytes>( fn materialize_conflict_hunks( hunks: &[Merge], conflict_marker_style: ConflictMarkerStyle, + conflict_marker_len: usize, output: &mut dyn Write, ) -> io::Result<()> { let num_conflicts = hunks @@ -347,13 +411,21 @@ fn materialize_conflict_hunks( 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, output)?; + materialize_git_style_conflict( + left, + base, + right, + &conflict_info, + conflict_marker_len, + output, + )?; } _ => { materialize_jj_style_conflict( hunk, &conflict_info, conflict_marker_style, + conflict_marker_len, output, )?; } @@ -368,22 +440,35 @@ 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, &format!("Side #1 ({conflict_info})"), )?; output.write_all(left)?; - write_conflict_marker(output, ConflictMarkerKind::GitAncestor, "Base")?; + write_conflict_marker( + output, + ConflictMarkerKind::GitAncestor, + 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, "")?; + write_conflict_marker( + output, + ConflictMarkerKind::GitSeparator, + conflict_marker_len, + "", + )?; output.write_all(right)?; write_conflict_marker( output, ConflictMarkerKind::ConflictEnd, + conflict_marker_len, &format!("Side #2 ({conflict_info} ends)"), )?; @@ -394,44 +479,49 @@ fn materialize_jj_style_conflict( hunk: &Merge, conflict_info: &str, conflict_marker_style: ConflictMarkerStyle, + conflict_marker_len: usize, output: &mut dyn Write, ) -> io::Result<()> { // Write a positive snapshot (side) of a conflict - fn write_side(add_index: usize, data: &[u8], output: &mut dyn Write) -> io::Result<()> { + let write_side = |add_index: usize, data: &[u8], output: &mut dyn Write| { write_conflict_marker( output, ConflictMarkerKind::Add, + conflict_marker_len, &format!("Contents of side #{}", add_index + 1), )?; output.write_all(data) - } + }; // Write a negative snapshot (base) of a conflict - fn write_base(base_str: &str, data: &[u8], output: &mut dyn Write) -> io::Result<()> { + let write_base = |base_str: &str, data: &[u8], output: &mut dyn Write| { write_conflict_marker( output, ConflictMarkerKind::Remove, + conflict_marker_len, &format!("Contents of {base_str}"), )?; output.write_all(data) - } + }; // Write a diff from a negative term to a positive term - fn write_diff( - base_str: &str, - add_index: usize, - diff: &[DiffHunk], - output: &mut dyn Write, - ) -> io::Result<()> { - write_conflict_marker( - output, - ConflictMarkerKind::Diff, - &format!("Changes from {base_str} to side #{}", add_index + 1), - )?; - write_diff_hunks(diff, output) - } + let write_diff = + |base_str: &str, add_index: usize, diff: &[DiffHunk], output: &mut dyn Write| { + write_conflict_marker( + output, + ConflictMarkerKind::Diff, + conflict_marker_len, + &format!("Changes from {base_str} to side #{}", add_index + 1), + )?; + write_diff_hunks(diff, output) + }; - write_conflict_marker(output, ConflictMarkerKind::ConflictStart, conflict_info)?; + write_conflict_marker( + output, + ConflictMarkerKind::ConflictStart, + conflict_marker_len, + conflict_info, + )?; let mut add_index = 0; for (base_index, left) in hunk.removes().enumerate() { // The vast majority of conflicts one actually tries to resolve manually have 1 @@ -484,6 +574,7 @@ fn materialize_jj_style_conflict( write_conflict_marker( output, ConflictMarkerKind::ConflictEnd, + conflict_marker_len, &format!("{conflict_info} ends"), )?; Ok(()) @@ -532,9 +623,16 @@ pub fn materialized_diff_stream<'a>( /// has to provide the expected number of merge sides (adds). Conflict /// markers that are otherwise valid will be considered invalid if /// they don't have the expected arity. +/// +/// All conflict markers in the file must have the expected length. Any conflict +/// markers with a different length will be ignored. // TODO: "parse" is not usually the opposite of "materialize", so maybe we // should rename them to "serialize" and "deserialize"? -pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option>> { +pub fn parse_conflict( + input: &[u8], + num_sides: usize, + conflict_marker_len: usize, +) -> Option>> { if input.is_empty() { return None; } @@ -544,7 +642,7 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option { conflict_start = Some(pos); conflict_start_len = line.len(); @@ -552,7 +650,7 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option { if let Some(conflict_start_index) = conflict_start { let conflict_body = &input[conflict_start_index + conflict_start_len..pos]; - let hunk = parse_conflict_hunk(conflict_body); + let hunk = parse_conflict_hunk(conflict_body, conflict_marker_len); if hunk.num_sides() == num_sides { let resolved_slice = &input[resolved_start..conflict_start_index]; if !resolved_slice.is_empty() { @@ -584,27 +682,29 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option Merge { +fn parse_conflict_hunk(input: &[u8], conflict_marker_len: usize) -> Merge { // If the hunk starts with a conflict marker, find its first character let initial_conflict_marker = input .lines_with_terminator() .next() - .and_then(parse_conflict_marker); + .and_then(|line| parse_conflict_marker(line, conflict_marker_len)); match initial_conflict_marker { // JJ-style conflicts must start with one of these 3 conflict marker lines Some(ConflictMarkerKind::Diff | ConflictMarkerKind::Remove | ConflictMarkerKind::Add) => { - parse_jj_style_conflict_hunk(input) + parse_jj_style_conflict_hunk(input, conflict_marker_len) } // Git-style conflicts either must not start with a conflict marker line, or must start with // the "|||||||" conflict marker line (if the first side was empty) - None | Some(ConflictMarkerKind::GitAncestor) => parse_git_style_conflict_hunk(input), + None | Some(ConflictMarkerKind::GitAncestor) => { + parse_git_style_conflict_hunk(input, conflict_marker_len) + } // No other conflict markers are allowed at the start of a hunk Some(_) => Merge::resolved(BString::new(vec![])), } } -fn parse_jj_style_conflict_hunk(input: &[u8]) -> Merge { +fn parse_jj_style_conflict_hunk(input: &[u8], conflict_marker_len: usize) -> Merge { enum State { Diff, Remove, @@ -615,7 +715,7 @@ fn parse_jj_style_conflict_hunk(input: &[u8]) -> Merge { let mut removes = vec![]; let mut adds = vec![]; for line in input.lines_with_terminator() { - match parse_conflict_marker(line) { + match parse_conflict_marker(line, conflict_marker_len) { Some(ConflictMarkerKind::Diff) => { state = State::Diff; removes.push(BString::new(vec![])); @@ -675,7 +775,7 @@ fn parse_jj_style_conflict_hunk(input: &[u8]) -> Merge { } } -fn parse_git_style_conflict_hunk(input: &[u8]) -> Merge { +fn parse_git_style_conflict_hunk(input: &[u8], conflict_marker_len: usize) -> Merge { #[derive(PartialEq, Eq)] enum State { Left, @@ -687,7 +787,7 @@ fn parse_git_style_conflict_hunk(input: &[u8]) -> Merge { let mut base = BString::new(vec![]); let mut right = BString::new(vec![]); for line in input.lines_with_terminator() { - match parse_conflict_marker(line) { + match parse_conflict_marker(line, conflict_marker_len) { Some(ConflictMarkerKind::GitAncestor) => { if state == State::Left { state = State::Base; @@ -743,21 +843,46 @@ 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?; - materialize_merge_result(&merge_hunk, conflict_marker_style, &mut old_content).unwrap(); + let materialized_marker_len = + materialize_merge_result(&merge_hunk, conflict_marker_style, &mut old_content).unwrap(); if content == old_content { return Ok(file_ids.clone()); } + let max_content_marker_len = find_max_conflict_marker_len(content); + + // If all conflict markers in the file content are shorter than the materialized + // conflict markers, then the file is considered resolved. + if max_content_marker_len < materialized_marker_len.unwrap_or(MIN_CONFLICT_MARKER_LEN) { + let file_id = store.write_file(path, &mut &content[..]).await?; + return Ok(Merge::normal(file_id)); + } + + // Only accept the longest conflict markers in the file, also requiring that + // they are at least as long as the materialized conflict markers. We need to + // allow conflict markers to be longer than the materialized conflict markers, + // since the user could have removed the "fake" conflict markers from the file + // while editing, in which case their existing conflict markers will be + // considered longer than necessary the next time the file is snapshotted. + let expected_marker_len = materialized_marker_len + .unwrap_or_else(|| choose_conflict_marker_len(&merge_hunk)) + .max(max_content_marker_len); + // Parse conflicts from the new content using the arity of the simplified // conflicts initially. If unsuccessful, attempt to parse conflicts from with // the arity of the unsimplified conflicts since such a conflict may be // present in the working copy if written by an earlier version of jj. let (used_file_ids, hunks) = 'hunks: { - if let Some(hunks) = parse_conflict(content, simplified_file_ids.num_sides()) { + if let Some(hunks) = parse_conflict( + content, + simplified_file_ids.num_sides(), + expected_marker_len, + ) { break 'hunks (simplified_file_ids, hunks); }; if simplified_file_ids.num_sides() != file_ids.num_sides() { - if let Some(hunks) = parse_conflict(content, file_ids.num_sides()) { + if let Some(hunks) = parse_conflict(content, file_ids.num_sides(), expected_marker_len) + { break 'hunks (file_ids, hunks); }; }; diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index e3c4a00f6f..e1b82b202b 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -19,6 +19,7 @@ use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::parse_conflict; use jj_lib::conflicts::update_from_content; use jj_lib::conflicts::ConflictMarkerStyle; +use jj_lib::conflicts::MIN_CONFLICT_MARKER_LEN; use jj_lib::merge::Merge; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; @@ -505,7 +506,7 @@ fn test_materialize_parse_roundtrip() { // The first add should always be from the left side insta::assert_debug_snapshot!( - parse_conflict(materialized.as_bytes(), conflict.num_sides()), + parse_conflict(materialized.as_bytes(), conflict.num_sides(), MIN_CONFLICT_MARKER_LEN), @r###" Some( [ @@ -628,7 +629,8 @@ fn test_materialize_conflict_no_newlines_at_eof() { // BUG(#3968): These conflict markers cannot be parsed insta::assert_debug_snapshot!(parse_conflict( materialized.as_bytes(), - conflict.num_sides() + conflict.num_sides(), + MIN_CONFLICT_MARKER_LEN ),@"None"); } @@ -795,7 +797,8 @@ line 3 line 4 line 5 "}, - 2 + 2, + 7 ), None ); @@ -817,7 +820,8 @@ fn test_parse_conflict_simple() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), @r###" Some( @@ -853,7 +857,8 @@ fn test_parse_conflict_simple() { >>>>>>> More and more text line 5 "}, - 2 + 2, + 7 ), @r###" Some( @@ -892,7 +897,8 @@ fn test_parse_conflict_simple() { >>>>>>> Random text line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -931,7 +937,8 @@ fn test_parse_conflict_simple() { >>>>>>> Random text line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -969,7 +976,8 @@ fn test_parse_conflict_simple() { >>>>>>> End line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1005,7 +1013,8 @@ fn test_parse_conflict_simple() { >>>>>>> End line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1027,26 +1036,6 @@ fn test_parse_conflict_simple() { ) "# ); - // The conflict markers are too long and shouldn't parse (though we may - // decide to change this in the future) - insta::assert_debug_snapshot!( - parse_conflict(indoc! {b" - line 1 - <<<<<<<<<<< - %%%%%%%%%%% - line 2 - -line 3 - +left - line 4 - +++++++++++ - right - >>>>>>>>>>> - line 5 - "}, - 2 - ), - @"None" - ); } #[test] @@ -1071,7 +1060,8 @@ fn test_parse_conflict_multi_way() { >>>>>>> line 5 "}, - 3 + 3, + 7 ), @r###" Some( @@ -1114,7 +1104,8 @@ fn test_parse_conflict_multi_way() { >>>>>>> Random text line 5 "}, - 3 + 3, + 7 ), @r###" Some( @@ -1160,7 +1151,8 @@ fn test_parse_conflict_multi_way() { >>>>>>> Random text line 5 "}, - 3 + 3, + 7 ), @r#" Some( @@ -1203,7 +1195,8 @@ fn test_parse_conflict_crlf_markers() { >>>>>>>\r line 5\r "}, - 2 + 2, + 7 ), @r#" Some( @@ -1248,7 +1241,8 @@ fn test_parse_conflict_diff_stripped_whitespace() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1290,7 +1284,8 @@ fn test_parse_conflict_wrong_arity() { >>>>>>> line 5 "}, - 3 + 3, + 7 ), None ); @@ -1311,7 +1306,8 @@ fn test_parse_conflict_malformed_missing_removes() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1334,7 +1330,8 @@ fn test_parse_conflict_malformed_marker() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1358,7 +1355,8 @@ fn test_parse_conflict_malformed_diff() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1380,7 +1378,8 @@ fn test_parse_conflict_snapshot_missing_header() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1400,7 +1399,8 @@ fn test_parse_conflict_wrong_git_style() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1422,7 +1422,8 @@ fn test_parse_conflict_git_reordered_headers() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1448,7 +1449,8 @@ fn test_parse_conflict_git_too_many_sides() { >>>>>>> line 5 "}, - 3 + 3, + 7 ), None ); @@ -1471,7 +1473,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1489,7 +1492,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1506,7 +1510,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> Conflict 1 of 1 ends line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1541,7 +1546,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> Side #2 (Conflict 1 of 1 ends) line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1810,6 +1816,170 @@ fn test_update_conflict_from_content_simplified_conflict() { ); } +#[test] +fn test_update_conflict_from_content_with_long_markers() { + let test_repo = TestRepo::init(); + let store = test_repo.repo.store(); + + // Create conflicts which contain conflict markers of varying lengths + let path = RepoPath::from_internal_string("dir/file"); + let base_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 + line 3 + "}, + ); + let left_file_id = testutils::write_file( + store, + path, + indoc! {" + <<<< left 1 + line 2 + <<<<<<<<<<<< left 3 + "}, + ); + let right_file_id = testutils::write_file( + store, + path, + indoc! {" + >>>>>>> right 1 + line 2 + >>>>>>>>>>>> right 3 + "}, + ); + 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 + let materialized = + materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Snapshot); + 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 + >>>>>>>>>>>>>>>> Conflict 1 of 2 ends + line 2 + <<<<<<<<<<<<<<<< Conflict 2 of 2 + ++++++++++++++++ Contents of side #1 + <<<<<<<<<<<< left 3 + ---------------- Contents of base + line 3 + ++++++++++++++++ Contents of side #2 + >>>>>>>>>>>> right 3 + >>>>>>>>>>>>>>>> Conflict 2 of 2 ends + "## ); + + // Parse the conflict markers using a different conflict marker style. This is + // to avoid the two versions of the file being obviously identical, so that we + // can test the actual parsing logic. + let parse = |conflict, content| { + update_from_content(conflict, store, path, content, ConflictMarkerStyle::Diff) + .block_on() + .unwrap() + }; + assert_eq!(parse(&conflict, materialized.as_bytes()), conflict); + + // Test resolving the conflict, leaving some fake conflict markers which should + // not be parsed since they are too short + let resolved_file_contents = indoc! {" + <<<<<<<<<<<< not a real conflict! + ++++++++++++ + left + ------------ + base + ++++++++++++ + right + >>>>>>>>>>>> + "}; + let resolved_file_id = testutils::write_file(store, path, resolved_file_contents); + assert_eq!( + parse(&conflict, resolved_file_contents.as_bytes()), + Merge::normal(resolved_file_id) + ); + + // Resolve one of the conflicts, decreasing the minimum conflict marker length + let new_conflict_contents = indoc! {" + <<<<<<<<<<<<<<<< Conflict 1 of 2 + ++++++++++++++++ Contents of side #1 + <<<< left 1 + ---------------- Contents of base + line 1 + ++++++++++++++++ Contents of side #2 + >>>>>>> right 1 + >>>>>>>>>>>>>>>> Conflict 1 of 2 ends + line 2 + line 3 + "}; + let new_conflict = parse(&conflict, new_conflict_contents.as_bytes()); + let new_left_file_id = testutils::write_file( + store, + path, + indoc! {" + <<<< left 1 + line 2 + line 3 + "}, + ); + let new_right_file_id = testutils::write_file( + store, + path, + indoc! {" + >>>>>>> right 1 + line 2 + line 3 + "}, + ); + assert_eq!( + new_conflict, + Merge::from_removes_adds( + vec![Some(base_file_id.clone())], + vec![ + Some(new_left_file_id.clone()), + Some(new_right_file_id.clone()) + ] + ) + ); + + // The conflict markers should still parse in future snapshots even though + // they're now longer than necessary + assert_eq!( + parse(&new_conflict, new_conflict_contents.as_bytes()), + new_conflict + ); + + // If we add back the second conflict, it should still be parsed correctly + // (the fake conflict markers shouldn't be interpreted as conflict 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 + insta::assert_snapshot!( + materialize_conflict_string(store, path, &new_conflict, ConflictMarkerStyle::Snapshot), + @r##" + <<<<<<<<<<< Conflict 1 of 1 + +++++++++++ Contents of side #1 + <<<< left 1 + ----------- Contents of base + line 1 + +++++++++++ Contents of side #2 + >>>>>>> right 1 + >>>>>>>>>>> Conflict 1 of 1 ends + line 2 + line 3 + "## + ); +} + fn materialize_conflict_string( store: &Store, path: &RepoPath,