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,