ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

conflicts: resolve trivial merge of A+A-B+C-C to A

Browse source 
This changes the behavior in one of the cases ilyagr@
[mentioned](https://github.com/martinvonz/jj/pull/1610#discussion_r1199823932)
to match his suggestion. After some more thinking while working on
tree-level conflicts, I now think it's clear that the added `+C-C`
terms should have no effect on the result. A very similar argument is
that `Conflict::simplify()` should not change the result of
`trivial_merge()`. I'll add tests for that next.
This commit is contained in:
Martin von Zweigbergk 2023-06-13 04:30:00 -07:00 committed by Martin von Zweigbergk
parent 82883e648d
commit 3f95dafd67
2 changed files with 37 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
lib/src/files.rs
View file

@ -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!(

66
lib/src/merge.rs
View file

@ -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);