From e48ace56d1e989d40ac3499f556c1ce603fa88f9 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Fri, 17 Feb 2023 10:05:47 -0800 Subject: [PATCH] conflicts: replace missing files by empty in materialized conflict When we materialize modify/delete conflicts, we currently don't include any context lines. That's because modify/delete conflicts have only two sides, so there's no common base to compare to. Hunks that are unchanged on the "modify" side are therefore not considered conflicting, and since they they don't contribute new changes, they're simply skipped (here: https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230). It seems more useful to instead pretend that the missing side is an empty file. That way we'll get a conflict in the entire file. We can still decide later to make e.g. `jj resolve` prompt the user on modify/delete conflicts just like `hg resolve` does (or maybe it actually happens earlier there, I don't remember). Closes #1244. --- CHANGELOG.md | 3 +++ lib/src/conflicts.rs | 10 ++++++++-- lib/tests/test_conflicts.rs | 6 +++++- tests/test_obslog_command.rs | 10 +++++++--- tests/test_resolve_command.rs | 13 ++++++------- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5ff5bdeb..25ee5ee56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed bugs +* Modify/delete conflicts now include context lines + [#1244](https://github.com/martinvonz/jj/issues/1244). + ## [0.7.0] - 2023-02-16 ### Breaking changes diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 1a4a41f75..bba79f998 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::cmp::max; use std::io::{Cursor, Write}; use itertools::Itertools; @@ -154,14 +155,19 @@ pub fn extract_file_conflict_as_single_hunk( if file_adds.len() != conflict.adds.len() || file_removes.len() != conflict.removes.len() { return None; } - let added_content = file_adds + let mut added_content = file_adds .iter() .map(|part| get_file_contents(store, path, part)) .collect_vec(); - let removed_content = file_removes + let mut removed_content = file_removes .iter() .map(|part| get_file_contents(store, path, part)) .collect_vec(); + // If the conflict had removed the file on one side, we pretend that the file + // was empty there. + let l = max(added_content.len(), removed_content.len() + 1); + added_content.resize(l, vec![]); + removed_content.resize(l - 1, vec![]); Some(ConflictHunk { removes: removed_content, diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index aa7bdecef..79ca7e626 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -197,12 +197,16 @@ line 5 removes: vec![file_conflict_part(&base_id)], adds: vec![file_conflict_part(&modified_id)], }; - // TODO: THis should have context around the conflict (#1244) insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###" <<<<<<< %%%%%%% + line 1 + line 2 -line 3 +modified + line 4 + line 5 + +++++++ >>>>>>> "### ); diff --git a/tests/test_obslog_command.rs b/tests/test_obslog_command.rs index 204aced56..b1fbbea71 100644 --- a/tests/test_obslog_command.rs +++ b/tests/test_obslog_command.rs @@ -49,8 +49,10 @@ fn test_obslog_with_or_without_diff() { │ Resolved conflict in file1: │ 1 1: <<<<<<>>>>>> + │ 3 : foo + │ 4 : +bar + │ 5 : +++++++ + │ 6 : >>>>>>> o rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:09.000 +07:00 af536e5af67e conflict │ my description o rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:09.000 +07:00 6fbba7bcb590 @@ -86,10 +88,12 @@ fn test_obslog_with_or_without_diff() { index e155302a24...2ab19ae607 100644 --- a/file1 +++ b/file1 - @@ -1,4 +1,1 @@ + @@ -1,6 +1,1 @@ -<<<<<<< -%%%%%%% + - foo -+bar + -+++++++ ->>>>>>> +resolved rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:09.000 +07:00 af536e5af67e conflict diff --git a/tests/test_resolve_command.rs b/tests/test_resolve_command.rs index 0e2f8fd12..7586c368e 100644 --- a/tests/test_resolve_command.rs +++ b/tests/test_resolve_command.rs @@ -356,8 +356,8 @@ fn test_baseless_conflict_input_files() { std::fs::read_to_string(repo_path.join("file")).unwrap() , @r###" <<<<<<< - +++++++ - a + %%%%%%% + +a +++++++ b >>>>>>> @@ -427,15 +427,14 @@ fn test_edit_delete_conflict_input_files() { <<<<<<< %%%%%%% -base - +a + +++++++ + a >>>>>>> "###); check_resolve_produces_input_file(&mut test_env, &repo_path, "base", "base\n"); - check_resolve_produces_input_file(&mut test_env, &repo_path, "left", ""); - // Note that `a` ended up in "right" rather than "left". It's unclear if this - // can or should be fixed. - check_resolve_produces_input_file(&mut test_env, &repo_path, "right", "a\n"); + check_resolve_produces_input_file(&mut test_env, &repo_path, "left", "a\n"); + check_resolve_produces_input_file(&mut test_env, &repo_path, "right", ""); } #[test]