diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d3e5c8150..5d914aed29 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 just renders conflicts as a series of snapshots, + showing each side of the conflict and the base(s). ### Fixed bugs diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index b9c347018c..0468e95c35 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 4d8688afbd..6079830d2b 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -238,6 +238,8 @@ async fn materialize_tree_value_no_access_denied( pub enum ConflictMarkerStyle { /// Style which shows a snapshot and a series of diffs to apply. Diff, + /// Style which shows a snapshot for each base and side. + Snapshot, } pub fn materialize_merge_result>( @@ -272,7 +274,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<()> { let num_conflicts = hunks @@ -304,6 +306,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 99c350b2de..5eb357dafd 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -615,6 +615,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 +804,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] @@ -1065,6 +1189,249 @@ fn test_update_conflict_from_content_simplified_conflict() { ); } +#[test] +fn test_two_sided_conflict_with_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())], + ); + + // Make sure each materialized conflict is able to be parsed correctly + let materialize_and_verify_parsed = |conflict_marker_style| { + let content = materialize_conflict_string(store, path, &conflict, conflict_marker_style); + // Validate parsing conflict markers with different config options + for other_style in [ConflictMarkerStyle::Diff, ConflictMarkerStyle::Snapshot] { + let parsed = + update_from_content(&conflict, store, path, content.as_bytes(), other_style) + .block_on() + .unwrap(); + + assert_eq!( + parsed, conflict, + "parse {conflict_marker_style:?} conflict markers with {other_style:?}" + ); + } + + content + }; + + // Test "diff" conflict markers + insta::assert_snapshot!( + &materialize_and_verify_parsed(ConflictMarkerStyle::Diff), + @r##" + line 1 + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base 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 + >>>>>>> Conflict 1 of 1 ends + line 5 + "## + ); + // Test "snapshot" conflict markers + insta::assert_snapshot!( + &materialize_and_verify_parsed(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 + line 2 base + line 3 base + line 4 base + +++++++ Contents of side #2 + line 2 b.1 + line 3 base + line 4 b.2 + >>>>>>> Conflict 1 of 1 ends + line 5 + "## + ); +} + +#[test] +fn test_many_sided_conflict_with_different_markers() { + 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())], + ); + + // Make sure each materialized conflict is able to be parsed correctly + let materialize_and_verify_parsed = |conflict_marker_style| { + let content = materialize_conflict_string(store, path, &conflict, conflict_marker_style); + // Validate parsing conflict markers with different config options + for other_style in [ConflictMarkerStyle::Diff, ConflictMarkerStyle::Snapshot] { + let parsed = + update_from_content(&conflict, store, path, content.as_bytes(), other_style) + .block_on() + .unwrap(); + + assert_eq!( + parsed, conflict, + "parse {conflict_marker_style:?} conflict markers with {other_style:?}" + ); + } + + content + }; + + // Test "diff" conflict markers + insta::assert_snapshot!( + &materialize_and_verify_parsed(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 "snapshot" conflict markers + insta::assert_snapshot!( + &materialize_and_verify_parsed(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 + "## + ); +} + fn materialize_conflict_string( store: &Store, path: &RepoPath,