From 443528159e196c287741723727015cc90d805898 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 27 Jun 2021 09:36:32 -0700 Subject: [PATCH] conflicts: use new multi-way diff for materialized multi-way conflicts This copies the conflict marker format I added a while ago to Mercurial (https://phab.mercurial-scm.org/D9551), except that it uses `+++++++` instead of `=======` for sections that are pure adds. The reason I made that change is because we also have support for pure removes (Mercurial never ends up in that situation because it has exactly one remove and two adds). This change resolves part of issue #19. --- README.md | 9 +-- lib/src/conflicts.rs | 153 +++++++++++++++++++++++++------------------ lib/src/store.rs | 32 --------- 3 files changed, 93 insertions(+), 101 deletions(-) diff --git a/README.md b/README.md index 775a2fc4a5..7b51656a6f 100644 --- a/README.md +++ b/README.md @@ -349,10 +349,11 @@ Working copy now at: 619f58d8a988 added 0 files, modified 1 files, removed 0 files $ cat file1 <<<<<<< -a -||||||| -b1 -======= +------- ++++++++ +-b1 ++a ++++++++ b2 >>>>>>> $ echo resolved > file1 diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 053b1d442b..5e932bfb85 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -12,11 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::cmp::min; use std::io::{Cursor, Write}; use itertools::Itertools; +use crate::diff::{find_line_ranges, Diff, DiffHunk}; use crate::files; +use crate::files::{MergeHunk, MergeResult}; use crate::repo_path::RepoPath; use crate::store::{Conflict, ConflictPart, TreeValue}; use crate::store_wrapper::StoreWrapper; @@ -76,6 +79,49 @@ fn file_parts(parts: &[ConflictPart]) -> Vec<&ConflictPart> { .collect_vec() } +fn get_file_contents(store: &StoreWrapper, path: &RepoPath, part: &ConflictPart) -> Vec { + if let TreeValue::Normal { + id, + executable: false, + } = &part.value + { + let mut content: Vec = vec![]; + store + .read_file(path, id) + .unwrap() + .read_to_end(&mut content) + .unwrap(); + content + } else { + panic!("unexpectedly got a non-file conflict part"); + } +} + +fn write_diff_hunks(left: &[u8], right: &[u8], file: &mut dyn Write) -> std::io::Result<()> { + let diff = Diff::for_tokenizer(&[left, right], &find_line_ranges); + for hunk in diff.hunks() { + match hunk { + DiffHunk::Matching(content) => { + for line in content.split_inclusive(|b| *b == b'\n') { + file.write_all(b" ")?; + file.write_all(line)?; + } + } + DiffHunk::Different(content) => { + for line in content[0].split_inclusive(|b| *b == b'\n') { + file.write_all(b"-")?; + file.write_all(line)?; + } + for line in content[1].split_inclusive(|b| *b == b'\n') { + file.write_all(b"+")?; + file.write_all(line)?; + } + } + } + } + Ok(()) +} + pub fn materialize_conflict( store: &StoreWrapper, path: &RepoPath, @@ -91,78 +137,55 @@ pub fn materialize_conflict( return; } - match conflict.to_three_way() { - None => { - file.write_all(b"Unresolved complex conflict.\n").unwrap(); + let added_content = file_adds + .iter() + .map(|part| get_file_contents(store, path, part)) + .collect_vec(); + let removed_content = file_removes + .iter() + .map(|part| get_file_contents(store, path, part)) + .collect_vec(); + let removed_slices = removed_content + .iter() + .map(|vec| vec.as_slice()) + .collect_vec(); + let added_slices = added_content.iter().map(|vec| vec.as_slice()).collect_vec(); + + let merge_result = files::merge(&removed_slices, &added_slices); + match merge_result { + MergeResult::Resolved(content) => { + file.write_all(&content).unwrap(); } - Some((Some(left), Some(base), Some(right))) => { - match (left.value, base.value, right.value) { - ( - TreeValue::Normal { - id: left_id, - executable: false, - }, - TreeValue::Normal { - id: base_id, - executable: false, - }, - TreeValue::Normal { - id: right_id, - executable: false, - }, - ) => { - let mut left_contents: Vec = vec![]; - let mut base_contents: Vec = vec![]; - let mut right_contents: Vec = vec![]; - store - .read_file(path, &left_id) - .unwrap() - .read_to_end(&mut left_contents) - .unwrap(); - store - .read_file(path, &base_id) - .unwrap() - .read_to_end(&mut base_contents) - .unwrap(); - store - .read_file(path, &right_id) - .unwrap() - .read_to_end(&mut right_contents) - .unwrap(); - let merge_result = - files::merge(&[&base_contents], &[&left_contents, &right_contents]); - match merge_result { - files::MergeResult::Resolved(contents) => { - file.write_all(&contents).unwrap(); + MergeResult::Conflict(hunks) => { + for hunk in hunks { + match hunk { + MergeHunk::Resolved(content) => { + file.write_all(&content).unwrap(); + } + MergeHunk::Conflict { removes, adds } => { + let num_diffs = min(removes.len(), adds.len()); + + // TODO: Pair up a remove with an add in a way that minimizes the size of + // the diff + file.write_all(b"<<<<<<<\n").unwrap(); + for i in 0..num_diffs { + file.write_all(b"-------\n").unwrap(); + file.write_all(b"+++++++\n").unwrap(); + write_diff_hunks(&removes[i], &adds[i], file).unwrap(); + } + for slice in removes.iter().skip(num_diffs) { + file.write_all(b"-------\n").unwrap(); + file.write_all(slice).unwrap(); } - files::MergeResult::Conflict(hunks) => { - for hunk in hunks { - match hunk { - files::MergeHunk::Resolved(contents) => { - file.write_all(&contents).unwrap(); - } - files::MergeHunk::Conflict { removes, adds } => { - file.write_all(b"<<<<<<<\n").unwrap(); - file.write_all(&adds[0]).unwrap(); - file.write_all(b"|||||||\n").unwrap(); - file.write_all(&removes[0]).unwrap(); - file.write_all(b"=======\n").unwrap(); - file.write_all(&adds[1]).unwrap(); - file.write_all(b">>>>>>>\n").unwrap(); - } - } - } + for slice in adds.iter().skip(num_diffs) { + file.write_all(b"+++++++\n").unwrap(); + file.write_all(slice).unwrap(); } + file.write_all(b">>>>>>>\n").unwrap(); } } - _ => { - file.write_all(b"Unresolved 3-way conflict.\n").unwrap(); - } } } - Some(_) => { - file.write_all(b"Unresolved complex conflict.\n").unwrap(); - } } } diff --git a/lib/src/store.rs b/lib/src/store.rs index 634e84a42e..e208b29f37 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -182,38 +182,6 @@ pub struct Conflict { pub adds: Vec, } -impl Conflict { - // Returns (left,base,right) if this conflict is a 3-way conflict - pub fn to_three_way( - &self, - ) -> Option<( - Option, - Option, - Option, - )> { - if self.removes.len() == 1 && self.adds.len() == 2 { - // Regular (modify/modify) 3-way conflict - Some(( - Some(self.adds[0].clone()), - Some(self.removes[0].clone()), - Some(self.adds[1].clone()), - )) - } else if self.removes.is_empty() && self.adds.len() == 2 { - // Add/add conflict - Some((Some(self.adds[0].clone()), None, Some(self.adds[1].clone()))) - } else if self.removes.len() == 1 && self.adds.len() == 1 { - // Modify/delete conflict - Some(( - Some(self.adds[0].clone()), - Some(self.removes[0].clone()), - None, - )) - } else { - None - } - } -} - impl Default for Conflict { fn default() -> Self { Conflict {