diff --git a/lib/src/files.rs b/lib/src/files.rs index 5beac6a9c..0ee89af14 100644 --- a/lib/src/files.rs +++ b/lib/src/files.rs @@ -362,10 +362,7 @@ mod tests { // One side unchanged, two other sides make the same change assert_eq!( merge(&[b"a", b"a"], &[b"b", b"a", b"b"]), - 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"b".to_vec()] - })]) + MergeResult::Resolved(b"b".to_vec()) ); // One side unchanged, two other sides make the different change assert_eq!( diff --git a/lib/src/merge.rs b/lib/src/merge.rs index a098f427d..8d8a85fe6 100644 --- a/lib/src/merge.rs +++ b/lib/src/merge.rs @@ -42,17 +42,6 @@ where }; } - // Check if all sides made the same change. - // This matches what Git and Mercurial do (in the 3-way case at least), but not - // what Darcs and Pijul do. It means that repeated 3-way merging of multiple - // trees may give different results depending on the order of merging. - // TODO: Consider removing this special case, making the algorithm more strict, - // and maybe add a more lenient version that is used when the user explicitly - // asks for conflict resolution. - if removes.iter().all_equal() && adds.iter().all_equal() { - return Some(&adds[0]); - } - // Number of occurrences of each value, with positive indexes counted as +1 and // negative as -1, thereby letting positive and negative terms with the same // value (i.e. key in the map) cancel each other. @@ -64,15 +53,32 @@ where counts.entry(value).and_modify(|e| *e -= 1).or_insert(-1); } - // If there is a single value (i.e. key in the map) with a count of 1 left, then - // that is the result. Values with a count of 0 means that they have - // cancelled out, so we skip them. - let x = counts - .iter() - .filter(|&(_, count)| *count != 0) + // Collect non-zero value. Values with a count of 0 means that they have + // cancelled out. + let counts = counts + .into_iter() + .filter(|&(_, count)| count != 0) .collect_vec(); - match x[..] { - [(value, 1)] => Some(value), + match counts[..] { + [(value, 1)] => { + // If there is a single value with a count of 1 left, then that is the result. + Some(value) + } + [(value1, count1), (value2, count2)] => { + // All sides made the same change. + // This matches what Git and Mercurial do (in the 3-way case at least), but not + // what Darcs and Pijul do. It means that repeated 3-way merging of multiple + // trees may give different results depending on the order of merging. + // TODO: Consider removing this special case, making the algorithm more strict, + // and maybe add a more lenient version that is used when the user explicitly + // asks for conflict resolution. + assert_eq!(count1 + count2, 1); + if count1 > 0 { + Some(value1) + } else { + Some(value2) + } + } _ => None, } } @@ -92,19 +98,19 @@ mod tests { assert_eq!(trivial_merge(&[0, 0], &[0, 0, 0]), Some(&0)); assert_eq!(trivial_merge(&[0, 0], &[0, 0, 1]), Some(&1)); assert_eq!(trivial_merge(&[0, 0], &[0, 1, 0]), Some(&1)); - assert_eq!(trivial_merge(&[0, 0], &[0, 1, 1]), None); + assert_eq!(trivial_merge(&[0, 0], &[0, 1, 1]), Some(&1)); assert_eq!(trivial_merge(&[0, 0], &[0, 1, 2]), None); assert_eq!(trivial_merge(&[0, 0], &[1, 0, 0]), Some(&1)); - assert_eq!(trivial_merge(&[0, 0], &[1, 0, 1]), None); + assert_eq!(trivial_merge(&[0, 0], &[1, 0, 1]), Some(&1)); assert_eq!(trivial_merge(&[0, 0], &[1, 0, 2]), None); - assert_eq!(trivial_merge(&[0, 0], &[1, 1, 0]), None); + assert_eq!(trivial_merge(&[0, 0], &[1, 1, 0]), Some(&1)); assert_eq!(trivial_merge(&[0, 0], &[1, 1, 1]), Some(&1)); assert_eq!(trivial_merge(&[0, 0], &[1, 1, 2]), None); assert_eq!(trivial_merge(&[0, 0], &[1, 2, 0]), None); assert_eq!(trivial_merge(&[0, 0], &[1, 2, 1]), None); assert_eq!(trivial_merge(&[0, 0], &[1, 2, 2]), None); assert_eq!(trivial_merge(&[0, 0], &[1, 2, 3]), None); - assert_eq!(trivial_merge(&[0, 1], &[0, 0, 0]), None); + assert_eq!(trivial_merge(&[0, 1], &[0, 0, 0]), Some(&0)); assert_eq!(trivial_merge(&[0, 1], &[0, 0, 1]), Some(&0)); assert_eq!(trivial_merge(&[0, 1], &[0, 0, 2]), None); assert_eq!(trivial_merge(&[0, 1], &[0, 1, 0]), Some(&0)); @@ -112,28 +118,28 @@ mod tests { assert_eq!(trivial_merge(&[0, 1], &[0, 1, 2]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[0, 2, 0]), None); assert_eq!(trivial_merge(&[0, 1], &[0, 2, 1]), Some(&2)); - assert_eq!(trivial_merge(&[0, 1], &[0, 2, 2]), None); + assert_eq!(trivial_merge(&[0, 1], &[0, 2, 2]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[0, 2, 3]), None); assert_eq!(trivial_merge(&[0, 1], &[1, 0, 0]), Some(&0)); assert_eq!(trivial_merge(&[0, 1], &[1, 0, 1]), Some(&1)); assert_eq!(trivial_merge(&[0, 1], &[1, 0, 2]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[1, 1, 0]), Some(&1)); - assert_eq!(trivial_merge(&[0, 1], &[1, 1, 1]), None); + assert_eq!(trivial_merge(&[0, 1], &[1, 1, 1]), Some(&1)); assert_eq!(trivial_merge(&[0, 1], &[1, 1, 2]), None); assert_eq!(trivial_merge(&[0, 1], &[1, 2, 0]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[1, 2, 1]), None); - assert_eq!(trivial_merge(&[0, 1], &[1, 2, 2]), None); + assert_eq!(trivial_merge(&[0, 1], &[1, 2, 2]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[1, 2, 3]), None); assert_eq!(trivial_merge(&[0, 1], &[2, 0, 0]), None); assert_eq!(trivial_merge(&[0, 1], &[2, 0, 1]), Some(&2)); - assert_eq!(trivial_merge(&[0, 1], &[2, 0, 2]), None); + assert_eq!(trivial_merge(&[0, 1], &[2, 0, 2]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[2, 0, 3]), None); assert_eq!(trivial_merge(&[0, 1], &[2, 1, 0]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[2, 1, 1]), None); - assert_eq!(trivial_merge(&[0, 1], &[2, 1, 2]), None); + assert_eq!(trivial_merge(&[0, 1], &[2, 1, 2]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[2, 1, 3]), None); - assert_eq!(trivial_merge(&[0, 1], &[2, 2, 0]), None); - assert_eq!(trivial_merge(&[0, 1], &[2, 2, 1]), None); + assert_eq!(trivial_merge(&[0, 1], &[2, 2, 0]), Some(&2)); + assert_eq!(trivial_merge(&[0, 1], &[2, 2, 1]), Some(&2)); assert_eq!(trivial_merge(&[0, 1], &[2, 2, 2]), None); assert_eq!(trivial_merge(&[0, 1], &[2, 2, 3]), None); assert_eq!(trivial_merge(&[0, 1], &[2, 3, 0]), None);