diff --git a/CHANGELOG.md b/CHANGELOG.md index 263390d10c..eb97c8edcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### New features +* Conflict markers now include an explanation of what each part of the conflict + represents. + + ### Fixed bugs ## [0.17.0] - 2024-05-01 diff --git a/cli/tests/test_cat_command.rs b/cli/tests/test_cat_command.rs index f15c6da557..0163bbc27d 100644 --- a/cli/tests/test_cat_command.rs +++ b/cli/tests/test_cat_command.rs @@ -89,11 +89,11 @@ fn test_cat() { test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "@", "-d", "@--"]); let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file1"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -b +a - +++++++ + +++++++ Contents of side #2 c >>>>>>> "###); diff --git a/cli/tests/test_chmod_command.rs b/cli/tests/test_chmod_command.rs index 4a6db40af3..0015200c29 100644 --- a/cli/tests/test_chmod_command.rs +++ b/cli/tests/test_chmod_command.rs @@ -71,11 +71,11 @@ fn test_chmod_regular_conflict() { let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +x - +++++++ + +++++++ Contents of side #2 n >>>>>>> "###); @@ -90,11 +90,11 @@ fn test_chmod_regular_conflict() { let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +x - +++++++ + +++++++ Contents of side #2 n >>>>>>> "###); @@ -107,11 +107,11 @@ fn test_chmod_regular_conflict() { let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +x - +++++++ + +++++++ Contents of side #2 n >>>>>>> "###); @@ -201,10 +201,10 @@ fn test_chmod_file_dir_deletion_conflicts() { let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "-r=file_deletion", "file"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 a - %%%%%%% + %%%%%%% Changes from base to side #2 -base >>>>>>> "###); @@ -234,10 +234,10 @@ fn test_chmod_file_dir_deletion_conflicts() { let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "-r=file_deletion", "file"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 a - %%%%%%% + %%%%%%% Changes from base to side #2 -base >>>>>>> "###); diff --git a/cli/tests/test_diffedit_command.rs b/cli/tests/test_diffedit_command.rs index 3b30a36307..e08ed7b607 100644 --- a/cli/tests/test_diffedit_command.rs +++ b/cli/tests/test_diffedit_command.rs @@ -395,11 +395,11 @@ fn test_diffedit_merge() { assert!(!repo_path.join("file1").exists()); let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file2"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -a +c - +++++++ + +++++++ Contents of side #2 b >>>>>>> "###); diff --git a/cli/tests/test_interdiff_command.rs b/cli/tests/test_interdiff_command.rs index 67257be912..b3f8da715e 100644 --- a/cli/tests/test_interdiff_command.rs +++ b/cli/tests/test_interdiff_command.rs @@ -153,11 +153,11 @@ fn test_interdiff_conflicting() { --- a/file +++ b/file @@ -1,7 +1,1 @@ - -<<<<<<< - -%%%%%%% + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 --foo -+abc - -+++++++ + -+++++++ Contents of side #2 -bar ->>>>>>> +def diff --git a/cli/tests/test_obslog_command.rs b/cli/tests/test_obslog_command.rs index dec0cda0ad..d328f2e6df 100644 --- a/cli/tests/test_obslog_command.rs +++ b/cli/tests/test_obslog_command.rs @@ -59,10 +59,10 @@ fn test_obslog_with_or_without_diff() { @ rlvkpnrz test.user@example.com 2001-02-03 08:05:10 66b42ad3 │ my description │ Resolved conflict in file1: - │ 1 1: <<<<<<>>>>>> @@ -111,10 +111,10 @@ fn test_obslog_with_or_without_diff() { --- a/file1 +++ b/file1 @@ -1,7 +1,1 @@ - -<<<<<<< - -%%%%%%% + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 --foo - -+++++++ + -+++++++ Contents of side #2 -foo -bar ->>>>>>> diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index 02ea988837..d5f79f693c 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -69,14 +69,14 @@ fn test_resolution() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - %%%%%%% - -base - +a - +++++++ - b - >>>>>>> - "###); + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 + -base + +a + +++++++ Contents of side #2 + b + >>>>>>> + "###); let editor_script = test_env.set_up_fake_editor(); // Check that output file starts out empty and resolve the conflict @@ -97,16 +97,21 @@ fn test_resolution() { insta::assert_snapshot!( std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###" "###); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in file: - 1 1: <<<<<<>>>>>> + diff --git a/file b/file + index 0000000000...88425ec521 100644 + --- a/file + +++ b/file + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --base + -+a + -+++++++ Contents of side #2 + -b + ->>>>>>> + +resolution "###); insta::assert_snapshot!(test_env.jj_cmd_cli_error(&repo_path, &["resolve", "--list"]), @r###" @@ -132,16 +137,21 @@ fn test_resolution() { Parent commit : royxmykx db6a4daf b | b Added 0 files, modified 1 files, removed 0 files "###); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in file: - 1 1: <<<<<<>>>>>> + diff --git a/file b/file + index 0000000000...88425ec521 100644 + --- a/file + +++ b/file + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --base + -+a + -+++++++ Contents of side #2 + -b + ->>>>>>> + +resolution "###); insta::assert_snapshot!(test_env.jj_cmd_cli_error(&repo_path, &["resolve", "--list"]), @r###" @@ -151,7 +161,7 @@ fn test_resolution() { // Check that the output file starts with conflict markers if // `merge-tool-edits-conflict-markers=true` test_env.jj_cmd_ok(&repo_path, &["undo"]); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @""); std::fs::write( &editor_script, @@ -168,30 +178,35 @@ fn test_resolution() { ); insta::assert_snapshot!( std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(), @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +a - +++++++ + +++++++ Contents of side #2 b >>>>>>> "###); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in file: - 1 1: <<<<<<>>>>>> + diff --git a/file b/file + index 0000000000...88425ec521 100644 + --- a/file + +++ b/file + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --base + -+a + -+++++++ Contents of side #2 + -b + ->>>>>>> + +resolution "###); // Check that if merge tool leaves conflict markers in output file and // `merge-tool-edits-conflict-markers=true`, these markers are properly parsed. test_env.jj_cmd_ok(&repo_path, &["undo"]); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @""); std::fs::write( &editor_script, @@ -238,25 +253,31 @@ fn test_resolution() { "###); insta::assert_snapshot!( std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +a - +++++++ + +++++++ Contents of side #2 b >>>>>>> "###); // Note the "Modified" below - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Modified conflict in file: - 1 1: <<<<<<< - 2 2: %%%%%%% - 3 3: -basesome - 4 4: +afake - 5 5: +++++++ - 6 6: bconflict - 7 7: >>>>>>> + diff --git a/file b/file + --- a/file + +++ b/file + @@ -1,7 +1,7 @@ + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 + --base + -+a + +-some + ++fake + +++++++ Contents of side #2 + -b + +conflict + >>>>>>> "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]), @r###" @@ -267,7 +288,7 @@ fn test_resolution() { // `merge-tool-edits-conflict-markers=false` or is not specified, // `jj` considers the conflict resolved. test_env.jj_cmd_ok(&repo_path, &["undo"]); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @""); std::fs::write( &editor_script, @@ -300,16 +321,26 @@ fn test_resolution() { std::fs::read_to_string(test_env.env_root().join("editor3")).unwrap(), @r###" "###); // Note the "Resolved" below - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in file: - 1 1: <<<<<<< - 2 2: %%%%%%% - 3 3: -basesome - 4 4: +afake - 5 5: +++++++ - 6 6: bconflict - 7 7: >>>>>>> + diff --git a/file b/file + index 0000000000...0610716cc1 100644 + --- a/file + +++ b/file + @@ -1,7 +1,7 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --base + -+a + -+++++++ Contents of side #2 + -b + +<<<<<<< + +%%%%%%% + +-some + ++fake + ++++++++ + +conflict + >>>>>>> "###); insta::assert_snapshot!(test_env.jj_cmd_cli_error(&repo_path, &["resolve", "--list"]), @r###" @@ -370,14 +401,14 @@ fn test_normal_conflict_input_files() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - %%%%%%% - -base - +a - +++++++ - b - >>>>>>> - "###); + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 + -base + +a + +++++++ Contents of side #2 + b + >>>>>>> + "###); check_resolve_produces_input_file(&mut test_env, &repo_path, "base", "base\n"); check_resolve_produces_input_file(&mut test_env, &repo_path, "left", "a\n"); @@ -411,10 +442,10 @@ fn test_baseless_conflict_input_files() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 +a - +++++++ + +++++++ Contents of side #2 b >>>>>>> "###); @@ -482,10 +513,10 @@ fn test_edit_delete_conflict_input_files() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 a - %%%%%%% + %%%%%%% Changes from base to side #2 -base >>>>>>> "###); @@ -654,22 +685,22 @@ fn test_multiple_conflicts() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("this_file_has_a_very_long_name_to_test_padding")).unwrap() , @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -first base +first a - +++++++ + +++++++ Contents of side #2 first b >>>>>>> "###); insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("another_file")).unwrap() , @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -second base +second a - +++++++ + +++++++ Contents of side #2 second b >>>>>>> "###); @@ -707,16 +738,21 @@ fn test_multiple_conflicts() { There are unresolved conflicts at these paths: this_file_has_a_very_long_name_to_test_padding 2-sided conflict "###); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in another_file: - 1 : <<<<<<< - 2 : %%%%%%% - 3 1: -secondresolution baseanother_file - 4 : +second a - 5 : +++++++ - 6 : second b - 7 : >>>>>>> + diff --git a/another_file b/another_file + index 0000000000...a9fcc7d486 100644 + --- a/another_file + +++ b/another_file + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --second base + -+second a + -+++++++ Contents of side #2 + -second b + ->>>>>>> + +resolution another_file "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]), @r###" @@ -733,7 +769,7 @@ fn test_multiple_conflicts() { // For the rest of the test, we call `jj resolve` several times in a row to // resolve each conflict in the order it chooses. test_env.jj_cmd_ok(&repo_path, &["undo"]); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @""); std::fs::write( &editor_script, @@ -741,16 +777,21 @@ fn test_multiple_conflicts() { ) .unwrap(); test_env.jj_cmd_ok(&repo_path, &["resolve"]); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in another_file: - 1 : <<<<<<< - 2 : %%%%%%% - 3 1: first resolution for auto-secondchosen basefile - 4 : +second a - 5 : +++++++ - 6 : second b - 7 : >>>>>>> + diff --git a/another_file b/another_file + index 0000000000...7903e1c1c7 100644 + --- a/another_file + +++ b/another_file + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --second base + -+second a + -+++++++ Contents of side #2 + -second b + ->>>>>>> + +first resolution for auto-chosen file "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]), @r###" @@ -763,24 +804,34 @@ fn test_multiple_conflicts() { .unwrap(); test_env.jj_cmd_ok(&repo_path, &["resolve"]); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r###" - Resolved conflict in another_file: - 1 : <<<<<<< - 2 : %%%%%%% - 3 1: first resolution for auto-secondchosen basefile - 4 : +second a - 5 : +++++++ - 6 : second b - 7 : >>>>>>> - Resolved conflict in this_file_has_a_very_long_name_to_test_padding: - 1 : <<<<<<< - 2 : %%%%%%% - 3 1: second resolution for auto-firstchosen basefile - 4 : +first a - 5 : +++++++ - 6 : first b - 7 : >>>>>>> + diff --git a/another_file b/another_file + index 0000000000...7903e1c1c7 100644 + --- a/another_file + +++ b/another_file + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --second base + -+second a + -+++++++ Contents of side #2 + -second b + ->>>>>>> + +first resolution for auto-chosen file + diff --git a/this_file_has_a_very_long_name_to_test_padding b/this_file_has_a_very_long_name_to_test_padding + index 0000000000...f8c72adf17 100644 + --- a/this_file_has_a_very_long_name_to_test_padding + +++ b/this_file_has_a_very_long_name_to_test_padding + @@ -1,7 +1,1 @@ + -<<<<<<< Conflict 1 of 1 + -%%%%%%% Changes from base to side #1 + --first base + -+first a + -+++++++ Contents of side #2 + -first b + ->>>>>>> + +second resolution for auto-chosen file "###); insta::assert_snapshot!(test_env.jj_cmd_cli_error(&repo_path, &["resolve", "--list"]), diff --git a/cli/tests/test_restore_command.rs b/cli/tests/test_restore_command.rs index 6a352e3a52..726b5a5f8c 100644 --- a/cli/tests/test_restore_command.rs +++ b/cli/tests/test_restore_command.rs @@ -171,25 +171,25 @@ fn test_restore_conflicted_merge() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - %%%%%%% - -base - +a - +++++++ - b - >>>>>>> - "###); + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 + -base + +a + +++++++ Contents of side #2 + b + >>>>>>> + "###); // Overwrite the file... std::fs::write(repo_path.join("file"), "resolution").unwrap(); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), @r###" Resolved conflict in file: - 1 : <<<<<<< - 2 : %%%%%%% + 1 : <<<<<<< Conflict 1 of 1 + 2 : %%%%%%% Changes from base to side #1 3 : -base 4 : +a - 5 : +++++++ + 5 : +++++++ Contents of side #2 6 : b 7 : >>>>>>> 1: resolution @@ -210,11 +210,11 @@ fn test_restore_conflicted_merge() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +a - +++++++ + +++++++ Contents of side #2 b >>>>>>> "###); @@ -226,11 +226,11 @@ fn test_restore_conflicted_merge() { insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), @r###" Resolved conflict in file: - 1 : <<<<<<< - 2 : %%%%%%% + 1 : <<<<<<< Conflict 1 of 1 + 2 : %%%%%%% Changes from base to side #1 3 : -base 4 : +a - 5 : +++++++ + 5 : +++++++ Contents of side #2 6 : b 7 : >>>>>>> 1: resolution @@ -251,11 +251,11 @@ fn test_restore_conflicted_merge() { insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -base +a - +++++++ + +++++++ Contents of side #2 b >>>>>>> "###); diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index 871939c6dd..bb1c5ee7ee 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -649,14 +649,14 @@ fn test_squash_from_multiple() { // The changes from the sources have been applied let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=d", "file"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 -a +d - %%%%%%% + %%%%%%% Changes from base #2 to side #2 -a +b - +++++++ + +++++++ Contents of side #3 c >>>>>>> "###); @@ -786,14 +786,14 @@ fn test_squash_from_multiple_partial() { // The selected changes from the sources have been applied let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=d", "file1"]); insta::assert_snapshot!(stdout, @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 -a +d - %%%%%%% + %%%%%%% Changes from base #2 to side #2 -a +b - +++++++ + +++++++ Contents of side #3 c >>>>>>> "###); diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index e9627d7859..a4fa0fc994 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -19,6 +19,7 @@ use std::iter::zip; use futures::StreamExt; use itertools::Itertools; +use regex::bytes::Regex; use crate::backend::{BackendResult, CommitId, FileId, SymlinkId, TreeId, TreeValue}; use crate::diff::{find_line_ranges, Diff, DiffHunk}; @@ -28,11 +29,29 @@ use crate::merge::{Merge, MergeBuilder, MergedTreeValue}; use crate::repo_path::RepoPath; use crate::store::Store; -const CONFLICT_START_LINE: &[u8] = b"<<<<<<<\n"; -const CONFLICT_END_LINE: &[u8] = b">>>>>>>\n"; -const CONFLICT_DIFF_LINE: &[u8] = b"%%%%%%%\n"; -const CONFLICT_MINUS_LINE: &[u8] = b"-------\n"; -const CONFLICT_PLUS_LINE: &[u8] = b"+++++++\n"; +const CONFLICT_START_LINE: &[u8] = b"<<<<<<<"; +const CONFLICT_END_LINE: &[u8] = b">>>>>>>"; +const CONFLICT_DIFF_LINE: &[u8] = b"%%%%%%%"; +const CONFLICT_MINUS_LINE: &[u8] = b"-------"; +const CONFLICT_PLUS_LINE: &[u8] = b"+++++++"; +const CONFLICT_START_LINE_CHAR: u8 = CONFLICT_START_LINE[0]; +const CONFLICT_END_LINE_CHAR: u8 = CONFLICT_END_LINE[0]; +const CONFLICT_DIFF_LINE_CHAR: u8 = CONFLICT_DIFF_LINE[0]; +const CONFLICT_MINUS_LINE_CHAR: u8 = CONFLICT_MINUS_LINE[0]; +const CONFLICT_PLUS_LINE_CHAR: u8 = CONFLICT_PLUS_LINE[0]; + +/// A conflict marker is one of the separators, optionally followed by a space +/// and some text. +// TODO: All the `{7}` could be replaced with `{7,}` to allow longer +// separators. This could be useful to make it possible to allow conflict +// markers inside the text of the conflicts. +static CONFLICT_MARKER_REGEX: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { + Regex::new( + r"(<{7}|>{7}|%{7}|\-{7}|\+{7})( .*)? +", + ) + .unwrap() +}); fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result<()> { for hunk in hunks { @@ -185,19 +204,37 @@ pub fn materialize_merge_result( output.write_all(&content.0)?; } MergeResult::Conflict(hunks) => { + let num_conflicts = hunks + .iter() + .filter(|hunk| hunk.as_resolved().is_none()) + .count(); + let mut conflict_index = 0; for hunk in hunks { if let Some(content) = hunk.as_resolved() { output.write_all(&content.0)?; } else { + conflict_index += 1; output.write_all(CONFLICT_START_LINE)?; + output.write_all( + format!(" Conflict {conflict_index} of {num_conflicts}\n").as_bytes(), + )?; let mut add_index = 0; - for left in hunk.removes() { + for (base_index, left) in hunk.removes().enumerate() { + // The vast majority of conflicts one actually tries to + // resolve manually have 1 base. + let base_str = if hunk.removes().len() == 1 { + "base".to_string() + } else { + format!("base #{}", base_index + 1) + }; + let right1 = if let Some(right1) = hunk.get_add(add_index) { right1 } else { // If we have no more positive terms, emit the remaining negative // terms as snapshots. output.write_all(CONFLICT_MINUS_LINE)?; + output.write_all(format!(" Contents of {base_str}\n").as_bytes())?; output.write_all(&left.0)?; continue; }; @@ -217,8 +254,18 @@ pub fn materialize_merge_result( // the current positive term as a snapshot and the next // positive term as a diff. output.write_all(CONFLICT_PLUS_LINE)?; + output.write_all( + format!(" Contents of side #{}\n", add_index + 1).as_bytes(), + )?; output.write_all(&right1.0)?; output.write_all(CONFLICT_DIFF_LINE)?; + output.write_all( + format!( + " Changes from {base_str} to side #{}\n", + add_index + 2 + ) + .as_bytes(), + )?; write_diff_hunks(&diff2, output)?; add_index += 2; continue; @@ -226,16 +273,24 @@ pub fn materialize_merge_result( } output.write_all(CONFLICT_DIFF_LINE)?; + output.write_all( + format!(" Changes from {base_str} to side #{}\n", add_index + 1) + .as_bytes(), + )?; write_diff_hunks(&diff1, output)?; add_index += 1; } // Emit the remaining positive terms as snapshots. - for slice in hunk.adds().skip(add_index) { + for (add_index, slice) in hunk.adds().enumerate().skip(add_index) { output.write_all(CONFLICT_PLUS_LINE)?; + output.write_all( + format!(" Contents of side #{}\n", add_index + 1).as_bytes(), + )?; output.write_all(&slice.0)?; } output.write_all(CONFLICT_END_LINE)?; + output.write_all(b"\n")?; } } } @@ -267,21 +322,25 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option Merge { let mut removes = vec![]; let mut adds = vec![]; for line in input.split_inclusive(|b| *b == b'\n') { - match line { - CONFLICT_DIFF_LINE => { - state = State::Diff; - removes.push(ContentHunk(vec![])); - adds.push(ContentHunk(vec![])); - continue; - } - CONFLICT_MINUS_LINE => { - state = State::Minus; - removes.push(ContentHunk(vec![])); - continue; - } - CONFLICT_PLUS_LINE => { - state = State::Plus; - adds.push(ContentHunk(vec![])); - continue; + if CONFLICT_MARKER_REGEX.is_match_at(line, 0) { + match line[0] { + CONFLICT_DIFF_LINE_CHAR => { + state = State::Diff; + removes.push(ContentHunk(vec![])); + adds.push(ContentHunk(vec![])); + continue; + } + CONFLICT_MINUS_LINE_CHAR => { + state = State::Minus; + removes.push(ContentHunk(vec![])); + continue; + } + CONFLICT_PLUS_LINE_CHAR => { + state = State::Plus; + adds.push(ContentHunk(vec![])); + continue; + } + _ => {} } - _ => {} }; match state { State::Diff => { diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index cd4c67ae09..1dd4805f9a 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -77,12 +77,12 @@ fn test_materialize_conflict_basic() { @r###" line 1 line 2 - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 left 3.1 left 3.2 left 3.3 - %%%%%%% + %%%%%%% Changes from base to side #2 -line 3 +right 3.1 >>>>>>> @@ -101,11 +101,11 @@ fn test_materialize_conflict_basic() { @r###" line 1 line 2 - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -line 3 +right 3.1 - +++++++ + +++++++ Contents of side #2 left 3.1 left 3.2 left 3.3 @@ -216,16 +216,16 @@ fn test_materialize_conflict_multi_rebase_conflicts() { &materialize_conflict_string(store, path, &conflict), @r###" line 1 - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 line 2 a.1 line 2 a.2 line 2 a.3 - %%%%%%% + %%%%%%% Changes from base #1 to side #2 -line 2 base +line 2 b.1 +line 2 b.2 - %%%%%%% + %%%%%%% Changes from base #2 to side #3 -line 2 base +line 2 c.1 >>>>>>> @@ -240,15 +240,15 @@ fn test_materialize_conflict_multi_rebase_conflicts() { &materialize_conflict_string(store, path, &conflict), @r###" line 1 - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 -line 2 base +line 2 c.1 - %%%%%%% + %%%%%%% Changes from base #2 to side #2 -line 2 base +line 2 b.1 +line 2 b.2 - +++++++ + +++++++ Contents of side #3 line 2 a.1 line 2 a.2 line 2 a.3 @@ -264,15 +264,15 @@ fn test_materialize_conflict_multi_rebase_conflicts() { &materialize_conflict_string(store, path, &conflict), @r###" line 1 - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 -line 2 base +line 2 c.1 - +++++++ + +++++++ Contents of side #2 line 2 a.1 line 2 a.2 line 2 a.3 - %%%%%%% + %%%%%%% Changes from base #2 to side #3 -line 2 base +line 2 b.1 +line 2 b.2 @@ -282,6 +282,7 @@ fn test_materialize_conflict_multi_rebase_conflicts() { ); } +// TODO: With options #[test] fn test_materialize_parse_roundtrip() { let test_repo = TestRepo::init(); @@ -330,22 +331,22 @@ fn test_materialize_parse_roundtrip() { insta::assert_snapshot!( materialized, @r###" - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 2 + +++++++ Contents of side #1 line 1 left line 2 left - %%%%%%% + %%%%%%% Changes from base to side #2 -line 1 +line 1 right line 2 >>>>>>> line 3 - <<<<<<< - %%%%%%% + <<<<<<< Conflict 2 of 2 + %%%%%%% Changes from base to side #1 line 4 -line 5 +line 5 left - +++++++ + +++++++ Contents of side #2 line 4 right line 5 right >>>>>>> @@ -427,10 +428,10 @@ fn test_materialize_conflict_modify_delete() { insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict), @r###" line 1 line 2 - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 modified - %%%%%%% + %%%%%%% Changes from base to side #2 -line 3 >>>>>>> line 4 @@ -446,10 +447,10 @@ fn test_materialize_conflict_modify_delete() { insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict), @r###" line 1 line 2 - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 -line 3 - +++++++ + +++++++ Contents of side #2 modified >>>>>>> line 4 @@ -463,15 +464,15 @@ fn test_materialize_conflict_modify_delete() { vec![Some(modified_id.clone()), None], ); insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict), @r###" - <<<<<<< - %%%%%%% + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base to side #1 line 1 line 2 -line 3 +modified line 4 line 5 - +++++++ + +++++++ Contents of side #2 >>>>>>> "### ); @@ -516,16 +517,16 @@ fn test_materialize_conflict_two_forward_diffs() { insta::assert_snapshot!( &materialize_conflict_string(store, path, &conflict), @r###" - <<<<<<< - +++++++ + <<<<<<< Conflict 1 of 1 + +++++++ Contents of side #1 A - %%%%%%% + %%%%%%% Changes from base #1 to side #2 B - +++++++ + +++++++ Contents of side #3 D - %%%%%%% + %%%%%%% Changes from base #2 to side #4 C - ------- + ------- Contents of base #3 E >>>>>>> "### @@ -586,6 +587,42 @@ fn test_parse_conflict_simple() { ], ) "### + ); + insta::assert_debug_snapshot!( + parse_conflict(indoc! {b" + line 1 + <<<<<<<<<<< Text + %%%%%%%%%%% Different text + line 2 + -line 3 + +left + line 4 + +++++++++++ Yet <><>< more text + right + >>>>>>>>>>> More and more text + line 5 + "}, + 2 + ), + @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", + ), + ], + ) + "### ) } @@ -634,7 +671,50 @@ fn test_parse_conflict_multi_way() { ], ) "### + ); + insta::assert_debug_snapshot!( + parse_conflict(indoc! {b" + line 1 + <<<<<<< Random text + %%%%%%% Random text + line 2 + -line 3 + +left + line 4 + +++++++ Random text + right + %%%%%%% Random text + line 2 + +forward + line 3 + line 4 + >>>>>>> Random text + line 5 + "}, + 3 + ), + @r###" + Some( + [ + Resolved( + "line 1\n", + ), + Conflicted( + [ + "line 2\nleft\nline 4\n", + "line 2\nline 3\nline 4\n", + "right\n", + "line 2\nline 3\nline 4\n", + "line 2\nforward\nline 3\nline 4\n", + ], + ), + Resolved( + "line 5\n", + ), + ], ) + "### + ); } #[test] diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs index 8e4591f5ce..917e096f55 100644 --- a/lib/tests/test_local_working_copy.rs +++ b/lib/tests/test_local_working_copy.rs @@ -35,7 +35,8 @@ use jj_lib::working_copy::{CheckoutStats, SnapshotError, SnapshotOptions}; use jj_lib::workspace::LockedWorkspace; use test_case::test_case; use testutils::{ - commit_with_tree, create_tree, write_random_commit, TestRepoBackend, TestWorkspace, + commit_with_tree, create_nonlegacy_tree, create_tree, write_random_commit, TestRepoBackend, + TestWorkspace, }; fn to_owned_path_vec(paths: &[&RepoPath]) -> Vec { @@ -291,6 +292,158 @@ fn test_conflict_subdirectory() { .unwrap(); } +#[test] +fn test_conflict_per_file_issue_2795() { + let settings = testutils::user_settings(); + let mut test_workspace = TestWorkspace::init(&settings); + let repo = &test_workspace.repo; + + let path1 = RepoPath::from_internal_string("dir/file1"); + let path2 = RepoPath::from_internal_string("dir/file2"); + let side1 = create_nonlegacy_tree(repo, &[(path1, "A\n"), (path2, "X\n")]); + let base1 = create_nonlegacy_tree(repo, &[(path1, "B\n"), (path2, "X\n")]); + let side2 = create_nonlegacy_tree(repo, &[(path1, "C\n"), (path2, "X\n")]); + let base2 = create_nonlegacy_tree(repo, &[(path1, "C\n"), (path2, "Y\n")]); + let side3 = create_nonlegacy_tree(repo, &[(path1, "C\n"), (path2, "Z\n")]); + let merged_tree = side1 + .merge(&base1, &side2) + .unwrap() + .merge(&base2, &side3) + .unwrap(); + assert_debug_snapshot!(tree_entries(&merged_tree), @r###" + [ + ( + "dir/file1", + Some( + Conflicted( + [ + Some( + File { + id: FileId( + "1c5c06be70ae033fe5fb", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "54b59f5ab966db7bcaf6", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "cf47364a5a9580666247", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "cf47364a5a9580666247", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "cf47364a5a9580666247", + ), + executable: false, + }, + ), + ], + ), + ), + ), + ( + "dir/file2", + Some( + Conflicted( + [ + Some( + File { + id: FileId( + "1f1bfb848adce5ec09e9", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "1f1bfb848adce5ec09e9", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "1f1bfb848adce5ec09e9", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "d93bab6433193bd3bb24", + ), + executable: false, + }, + ), + Some( + File { + id: FileId( + "bc0f2e7ac9ee0da40dc2", + ), + executable: false, + }, + ), + ], + ), + ), + ), + ] + "###); + let merged_commit = commit_with_tree(repo.store(), merged_tree.id()); + let repo = &test_workspace.repo; + let ws = &mut test_workspace.workspace; + ws.check_out(repo.op_id().clone(), None, &merged_commit) + .unwrap(); + // TODO: was #2795 fixed or are these not tree-level conflicts? + let file1_contents = std::fs::read_to_string(path1.to_fs_path(ws.workspace_root())).unwrap(); + assert_snapshot!(file1_contents, @r###" + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 + -B + +A + %%%%%%% Changes from base #2 to side #2 + C + +++++++ Contents of side #3 + C + >>>>>>> + "###); + let file2_contents = std::fs::read_to_string(path2.to_fs_path(ws.workspace_root())).unwrap(); + assert_snapshot!(file2_contents, @r###" + <<<<<<< Conflict 1 of 1 + %%%%%%% Changes from base #1 to side #1 + X + %%%%%%% Changes from base #2 to side #2 + -Y + +X + +++++++ Contents of side #3 + Z + >>>>>>> + "###); +} + #[test] fn test_conflict_empty_vs_nonempty_issue_3223() { let settings = testutils::user_settings(); diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index d9e5ae215f..a47ddc952d 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -329,6 +329,13 @@ pub fn create_tree(repo: &Arc, path_contents: &[(&RepoPath, &str)] MergedTree::resolved(create_single_tree(repo, path_contents)) } +pub fn create_nonlegacy_tree( + repo: &Arc, + path_contents: &[(&RepoPath, &str)], +) -> MergedTree { + MergedTree::resolved(create_single_tree(repo, path_contents)) +} + #[must_use] pub fn create_random_tree(repo: &Arc) -> MergedTreeId { let number = rand::random::();