From d2b06b9cf97d20d81af8b061688860d6a2d031ff Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sat, 16 Nov 2024 11:25:30 -0600 Subject: [PATCH] conflicts: add "snapshot" conflict marker style Adds a new "snapshot" conflict marker style which returns a series of snapshots, similar to Git's "diff3" conflict style. The "snapshot" option uses a subset of the conflict hunk headers as the "diff" option (it just doesn't use "%%%%%%%"), meaning that the two options are trivially compatible with each other (i.e. a file materialized with "snapshot" can be parsed with "diff" and vice versa). Example of "snapshot" conflict markers: ``` <<<<<<< Conflict 1 of 1 +++++++ Contents of side #1 fn example(word: String) { println!("word is {word}"); ------- Contents of base fn example(w: String) { println!("word is {w}"); +++++++ Contents of side #2 fn example(w: &str) { println!("word is {w}"); >>>>>>> Conflict 1 of 1 ends } ``` --- CHANGELOG.md | 2 + cli/src/config-schema.json | 3 +- lib/src/conflicts.rs | 13 +- lib/tests/test_conflicts.rs | 351 ++++++++++++++++++++++++++++++++++++ 4 files changed, 367 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d3e5c815..ae3f0f1c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * 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. + The new "snapshot" option renders conflicts as a series of snapshots, showing + each side and base of the conflict. ### Fixed bugs diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index b9c347018..0468e95c3 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -163,7 +163,8 @@ "type": "string", "description": "Conflict marker style to use when materializing conflicts in the working copy", "enum": [ - "diff" + "diff", + "snapshot" ], "default": "diff" } diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index d5b612d9b..48f9e3c36 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -240,6 +240,8 @@ pub enum ConflictMarkerStyle { /// Style which shows a snapshot and a series of diffs to apply. #[default] Diff, + /// Style which shows a snapshot for each base and side. + Snapshot, } pub fn materialize_merge_result>( @@ -274,7 +276,7 @@ pub fn materialize_merge_result_to_bytes>( fn materialize_conflict_hunks( hunks: &[Merge], - _conflict_marker_style: ConflictMarkerStyle, + conflict_marker_style: ConflictMarkerStyle, output: &mut dyn Write, ) -> io::Result<()> { // Write a positive snapshot (side) of a conflict @@ -338,6 +340,15 @@ fn materialize_conflict_hunks( write_base(&base_str, left, output)?; continue; }; + + // For any style other than "diff", always emit sides and bases separately + if conflict_marker_style != ConflictMarkerStyle::Diff { + write_side(add_index, right1, output)?; + write_base(&base_str, left, output)?; + add_index += 1; + continue; + } + let diff1 = Diff::by_line([&left, &right1]).hunks().collect_vec(); // Check if the diff against the next positive term is better. Since // we want to preserve the order of the terms, we don't match against diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 40518a5af..e3e19b3f0 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -116,6 +116,145 @@ fn test_materialize_conflict_basic() { line 5 "### ); + // Test materializing "snapshot" conflict markers + let conflict = Merge::from_removes_adds( + vec![Some(base_id.clone())], + vec![Some(left_id.clone()), Some(right_id.clone())], + ); + insta::assert_snapshot!( + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Snapshot), + @r##" + line 1 + line 2 + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 + left 3.1 + left 3.2 + left 3.3 + ------- Contents of base + line 3 + +++++++ Contents of side #2 + right 3.1 + >>>>>>> Conflict 1 of 1 ends + line 4 + line 5 + "## + ); +} + +#[test] +fn test_materialize_conflict_three_sides() { + let test_repo = TestRepo::init(); + let store = test_repo.repo.store(); + + let path = RepoPath::from_internal_string("file"); + let base_1_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 base + line 3 base + line 4 base + line 5 + "}, + ); + let base_2_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 base + line 5 + "}, + ); + let a_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 a.1 + line 3 a.2 + line 4 base + line 5 + "}, + ); + let b_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 b.1 + line 3 base + line 4 b.2 + line 5 + "}, + ); + let c_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 base + line 3 c.2 + line 5 + "}, + ); + + let conflict = Merge::from_removes_adds( + vec![Some(base_1_id.clone()), Some(base_2_id.clone())], + vec![Some(a_id.clone()), Some(b_id.clone()), Some(c_id.clone())], + ); + // Test materializing "diff" conflict markers + insta::assert_snapshot!( + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), + @r##" + line 1 + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 + -line 2 base + -line 3 base + +line 2 a.1 + +line 3 a.2 + line 4 base + +++++++ Contents of side #2 + line 2 b.1 + line 3 base + line 4 b.2 + %%%%%%% Changes from base #2 to side #3 + line 2 base + +line 3 c.2 + >>>>>>> Conflict 1 of 1 ends + line 5 + "## + ); + // Test materializing "snapshot" conflict markers + insta::assert_snapshot!( + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Snapshot), + @r##" + line 1 + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 + line 2 a.1 + line 3 a.2 + line 4 base + ------- Contents of base #1 + line 2 base + line 3 base + line 4 base + +++++++ Contents of side #2 + line 2 b.1 + line 3 base + line 4 b.2 + ------- Contents of base #2 + line 2 base + +++++++ Contents of side #3 + line 2 base + line 3 c.2 + >>>>>>> Conflict 1 of 1 ends + line 5 + "## + ); } #[test] @@ -341,6 +480,72 @@ fn test_materialize_parse_roundtrip() { "###); } +#[test] +fn test_materialize_parse_roundtrip_different_markers() { + let test_repo = TestRepo::init(); + let store = test_repo.repo.store(); + + let path = RepoPath::from_internal_string("file"); + let base_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 base + line 3 base + line 4 base + line 5 + "}, + ); + let a_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 a.1 + line 3 a.2 + line 4 base + line 5 + "}, + ); + let b_id = testutils::write_file( + store, + path, + indoc! {" + line 1 + line 2 b.1 + line 3 base + line 4 b.2 + line 5 + "}, + ); + + let conflict = Merge::from_removes_adds( + vec![Some(base_id.clone())], + vec![Some(a_id.clone()), Some(b_id.clone())], + ); + + let all_styles = [ConflictMarkerStyle::Diff, ConflictMarkerStyle::Snapshot]; + + // For every pair of conflict marker styles, materialize the conflict using the + // first style and parse it using the second. It should return the same result + // regardless of the conflict markers used for materialization and parsing. + for materialize_style in all_styles { + let materialized = materialize_conflict_string(store, path, &conflict, materialize_style); + for parse_style in all_styles { + let parsed = + update_from_content(&conflict, store, path, materialized.as_bytes(), parse_style) + .block_on() + .unwrap(); + + assert_eq!( + parsed, conflict, + "parse {materialize_style:?} conflict markers with {parse_style:?}" + ); + } + } +} + #[test] fn test_materialize_conflict_no_newlines_at_eof() { let test_repo = TestRepo::init(); @@ -615,6 +820,84 @@ fn test_parse_conflict_simple() { ) "### ); + // Test "snapshot" style + insta::assert_debug_snapshot!( + parse_conflict(indoc! {b" + line 1 + <<<<<<< Random text + +++++++ Random text + line 3.1 + line 3.2 + ------- Random text + line 3 + line 4 + +++++++ Random text + line 3 + line 4.1 + >>>>>>> Random text + line 5 + "}, + 2 + ), + @r#" + Some( + [ + Resolved( + "line 1\n", + ), + Conflicted( + [ + "line 3.1\nline 3.2\n", + "line 3\nline 4\n", + "line 3\nline 4.1\n", + ], + ), + Resolved( + "line 5\n", + ), + ], + ) + "# + ); + // Test "snapshot" style with reordered sections + insta::assert_debug_snapshot!( + parse_conflict(indoc! {b" + line 1 + <<<<<<< Random text + ------- Random text + line 3 + line 4 + +++++++ Random text + line 3.1 + line 3.2 + +++++++ Random text + line 3 + line 4.1 + >>>>>>> Random text + line 5 + "}, + 2 + ), + @r#" + Some( + [ + Resolved( + "line 1\n", + ), + Conflicted( + [ + "line 3.1\nline 3.2\n", + "line 3\nline 4\n", + "line 3\nline 4.1\n", + ], + ), + Resolved( + "line 5\n", + ), + ], + ) + "# + ); // The conflict markers are too long and shouldn't parse (though we may // decide to change this in the future) insta::assert_debug_snapshot!( @@ -726,6 +1009,52 @@ fn test_parse_conflict_multi_way() { ) "### ); + // Test "snapshot" style + insta::assert_debug_snapshot!( + parse_conflict(indoc! {b" + line 1 + <<<<<<< Random text + +++++++ Random text + line 3.1 + line 3.2 + +++++++ Random text + line 3 + line 4.1 + ------- Random text + line 3 + line 4 + ------- Random text + line 3 + +++++++ Random text + line 3 + line 4 + >>>>>>> Random text + line 5 + "}, + 3 + ), + @r#" + Some( + [ + Resolved( + "line 1\n", + ), + Conflicted( + [ + "line 3.1\nline 3.2\n", + "line 3\nline 4\n", + "line 3\nline 4.1\n", + "line 3\n", + "line 3\nline 4\n", + ], + ), + Resolved( + "line 5\n", + ), + ], + ) + "# + ); } #[test] @@ -906,6 +1235,28 @@ fn test_parse_conflict_malformed_diff() { ); } +#[test] +fn test_parse_conflict_snapshot_missing_header() { + // The "+++++++" header is missing + assert_eq!( + parse_conflict( + indoc! {b" + line 1 + <<<<<<< + left + ------- + base + +++++++ + right + >>>>>>> + line 5 + "}, + 2 + ), + None + ); +} + #[test] fn test_update_conflict_from_content() { let test_repo = TestRepo::init();