diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index d29e660e4..28310d537 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -12,7 +12,6 @@ // 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; @@ -102,9 +101,8 @@ fn get_file_contents(store: &Store, path: &RepoPath, part: &ConflictPart) -> Vec } } -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() { +fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result<()> { + for hunk in hunks { match hunk { DiffHunk::Matching(content) => { for line in content.split_inclusive(|b| *b == b'\n') { @@ -164,24 +162,39 @@ pub fn materialize_conflict( MergeHunk::Resolved(content) => { output.write_all(&content)?; } - 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 + MergeHunk::Conflict { + mut removes, + mut adds, + } => { output.write_all(CONFLICT_START_LINE)?; - for i in 0..num_diffs { + while !removes.is_empty() && !adds.is_empty() { + let left = &removes[0]; + let mut diffs = vec![]; + for right in &adds { + diffs.push( + Diff::for_tokenizer(&[left, right], &find_line_ranges) + .hunks() + .collect_vec(), + ); + } + let min_diff_index = diffs + .iter() + .position_min_by_key(|diff| diff_size(*diff)) + .unwrap(); output.write_all(CONFLICT_MINUS_LINE)?; output.write_all(CONFLICT_PLUS_LINE)?; - write_diff_hunks(&removes[i], &adds[i], output)?; + write_diff_hunks(&diffs[min_diff_index], output)?; + removes.remove(0); + adds.remove(min_diff_index); } - for slice in removes.iter().skip(num_diffs) { + + for slice in removes { output.write_all(CONFLICT_MINUS_LINE)?; - output.write_all(slice)?; + output.write_all(&slice)?; } - for slice in adds.iter().skip(num_diffs) { + for slice in adds { output.write_all(CONFLICT_PLUS_LINE)?; - output.write_all(slice)?; + output.write_all(&slice)?; } output.write_all(CONFLICT_END_LINE)?; } @@ -192,6 +205,16 @@ pub fn materialize_conflict( Ok(()) } +fn diff_size(hunks: &[DiffHunk]) -> usize { + hunks + .iter() + .map(|hunk| match hunk { + DiffHunk::Matching(_) => 0, + DiffHunk::Different(slices) => slices.iter().map(|slice| slice.len()).sum(), + }) + .sum() +} + pub fn conflict_to_materialized_value( store: &Store, path: &RepoPath, diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 28a1c3fb9..de2f6c8ab 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -41,7 +41,9 @@ line 5 &path, "line 1 line 2 -left +left 3.1 +left 3.2 +left 3.3 line 4 line 5 ", @@ -51,13 +53,13 @@ line 5 &path, "line 1 line 2 -right +right 3.1 line 4 line 5 ", ); - let conflict = Conflict { + let mut conflict = Conflict { removes: vec![ConflictPart { value: TreeValue::Normal { id: base_id, @@ -88,9 +90,32 @@ line 5 ------- +++++++ -line 3 - +left + +right 3.1 +++++++ - right + left 3.1 + left 3.2 + left 3.3 + >>>>>>> + line 4 + line 5 + "### + ); + // Test with the larger diff first. We still want the small diff. + conflict.adds.reverse(); + insta::assert_snapshot!( + &materialize_conflict_string(store, &path, &conflict), + @r###" + line 1 + line 2 + <<<<<<< + ------- + +++++++ + -line 3 + +right 3.1 + +++++++ + left 3.1 + left 3.2 + left 3.3 >>>>>>> line 4 line 5 @@ -163,8 +188,8 @@ line 5 ------- +++++++ -line 3 - +left +++++++ + left >>>>>>> line 4 line 5