I had another look at that patch and I think it is an improvement. It
basically factorizes a merge case that should not depend on the
particular implementations of RepoPatchV1 or RepoPatchV2.
To summarize, the RepoPatchV1 code goes from:
~~~
actualMerge (p1 :\/: p2) = case elegantMerge (p1:\/:p2) of
....
-- from a common starting context @wX@.
elegantMerge :: PrimPatch prim
=> (RepoPatchV1 prim :\/: RepoPatchV1 prim) wX wY
-> Maybe ((RepoPatchV1 prim :/\: RepoPatchV1 prim) wX wY)
elegantMerge (p1 :\/: p2) = do
p1' :> ip2' <- commute (invert p2 :> p1)
p1o :> _ <- commute (p2 :> p1')
guard $ unsafeCompare p1o p1 -- should be a redundant check
return $ invert ip2' :/\: p1'
~~~
to:
~~~
actualMerge (p1 :\/: p2) = case naturalMerge (p1:\/:p2) of
...
-- | The natural, non-conflicting merge.
naturalMerge :: (Invert p, Commute p)
=> (p :\/: p) wX wY -> Maybe ((p :/\: p) wX wY)
naturalMerge (p :\/: q) = do
q' :> ip' <- commute (invert p :> q)
-- TODO: find a small convincing example that demonstrates why
-- it is necessary to do this extra check here
_ <- commute (p :> q')
return (q' :/\: invert ip')
~~~
And RepoPatchV2 goes from:
~~~
merge (x :\/: y)
| Just (y' :> ix') <-
commute (invert (assertConsistent x) :> assertConsistent y)
, Just (y'' :> _) <- commute (x :> y')
, IsEq <- y'' =\/= y =
assertConsistent y' :/\: invert (assertConsistent ix')
-- If we detect equal patches, we have a duplicate.
| IsEq <- x =\/= y
, n <- commuteOrAddToCtx (invert x) $ non x =
Duplicate n :/\: Duplicate n
~~~
To:
~~~
merge (x :\/: y)
-- First try the natural (non-conflicting) merge.
| Just (y' :/\: x') <-
naturalMerge ((assertConsistent x) :\/: (assertConsistent y))
= assertConsistent y' :/\: assertConsistent x'
-- If we detect equal patches, we have a duplicate.
| IsEq <- x =\/= y
, n <- commuteOrAddToCtx (invert x) $ non x =
Duplicate n :/\: Duplicate n
-- | The natural, non-conflicting merge.
naturalMerge :: (Invert p, Commute p)
=> (p :\/: p) wX wY -> Maybe ((p :/\: p) wX wY)
naturalMerge (p :\/: q) = do
q' :> ip' <- commute (invert p :> q)
-- TODO: find a small convincing example that demonstrates why
-- it is necessary to do this extra check here
_ <- commute (p :> q')
return (q' :/\: invert ip')
~~~
Accepted.
|
