diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index c64980cb4..f7f2fc4cd 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -19,7 +19,7 @@ use itertools::Itertools; use crate::backend::{BackendResult, FileId, ObjectId, TreeValue}; use crate::diff::{find_line_ranges, Diff, DiffHunk}; -use crate::files::{ConflictHunk, MergeHunk, MergeResult}; +use crate::files::{ConflictHunk, ContentHunk, MergeHunk, MergeResult}; use crate::merge::trivial_merge; use crate::repo_path::RepoPath; use crate::store::Store; @@ -240,18 +240,18 @@ impl Conflict> { match hunk { MergeHunk::Resolved(slice) => { for buf in &mut removed_content { - buf.extend_from_slice(&slice); + buf.extend_from_slice(&slice.0); } for buf in &mut added_content { - buf.extend_from_slice(&slice); + buf.extend_from_slice(&slice.0); } } MergeHunk::Conflict(ConflictHunk { removes, adds }) => { for (i, buf) in removes.into_iter().enumerate() { - removed_content[i].extend(buf); + removed_content[i].extend(buf.0); } for (i, buf) in adds.into_iter().enumerate() { - added_content[i].extend(buf); + added_content[i].extend(buf.0); } } } @@ -358,7 +358,7 @@ fn describe_conflict_term(value: &TreeValue) -> String { } } -fn get_file_contents(store: &Store, path: &RepoPath, term: &Option) -> Vec { +fn get_file_contents(store: &Store, path: &RepoPath, term: &Option) -> ContentHunk { match term { Some(id) => { let mut content = vec![]; @@ -367,11 +367,11 @@ fn get_file_contents(store: &Store, path: &RepoPath, term: &Option) -> V .unwrap() .read_to_end(&mut content) .unwrap(); - content + ContentHunk(content) } // If the conflict had removed the file on one side, we pretend that the file // was empty there. - None => vec![], + None => ContentHunk(vec![]), } } @@ -403,18 +403,26 @@ pub fn materialize_merge_result( single_hunk: &ConflictHunk, output: &mut dyn Write, ) -> std::io::Result<()> { - let removed_slices = single_hunk.removes.iter().map(Vec::as_slice).collect_vec(); - let added_slices = single_hunk.adds.iter().map(Vec::as_slice).collect_vec(); + let removed_slices = single_hunk + .removes + .iter() + .map(|hunk| hunk.0.as_slice()) + .collect_vec(); + let added_slices = single_hunk + .adds + .iter() + .map(|hunk| hunk.0.as_slice()) + .collect_vec(); let merge_result = files::merge(&removed_slices, &added_slices); match merge_result { MergeResult::Resolved(content) => { - output.write_all(&content)?; + output.write_all(&content.0)?; } MergeResult::Conflict(hunks) => { for hunk in hunks { match hunk { MergeHunk::Resolved(content) => { - output.write_all(&content)?; + output.write_all(&content.0)?; } MergeHunk::Conflict(ConflictHunk { removes, adds }) => { output.write_all(CONFLICT_START_LINE)?; @@ -426,25 +434,27 @@ pub fn materialize_merge_result( // If we have no more positive terms, emit the remaining negative // terms as snapshots. output.write_all(CONFLICT_MINUS_LINE)?; - output.write_all(left)?; + output.write_all(&left.0)?; continue; }; - let diff1 = Diff::for_tokenizer(&[left, right1], &find_line_ranges) - .hunks() - .collect_vec(); + let diff1 = + Diff::for_tokenizer(&[&left.0, &right1.0], &find_line_ranges) + .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 // any later positive terms. if let Some(right2) = adds.get(add_index + 1) { - let diff2 = Diff::for_tokenizer(&[left, right2], &find_line_ranges) - .hunks() - .collect_vec(); + let diff2 = + Diff::for_tokenizer(&[&left.0, &right2.0], &find_line_ranges) + .hunks() + .collect_vec(); if diff_size(&diff2) < diff_size(&diff1) { // If the next positive term is a better match, emit // the current positive term as a snapshot and the next // positive term as a diff. output.write_all(CONFLICT_PLUS_LINE)?; - output.write_all(right1)?; + output.write_all(&right1.0)?; output.write_all(CONFLICT_DIFF_LINE)?; write_diff_hunks(&diff2, output)?; add_index += 2; @@ -460,7 +470,7 @@ pub fn materialize_merge_result( // Emit the remaining positive terms as snapshots. for slice in &adds[add_index..] { output.write_all(CONFLICT_PLUS_LINE)?; - output.write_all(slice)?; + output.write_all(&slice.0)?; } output.write_all(CONFLICT_END_LINE)?; } @@ -507,7 +517,7 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti { let resolved_slice = &input[resolved_start..conflict_start.unwrap()]; if !resolved_slice.is_empty() { - hunks.push(MergeHunk::Resolved(resolved_slice.to_vec())); + hunks.push(MergeHunk::Resolved(ContentHunk(resolved_slice.to_vec()))); } hunks.push(hunk); resolved_start = pos + line.len(); @@ -523,7 +533,9 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti None } else { if resolved_start < input.len() { - hunks.push(MergeHunk::Resolved(input[resolved_start..].to_vec())); + hunks.push(MergeHunk::Resolved(ContentHunk( + input[resolved_start..].to_vec(), + ))); } Some(hunks) } @@ -543,18 +555,18 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk { match line { CONFLICT_DIFF_LINE => { state = State::Diff; - removes.push(vec![]); - adds.push(vec![]); + removes.push(ContentHunk(vec![])); + adds.push(ContentHunk(vec![])); continue; } CONFLICT_MINUS_LINE => { state = State::Minus; - removes.push(vec![]); + removes.push(ContentHunk(vec![])); continue; } CONFLICT_PLUS_LINE => { state = State::Plus; - adds.push(vec![]); + adds.push(ContentHunk(vec![])); continue; } _ => {} @@ -562,26 +574,26 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk { match state { State::Diff => { if let Some(rest) = line.strip_prefix(b"-") { - removes.last_mut().unwrap().extend_from_slice(rest); + removes.last_mut().unwrap().0.extend_from_slice(rest); } else if let Some(rest) = line.strip_prefix(b"+") { - adds.last_mut().unwrap().extend_from_slice(rest); + adds.last_mut().unwrap().0.extend_from_slice(rest); } else if let Some(rest) = line.strip_prefix(b" ") { - removes.last_mut().unwrap().extend_from_slice(rest); - adds.last_mut().unwrap().extend_from_slice(rest); + removes.last_mut().unwrap().0.extend_from_slice(rest); + adds.last_mut().unwrap().0.extend_from_slice(rest); } else { // Doesn't look like a conflict - return MergeHunk::Resolved(vec![]); + return MergeHunk::Resolved(ContentHunk(vec![])); } } State::Minus => { - removes.last_mut().unwrap().extend_from_slice(line); + removes.last_mut().unwrap().0.extend_from_slice(line); } State::Plus => { - adds.last_mut().unwrap().extend_from_slice(line); + adds.last_mut().unwrap().0.extend_from_slice(line); } State::Unknown => { // Doesn't look like a conflict - return MergeHunk::Resolved(vec![]); + return MergeHunk::Resolved(ContentHunk(vec![])); } } } diff --git a/lib/src/files.rs b/lib/src/files.rs index 0ee89af14..61e5dbea5 100644 --- a/lib/src/files.rs +++ b/lib/src/files.rs @@ -143,63 +143,32 @@ impl<'a> Iterator for DiffLineIterator<'a> { } #[derive(PartialEq, Eq, Clone)] -pub struct ConflictHunk { - pub removes: Vec>, - pub adds: Vec>, +pub struct ContentHunk(pub Vec); + +impl Debug for ContentHunk { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { + String::from_utf8_lossy(&self.0).fmt(f) + } } -#[derive(PartialEq, Eq, Clone)] +#[derive(PartialEq, Eq, Clone, Debug)] +pub struct ConflictHunk { + pub removes: Vec, + pub adds: Vec, +} + +#[derive(PartialEq, Eq, Clone, Debug)] pub enum MergeHunk { - Resolved(Vec), + Resolved(ContentHunk), Conflict(ConflictHunk), } -impl Debug for MergeHunk { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - match self { - MergeHunk::Resolved(data) => f - .debug_tuple("Resolved") - .field(&String::from_utf8_lossy(data)) - .finish(), - MergeHunk::Conflict(ConflictHunk { removes, adds }) => f - .debug_struct("Conflict") - .field( - "removes", - &removes - .iter() - .map(|part| String::from_utf8_lossy(part)) - .collect_vec(), - ) - .field( - "adds", - &adds - .iter() - .map(|part| String::from_utf8_lossy(part)) - .collect_vec(), - ) - .finish(), - } - } -} - -#[derive(PartialEq, Eq, Clone)] +#[derive(PartialEq, Eq, Clone, Debug)] pub enum MergeResult { - Resolved(Vec), + Resolved(ContentHunk), Conflict(Vec), } -impl Debug for MergeResult { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - match self { - MergeResult::Resolved(data) => f - .debug_tuple("Resolved") - .field(&String::from_utf8_lossy(data)) - .finish(), - MergeResult::Conflict(hunks) => f.debug_tuple("Conflict").field(hunks).finish(), - } - } -} - /// A region where the base and two sides match. #[derive(Debug, PartialEq, Eq, Clone)] struct SyncRegion { @@ -218,29 +187,29 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult { diff_inputs.extend(adds); let diff = Diff::for_tokenizer(&diff_inputs, &diff::find_line_ranges); - let mut resolved_hunk: Vec = vec![]; + let mut resolved_hunk = ContentHunk(vec![]); let mut merge_hunks: Vec = vec![]; for diff_hunk in diff.hunks() { match diff_hunk { DiffHunk::Matching(content) => { - resolved_hunk.extend(content); + resolved_hunk.0.extend(content); } DiffHunk::Different(parts) => { if let Some(resolved) = trivial_merge(&parts[..num_diffs], &parts[num_diffs..]) { - resolved_hunk.extend(*resolved); + resolved_hunk.0.extend(*resolved); } else { - if !resolved_hunk.is_empty() { + if !resolved_hunk.0.is_empty() { merge_hunks.push(MergeHunk::Resolved(resolved_hunk)); - resolved_hunk = vec![]; + resolved_hunk = ContentHunk(vec![]); } merge_hunks.push(MergeHunk::Conflict(ConflictHunk { removes: parts[..num_diffs] .iter() - .map(|part| part.to_vec()) + .map(|part| ContentHunk(part.to_vec())) .collect_vec(), adds: parts[num_diffs..] .iter() - .map(|part| part.to_vec()) + .map(|part| ContentHunk(part.to_vec())) .collect_vec(), })); } @@ -251,7 +220,7 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult { if merge_hunks.is_empty() { MergeResult::Resolved(resolved_hunk) } else { - if !resolved_hunk.is_empty() { + if !resolved_hunk.0.is_empty() { merge_hunks.push(MergeHunk::Resolved(resolved_hunk)); } MergeResult::Conflict(merge_hunks) @@ -262,114 +231,115 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult { mod tests { use super::*; + fn hunk(data: &[u8]) -> ContentHunk { + ContentHunk(data.to_vec()) + } + #[test] fn test_merge_single_hunk() { // Unchanged and empty on all sides - assert_eq!( - merge(&[b""], &[b"", b""]), - MergeResult::Resolved(b"".to_vec()) - ); + assert_eq!(merge(&[b""], &[b"", b""]), MergeResult::Resolved(hunk(b""))); // Unchanged on all sides assert_eq!( merge(&[b"a"], &[b"a", b"a"]), - MergeResult::Resolved(b"a".to_vec()) + MergeResult::Resolved(hunk(b"a")) ); // One side removed, one side unchanged assert_eq!( merge(&[b"a\n"], &[b"", b"a\n"]), - MergeResult::Resolved(b"".to_vec()) + MergeResult::Resolved(hunk(b"")) ); // One side unchanged, one side removed assert_eq!( merge(&[b"a\n"], &[b"a\n", b""]), - MergeResult::Resolved(b"".to_vec()) + MergeResult::Resolved(hunk(b"")) ); // Both sides removed same line assert_eq!( merge(&[b"a\n"], &[b"", b""]), - MergeResult::Resolved(b"".to_vec()) + MergeResult::Resolved(hunk(b"")) ); // One side modified, one side unchanged assert_eq!( merge(&[b"a"], &[b"a b", b"a"]), - MergeResult::Resolved(b"a b".to_vec()) + MergeResult::Resolved(hunk(b"a b")) ); // One side unchanged, one side modified assert_eq!( merge(&[b"a"], &[b"a", b"a b"]), - MergeResult::Resolved(b"a b".to_vec()) + MergeResult::Resolved(hunk(b"a b")) ); // All sides added same content assert_eq!( merge(&[b"", b""], &[b"a\n", b"a\n", b"a\n"]), - MergeResult::Resolved(b"a\n".to_vec()) + MergeResult::Resolved(hunk(b"a\n")) ); // One side modified, two sides added assert_eq!( merge(&[b"a", b""], &[b"b", b"b", b"b"]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a".to_vec(), b"".to_vec()], - adds: vec![b"b".to_vec(), b"b".to_vec(), b"b".to_vec()] + removes: vec![hunk(b"a"), hunk(b"")], + adds: vec![hunk(b"b"), hunk(b"b"), hunk(b"b")] })]) ); // All sides removed same content assert_eq!( merge(&[b"a\n", b"a\n", b"a\n"], &[b"", b"", b"", b""]), - MergeResult::Resolved(b"".to_vec()) + MergeResult::Resolved(hunk(b"")) ); // One side modified, two sides removed assert_eq!( merge(&[b"a\n", b"a\n"], &[b"b\n", b"", b""]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a\n".to_vec(), b"a\n".to_vec()], - adds: vec![b"b\n".to_vec(), b"".to_vec(), b"".to_vec()] + removes: vec![hunk(b"a\n"), hunk(b"a\n")], + adds: vec![hunk(b"b\n"), hunk(b""), hunk(b"")] })]) ); // Three sides made the same change assert_eq!( merge(&[b"a", b"a"], &[b"b", b"b", b"b"]), - MergeResult::Resolved(b"b".to_vec()) + MergeResult::Resolved(hunk(b"b")) ); // One side removed, one side modified assert_eq!( merge(&[b"a\n"], &[b"", b"b\n"]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a\n".to_vec()], - adds: vec![b"".to_vec(), b"b\n".to_vec()] + removes: vec![hunk(b"a\n")], + adds: vec![hunk(b""), hunk(b"b\n")] })]) ); // One side modified, one side removed assert_eq!( merge(&[b"a\n"], &[b"b\n", b""]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a\n".to_vec()], - adds: vec![b"b\n".to_vec(), b"".to_vec()] + removes: vec![hunk(b"a\n")], + adds: vec![hunk(b"b\n"), hunk(b"")] })]) ); // Two sides modified in different ways assert_eq!( merge(&[b"a"], &[b"b", b"c"]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a".to_vec()], - adds: vec![b"b".to_vec(), b"c".to_vec()] + removes: vec![hunk(b"a")], + adds: vec![hunk(b"b"), hunk(b"c")] })]) ); // Two of three sides don't change, third side changes assert_eq!( merge(&[b"a", b"a"], &[b"a", b"", b"a"]), - MergeResult::Resolved(b"".to_vec()) + MergeResult::Resolved(hunk(b"")) ); // One side unchanged, two other sides make the same change assert_eq!( merge(&[b"a", b"a"], &[b"b", b"a", b"b"]), - MergeResult::Resolved(b"b".to_vec()) + MergeResult::Resolved(hunk(b"b")) ); // One side unchanged, two other sides make the different change assert_eq!( merge(&[b"a", b"a"], &[b"b", b"a", b"c"]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a".to_vec(), b"a".to_vec()], - adds: vec![b"b".to_vec(), b"a".to_vec(), b"c".to_vec()] + removes: vec![hunk(b"a"), hunk(b"a")], + adds: vec![hunk(b"b"), hunk(b"a"), hunk(b"c")] })]) ); // Merge of an unresolved conflict and another branch, where the other branch @@ -377,22 +347,22 @@ mod tests { // first. assert_eq!( merge(&[b"a", b"b"], &[b"b", b"a", b"c"]), - MergeResult::Resolved(b"c".to_vec()) + MergeResult::Resolved(hunk(b"c")) ); // Merge of an unresolved conflict and another branch. assert_eq!( merge(&[b"a", b"b"], &[b"c", b"d", b"e"]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a".to_vec(), b"b".to_vec()], - adds: vec![b"c".to_vec(), b"d".to_vec(), b"e".to_vec()] + removes: vec![hunk(b"a"), hunk(b"b")], + adds: vec![hunk(b"c"), hunk(b"d"), hunk(b"e")] })]) ); // Two sides made the same change, third side made a different change assert_eq!( merge(&[b"a", b"b"], &[b"c", b"c", b"c"]), MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk { - removes: vec![b"a".to_vec(), b"b".to_vec()], - adds: vec![b"c".to_vec(), b"c".to_vec(), b"c".to_vec()] + removes: vec![hunk(b"a"), hunk(b"b")], + adds: vec![hunk(b"c"), hunk(b"c"), hunk(b"c")] })]) ); } @@ -403,28 +373,28 @@ mod tests { assert_eq!( merge(&[b"a\n"], &[b"a\nb\n", b"a\nc\n"]), MergeResult::Conflict(vec![ - MergeHunk::Resolved(b"a\n".to_vec()), + MergeHunk::Resolved(hunk(b"a\n")), MergeHunk::Conflict(ConflictHunk { - removes: vec![b"".to_vec()], - adds: vec![b"b\n".to_vec(), b"c\n".to_vec()] + removes: vec![hunk(b"")], + adds: vec![hunk(b"b\n"), hunk(b"c\n")] }) ]) ); // Two sides changed different lines: no conflict assert_eq!( merge(&[b"a\nb\nc\n"], &[b"a2\nb\nc\n", b"a\nb\nc2\n"]), - MergeResult::Resolved(b"a2\nb\nc2\n".to_vec()) + MergeResult::Resolved(hunk(b"a2\nb\nc2\n")) ); // Conflict with non-conflicting lines around assert_eq!( merge(&[b"a\nb\nc\n"], &[b"a\nb1\nc\n", b"a\nb2\nc\n"]), MergeResult::Conflict(vec![ - MergeHunk::Resolved(b"a\n".to_vec()), + MergeHunk::Resolved(hunk(b"a\n")), MergeHunk::Conflict(ConflictHunk { - removes: vec![b"b\n".to_vec()], - adds: vec![b"b1\n".to_vec(), b"b2\n".to_vec()] + removes: vec![hunk(b"b\n")], + adds: vec![hunk(b"b1\n"), hunk(b"b2\n")] }), - MergeHunk::Resolved(b"c\n".to_vec()) + MergeHunk::Resolved(hunk(b"c\n")) ]) ); // One side changes a line and adds a block after. The other side just adds the @@ -460,7 +430,7 @@ b { " ] ), - MergeResult::Resolved( + MergeResult::Resolved(hunk( b"\ a { q @@ -470,8 +440,7 @@ b { x } " - .to_vec() - ) + )) ); } } diff --git a/lib/src/tree.rs b/lib/src/tree.rs index 4e91e6ae6..f83d0b2de 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -699,7 +699,7 @@ fn try_resolve_file_conflict( ); match merge_result { MergeResult::Resolved(merged_content) => { - let id = store.write_file(filename, &mut merged_content.as_slice())?; + let id = store.write_file(filename, &mut merged_content.0.as_slice())?; Ok(Some(TreeValue::File { id, executable })) } MergeResult::Conflict(_) => Ok(None), diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index f31e062c5..560f364e9 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -320,27 +320,31 @@ line 5 right @r###" Some( [ - Conflict { - removes: [ - "line 1\nline 2\n", - ], - adds: [ - "line 1 left\nline 2 left\n", - "line 1 right\nline 2\n", - ], - }, + Conflict( + ConflictHunk { + removes: [ + "line 1\nline 2\n", + ], + adds: [ + "line 1 left\nline 2 left\n", + "line 1 right\nline 2\n", + ], + }, + ), Resolved( "line 3\n", ), - Conflict { - removes: [ - "line 4\nline 5\n", - ], - adds: [ - "line 4\nline 5 left\n", - "line 4 right\nline 5 right\n", - ], - }, + Conflict( + ConflictHunk { + removes: [ + "line 4\nline 5\n", + ], + adds: [ + "line 4\nline 5 left\n", + "line 4 right\nline 5 right\n", + ], + }, + ), ], ) "###); @@ -488,15 +492,17 @@ line 5 Resolved( "line 1\n", ), - Conflict { - removes: [ - "line 2\nline 3\nline 4\n", - ], - adds: [ - "line 2\nleft\nline 4\n", - "right\n", - ], - }, + Conflict( + ConflictHunk { + removes: [ + "line 2\nline 3\nline 4\n", + ], + adds: [ + "line 2\nleft\nline 4\n", + "right\n", + ], + }, + ), Resolved( "line 5\n", ), @@ -536,17 +542,19 @@ line 5 Resolved( "line 1\n", ), - Conflict { - removes: [ - "line 2\nline 3\nline 4\n", - "line 2\nline 3\nline 4\n", - ], - adds: [ - "line 2\nleft\nline 4\n", - "right\n", - "line 2\nforward\nline 3\nline 4\n", - ], - }, + Conflict( + ConflictHunk { + removes: [ + "line 2\nline 3\nline 4\n", + "line 2\nline 3\nline 4\n", + ], + adds: [ + "line 2\nleft\nline 4\n", + "right\n", + "line 2\nforward\nline 3\nline 4\n", + ], + }, + ), Resolved( "line 5\n", ), diff --git a/src/merge_tools.rs b/src/merge_tools.rs index 57400c320..65cc3380a 100644 --- a/src/merge_tools.rs +++ b/src/merge_tools.rs @@ -187,9 +187,9 @@ pub fn run_mergetool( vec![] }; let files: HashMap<&str, _> = maplit::hashmap! { - "base" => content.removes.pop().unwrap(), - "right" => content.adds.pop().unwrap(), - "left" => content.adds.pop().unwrap(), + "base" => content.removes.pop().unwrap().0, + "right" => content.adds.pop().unwrap().0, + "left" => content.adds.pop().unwrap().0, "output" => initial_output_content.clone(), };