From dd26b7be40c44242e24f0d05538c315422b04537 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 5 Nov 2023 11:15:26 +0900 Subject: [PATCH] merge: add Merge constructor that accepts interleaved values Also migrated some callers of 3-way merge, where [left, base, right] order looks okay. --- cli/src/merge_tools/builtin.rs | 12 +++++------- lib/src/merge.rs | 16 ++++++++++++---- lib/src/merged_tree.rs | 5 +---- lib/src/refs.rs | 9 +++++---- lib/src/tree.rs | 9 +++++---- lib/tests/test_refs.rs | 18 ++++++++++-------- 6 files changed, 38 insertions(+), 31 deletions(-) diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 200ec2a73..ecf31a00a 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -722,13 +722,11 @@ mod tests { } } } - let merge = Merge::new( - vec![to_file_id(base_tree.path_value(&path))], - vec![ - to_file_id(left_tree.path_value(&path)), - to_file_id(right_tree.path_value(&path)), - ], - ); + let merge = Merge::from_vec(vec![ + to_file_id(left_tree.path_value(&path)), + to_file_id(base_tree.path_value(&path)), + to_file_id(right_tree.path_value(&path)), + ]); let content = extract_as_single_hunk(&merge, store, &path).block_on(); let slices = content.map(|ContentHunk(buf)| buf.as_slice()); let merge_result = files::merge(&slices); diff --git a/lib/src/merge.rs b/lib/src/merge.rs index d55a12469..05e75a0bd 100644 --- a/lib/src/merge.rs +++ b/lib/src/merge.rs @@ -132,6 +132,17 @@ impl Debug for Merge { } impl Merge { + /// Creates a `Merge` from the given values, in which positive and negative + /// terms alternate. + pub fn from_vec(values: impl Into>) -> Self { + let values = values.into(); + assert!( + values.len() & 1 != 0, + "must have one more adds than removes" + ); + Merge { values } + } + /// Creates a new merge object from the given removes and adds. pub fn new(removes: Vec, adds: Vec) -> Self { // TODO: removes and adds can be just IntoIterator. @@ -335,10 +346,7 @@ impl MergeBuilder { /// Requires that exactly one more "adds" than "removes" have been added to /// this builder. pub fn build(self) -> Merge { - assert!(self.values.len() & 1 != 0); - Merge { - values: self.values, - } + Merge::from_vec(self.values) } } diff --git a/lib/src/merged_tree.rs b/lib/src/merged_tree.rs index 72202beb5..58807a18a 100644 --- a/lib/src/merged_tree.rs +++ b/lib/src/merged_tree.rs @@ -414,10 +414,7 @@ impl MergedTree { MergedTree::Merge(conflict) => Ok(conflict.clone()), } }; - let nested = Merge::new( - vec![to_merge(base)?], - vec![to_merge(self)?, to_merge(other)?], - ); + let nested = Merge::from_vec(vec![to_merge(self)?, to_merge(base)?, to_merge(other)?]); let tree = merge_trees(&nested.flatten().simplify())?; // If the result can be resolved, then `merge_trees()` above would have returned // a resolved merge. However, that function will always preserve the arity of diff --git a/lib/src/refs.rs b/lib/src/refs.rs index 7b033a8c5..effd81a02 100644 --- a/lib/src/refs.rs +++ b/lib/src/refs.rs @@ -93,10 +93,11 @@ pub fn merge_ref_targets( return resolved.clone(); } - let mut merge = Merge::new( - vec![base.as_merge().clone()], - vec![left.as_merge().clone(), right.as_merge().clone()], - ) + let mut merge = Merge::from_vec(vec![ + left.as_merge().clone(), + base.as_merge().clone(), + right.as_merge().clone(), + ]) .flatten() .simplify(); if !merge.is_resolved() { diff --git a/lib/src/tree.rs b/lib/src/tree.rs index 373438e22..753eb7d3b 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -373,10 +373,11 @@ fn merge_tree_value( _ => { // Start by creating a Merge object. Merges can cleanly represent a single // resolved state, the absence of a state, or a conflicted state. - let conflict = Merge::new( - vec![maybe_base.cloned()], - vec![maybe_side1.cloned(), maybe_side2.cloned()], - ); + let conflict = Merge::from_vec(vec![ + maybe_side1.cloned(), + maybe_base.cloned(), + maybe_side2.cloned(), + ]); let filename = dir.join(basename); let expanded = conflict.try_map(|term| match term { Some(TreeValue::Conflict(id)) => store.read_conflict(&filename, id), diff --git a/lib/tests/test_refs.rs b/lib/tests/test_refs.rs index a6c7ccfab..ca6dbe3c2 100644 --- a/lib/tests/test_refs.rs +++ b/lib/tests/test_refs.rs @@ -167,19 +167,21 @@ fn test_merge_ref_targets() { // Left removed, right moved forward assert_eq!( merge_ref_targets(index, RefTarget::absent_ref(), &target1, &target3), - RefTarget::from_merge(Merge::new( - vec![Some(commit1.id().clone())], - vec![None, Some(commit3.id().clone())], - )) + RefTarget::from_merge(Merge::from_vec(vec![ + None, + Some(commit1.id().clone()), + Some(commit3.id().clone()), + ])) ); // Right removed, left moved forward assert_eq!( merge_ref_targets(index, &target3, &target1, RefTarget::absent_ref()), - RefTarget::from_merge(Merge::new( - vec![Some(commit1.id().clone())], - vec![Some(commit3.id().clone()), None], - )) + RefTarget::from_merge(Merge::from_vec(vec![ + Some(commit3.id().clone()), + Some(commit1.id().clone()), + None, + ])) ); // Left became conflicted, right moved forward