From 6bdae4599e97a07a357291ab28befd3ee74ecf4a 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 are at least as long as the materialized conflict markers based on the current tree. This can lead to some unintuitive edge cases which will be solved in the next commit. 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 ``` We should support these options for "git" conflict marker style as well, since Git actually does support producing longer conflict markers in some cases through .gitattributes: https://git-scm.com/docs/gitattributes#_conflict_marker_size We may also want to support passing the conflict marker length to merge tools as well in the future, since Git supports a "%L" parameter to pass the conflict marker length to merge drivers: https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver --- CHANGELOG.md | 4 + lib/src/conflicts.rs | 203 ++++++++++++----- lib/tests/test_conflicts.rs | 426 +++++++++++++++++++++++++++++++++--- 3 files changed, 554 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2486a9351..f77a4b3e9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New features +* 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. diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index ef937b051b..0438f802ac 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -49,8 +49,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. +const CONFLICT_MARKER_LEN_INCREMENT: usize = 4; fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> io::Result<()> { for hunk in hunks { @@ -259,13 +265,21 @@ impl ConflictMarkerLineChar { } } +/// Represents a conflict marker line parsed from the file. Conflict marker +/// lines consist of a single ASCII character repeated for a certain length. +struct ConflictMarkerLine { + kind: ConflictMarkerLineChar, + len: usize, +} + /// Write a conflict marker to an output file. fn write_conflict_marker( output: &mut dyn Write, kind: ConflictMarkerLineChar, + len: usize, suffix_text: &str, ) -> io::Result<()> { - let conflict_marker = BString::new(vec![kind.to_byte(); CONFLICT_MARKER_LEN]); + let conflict_marker = BString::new(vec![kind.to_byte(); len]); if suffix_text.is_empty() { writeln!(output, "{conflict_marker}") @@ -274,17 +288,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 = ConflictMarkerLineChar::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() { @@ -292,7 +302,31 @@ fn parse_conflict_marker(line: &[u8]) -> Option { } } - Some(kind) + Some(ConflictMarkerLine { kind, len }) +} + +/// Parse a conflict marker, expecting it to be at least a certain length. Any +/// shorter conflict markers are ignored. +fn parse_conflict_marker(line: &[u8], expected_len: usize) -> Option { + parse_conflict_marker_any_len(line) + .filter(|marker| marker.len >= expected_len) + .map(|marker| marker.kind) +} + +/// Given a Merge of files, choose the conflict marker length to use when +/// materializing conflicts. +pub fn choose_materialized_conflict_marker_len>(single_hunk: &Merge) -> usize { + let max_existing_marker_len = single_hunk + .iter() + .flat_map(|file| file.as_ref().lines_with_terminator()) + .filter_map(parse_conflict_marker_any_len) + .map(|marker| marker.len) + .max() + .unwrap_or_default(); + + max_existing_marker_len + .saturating_add(CONFLICT_MARKER_LEN_INCREMENT) + .max(MIN_CONFLICT_MARKER_LEN) } pub fn materialize_merge_result>( @@ -304,7 +338,23 @@ pub fn materialize_merge_result>( match &merge_result { MergeResult::Resolved(content) => output.write_all(content), MergeResult::Conflict(hunks) => { - materialize_conflict_hunks(hunks, conflict_marker_style, output) + let conflict_marker_len = choose_materialized_conflict_marker_len(single_hunk); + materialize_conflict_hunks(hunks, conflict_marker_style, conflict_marker_len, output) + } + } +} + +fn materialize_merge_result_with_marker_len>( + single_hunk: &Merge, + conflict_marker_style: ConflictMarkerStyle, + conflict_marker_len: usize, + output: &mut dyn Write, +) -> io::Result<()> { + let merge_result = files::merge(single_hunk); + match &merge_result { + MergeResult::Resolved(content) => output.write_all(content), + MergeResult::Conflict(hunks) => { + materialize_conflict_hunks(hunks, conflict_marker_style, conflict_marker_len, output) } } } @@ -317,9 +367,15 @@ pub fn materialize_merge_result_to_bytes>( match merge_result { MergeResult::Resolved(content) => content, MergeResult::Conflict(hunks) => { + let conflict_marker_len = choose_materialized_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() } } @@ -328,6 +384,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 @@ -345,13 +402,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, )?; } @@ -366,22 +431,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, ConflictMarkerLineChar::ConflictStart, + conflict_marker_len, &format!("Side #1 ({conflict_info})"), )?; output.write_all(left)?; - write_conflict_marker(output, ConflictMarkerLineChar::GitAncestor, "Base")?; + write_conflict_marker( + output, + ConflictMarkerLineChar::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, ConflictMarkerLineChar::GitSeparator, "")?; + write_conflict_marker( + output, + ConflictMarkerLineChar::GitSeparator, + conflict_marker_len, + "", + )?; output.write_all(right)?; write_conflict_marker( output, ConflictMarkerLineChar::ConflictEnd, + conflict_marker_len, &format!("Side #2 ({conflict_info} ends)"), )?; @@ -392,44 +470,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, ConflictMarkerLineChar::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, ConflictMarkerLineChar::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, - ConflictMarkerLineChar::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, + ConflictMarkerLineChar::Diff, + conflict_marker_len, + &format!("Changes from {base_str} to side #{}", add_index + 1), + )?; + write_diff_hunks(diff, output) + }; - write_conflict_marker(output, ConflictMarkerLineChar::ConflictStart, conflict_info)?; + write_conflict_marker( + output, + ConflictMarkerLineChar::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 @@ -482,6 +565,7 @@ fn materialize_jj_style_conflict( write_conflict_marker( output, ConflictMarkerLineChar::ConflictEnd, + conflict_marker_len, &format!("{conflict_info} ends"), )?; Ok(()) @@ -530,9 +614,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 be at least as long as the expected +/// length. Any shorter conflict markers 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, + expected_marker_len: usize, +) -> Option>> { if input.is_empty() { return None; } @@ -542,7 +633,7 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option { conflict_start = Some(pos); conflict_start_len = line.len(); @@ -550,7 +641,7 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option { if let Some(conflict_start_index) = conflict_start.take() { 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, expected_marker_len); if hunk.num_sides() == num_sides { let resolved_slice = &input[resolved_start..conflict_start_index]; if !resolved_slice.is_empty() { @@ -581,12 +672,12 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option Merge { +fn parse_conflict_hunk(input: &[u8], expected_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, expected_marker_len)); match initial_conflict_marker { // JJ-style conflicts must start with one of these 3 conflict marker lines @@ -594,16 +685,18 @@ fn parse_conflict_hunk(input: &[u8]) -> Merge { ConflictMarkerLineChar::Diff | ConflictMarkerLineChar::Remove | ConflictMarkerLineChar::Add, - ) => parse_jj_style_conflict_hunk(input), + ) => parse_jj_style_conflict_hunk(input, expected_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(ConflictMarkerLineChar::GitAncestor) => parse_git_style_conflict_hunk(input), + None | Some(ConflictMarkerLineChar::GitAncestor) => { + parse_git_style_conflict_hunk(input, expected_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], expected_marker_len: usize) -> Merge { enum State { Diff, Remove, @@ -614,7 +707,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, expected_marker_len) { Some(ConflictMarkerLineChar::Diff) => { state = State::Diff; removes.push(BString::new(vec![])); @@ -674,7 +767,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], expected_marker_len: usize) -> Merge { #[derive(PartialEq, Eq)] enum State { Left, @@ -686,7 +779,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, expected_marker_len) { Some(ConflictMarkerLineChar::GitAncestor) => { if state == State::Left { state = State::Base; @@ -742,7 +835,14 @@ 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 conflict_marker_len = choose_materialized_conflict_marker_len(&merge_hunk); + materialize_merge_result_with_marker_len( + &merge_hunk, + conflict_marker_style, + conflict_marker_len, + &mut old_content, + ) + .unwrap(); if content == old_content { return Ok(file_ids.clone()); } @@ -752,11 +852,16 @@ pub async fn update_from_content( // 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(), + conflict_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(), conflict_marker_len) + { break 'hunks (file_ids, hunks); }; }; diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index e3c4a00f6f..d27b9d9885 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,8 +1036,8 @@ 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) + // The conflict markers are longer than the originally materialized markers, but + // we allow them to parse anyway insta::assert_debug_snapshot!( parse_conflict(indoc! {b" line 1 @@ -1043,9 +1052,28 @@ fn test_parse_conflict_simple() { >>>>>>>>>>> line 5 "}, - 2 + 2, + 7 ), - @"None" + @r#" + Some( + [ + Resolved( + "line 1\n", + ), + Conflicted( + [ + "line 2\nleft\nline 4\n", + "line 2\nline 3\nline 4\n", + "right\n", + ], + ), + Resolved( + "line 5\n", + ), + ], + ) + "# ); } @@ -1071,7 +1099,8 @@ fn test_parse_conflict_multi_way() { >>>>>>> line 5 "}, - 3 + 3, + 7 ), @r###" Some( @@ -1114,7 +1143,8 @@ fn test_parse_conflict_multi_way() { >>>>>>> Random text line 5 "}, - 3 + 3, + 7 ), @r###" Some( @@ -1160,7 +1190,8 @@ fn test_parse_conflict_multi_way() { >>>>>>> Random text line 5 "}, - 3 + 3, + 7 ), @r#" Some( @@ -1203,7 +1234,8 @@ fn test_parse_conflict_crlf_markers() { >>>>>>>\r line 5\r "}, - 2 + 2, + 7 ), @r#" Some( @@ -1248,7 +1280,8 @@ fn test_parse_conflict_diff_stripped_whitespace() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1290,7 +1323,8 @@ fn test_parse_conflict_wrong_arity() { >>>>>>> line 5 "}, - 3 + 3, + 7 ), None ); @@ -1311,7 +1345,8 @@ fn test_parse_conflict_malformed_missing_removes() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1334,7 +1369,8 @@ fn test_parse_conflict_malformed_marker() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1358,7 +1394,8 @@ fn test_parse_conflict_malformed_diff() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1380,7 +1417,8 @@ fn test_parse_conflict_snapshot_missing_header() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1400,7 +1438,8 @@ fn test_parse_conflict_wrong_git_style() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1422,7 +1461,8 @@ fn test_parse_conflict_git_reordered_headers() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1448,7 +1488,8 @@ fn test_parse_conflict_git_too_many_sides() { >>>>>>> line 5 "}, - 3 + 3, + 7 ), None ); @@ -1471,7 +1512,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1489,7 +1531,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> line 5 "}, - 2 + 2, + 7 ), None ); @@ -1506,7 +1549,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> Conflict 1 of 1 ends line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1541,7 +1585,8 @@ fn test_parse_conflict_mixed_header_styles() { >>>>>>> Side #2 (Conflict 1 of 1 ends) line 5 "}, - 2 + 2, + 7 ), @r#" Some( @@ -1810,6 +1855,327 @@ 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 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 + "## + ); +} + +#[test] +fn test_update_from_content_malformed_conflict() { + let test_repo = TestRepo::init(); + let store = test_repo.repo.store(); + + let path = RepoPath::from_internal_string("dir/file"); + let base_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 + line 3 + line 4 + line 5 + "}, + ); + let left_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 left + line 3 + line 4 left + line 5 + "}, + ); + let right_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 right + line 3 + line 4 right + line 5 + "}, + ); + let conflict = Merge::from_removes_adds( + vec![Some(base_file_id.clone())], + vec![Some(left_file_id.clone()), Some(right_file_id.clone())], + ); + + let materialized = + materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); + insta::assert_snapshot!(materialized, @r##" + line 1 + <<<<<<< Conflict 1 of 2 + %%%%%%% Changes from base to side #1 + -line 2 + +line 2 left + +++++++ Contents of side #2 + line 2 right + >>>>>>> Conflict 1 of 2 ends + line 3 + <<<<<<< Conflict 2 of 2 + %%%%%%% Changes from base to side #1 + -line 4 + +line 4 left + +++++++ Contents of side #2 + line 4 right + >>>>>>> Conflict 2 of 2 ends + line 5 + "## + ); + + let parse = |conflict, content| { + update_from_content(conflict, store, path, content, ConflictMarkerStyle::Diff) + .block_on() + .unwrap() + }; + assert_eq!(parse(&conflict, materialized.as_bytes()), conflict); + + // Make a change to the second conflict that causes it to become invalid + let new_conflict_contents = indoc! {" + line 1 + <<<<<<< Conflict 1 of 2 + %%%%%%% Changes from base to side #1 + -line 2 + +line 2 left + +++++++ Contents of side #2 + line 2 right + >>>>>>> Conflict 1 of 2 ends + line 3 + <<<<<<< Conflict 2 of 2 + %%%%%%% Changes from base to side #1 + -line 4 + +line 4 left + line 4 right + >>>>>>> Conflict 2 of 2 ends + line 5 + "}; + // On the first snapshot, it will parse as a conflict containing conflict + // markers as text + let new_conflict = parse(&conflict, new_conflict_contents.as_bytes()); + let new_base_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 + line 3 + <<<<<<< Conflict 2 of 2 + %%%%%%% Changes from base to side #1 + -line 4 + +line 4 left + line 4 right + >>>>>>> Conflict 2 of 2 ends + line 5 + "}, + ); + let new_left_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 left + line 3 + <<<<<<< Conflict 2 of 2 + %%%%%%% Changes from base to side #1 + -line 4 + +line 4 left + line 4 right + >>>>>>> Conflict 2 of 2 ends + line 5 + "}, + ); + let new_right_file_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 right + line 3 + <<<<<<< Conflict 2 of 2 + %%%%%%% Changes from base to side #1 + -line 4 + +line 4 left + line 4 right + >>>>>>> Conflict 2 of 2 ends + line 5 + "}, + ); + assert_eq!( + new_conflict, + Merge::from_removes_adds( + vec![Some(new_base_file_id.clone())], + vec![ + Some(new_left_file_id.clone()), + Some(new_right_file_id.clone()) + ] + ) + ); + + // Snapshotting again will cause the conflict to appear resolved + let second_snapshot = parse(&new_conflict, new_conflict_contents.as_bytes()); + assert_ne!(second_snapshot, new_conflict); + assert!(second_snapshot.is_resolved()); +} + fn materialize_conflict_string( store: &Store, path: &RepoPath,