====================================================================== coalesce just-created files with replace patches. ======================================================================
> +coalesceFilePatch f (TokReplace _ _ _ :< AddFile) = Just $ FP f AddFile
> +coalesceFilePatch f (RmFile :< TokReplace _ _ _) = Just $ FP f RmFile
Makes sense. This only affects token-replaces on empty files; the
token-replace just disappears.
======================================================================
define and use Prim patch type
======================================================================
This further refines the big blob of type we call Patch. Now there are
four distinct types of patch that darcs manipulates:
- Prim (primitive patches): addfile, hunk, tokreplace, etc
in the user interface, darcs calls these 'changes'
- Patch (composite patches and mergers)
- ConflictedPatch
for darcs 2.0
- NamedPatch
this is what the user interface calls a patch, and what
other version control systems call a changeset
Sorry David, I was a bit overwhelmed by this patch and was not able to
review it properly.
> hunk ./src/Darcs/Patch/Apply.lhs 317
> +markup_file x p = mps (flatten_to_primitivesFL p)
> + where mps :: FL Prim C(a b) -> (FilePath, MarkedUpFile) -> (FilePath,
> MarkedUpFile)
> + mps NilFL = id
> + mps (pp:>:pps) = mps pps . markup_prim x pp
fold?
> hunk ./src/Darcs/Patch/Apply.lhs 414
> +applyToPop :: PatchInfo -> FL Prim C(x y) -> Population -> Population
> +applyToPop _ NilFL = id
> +applyToPop pinf (p:>:ps) = applyToPop pinf ps . applyToPop' pinf p
likewise, sounds like there is a fold or something similar in here
> +flatten_to_primitivesFL :: Patch C(x y) -> FL Prim C(x y)
> +flatten_to_primitivesFL p@(Merger _ _ _ _) =
> + sort_coalesceFL $ flatten_to_primitivesFL $ merger_undo p
> +flatten_to_primitivesFL p@(Regrem _ _ _ _) =
> + invert $ flatten_to_primitivesFL $ invert p
> +flatten_to_primitivesFL (ComP ps) =
> + concatFL $ mapFL_FL flatten_to_primitivesFL ps
> +flatten_to_primitivesFL (PP p) = p :>: NilFL
> hunk ./src/Darcs/Patch/Core.lhs 80
> is_null_patch :: Patch C(x y) -> Bool
> is_null_patch (ComP ps) = and $ mapFL is_null_patch ps
> -is_null_patch (FP _ (Binary x y)) = nullPS x && nullPS y
> -is_null_patch (FP _ (Hunk _ [] [])) = True
> is_null_patch _ = False
Should I be worried about this? Do we need to at least check for the
new Identity primitive patch?
> -{- INLINE flatten_to_primitivesFL -}
> -flatten_to_primitivesFL :: Patch C(a b) -> FL Patch C(a b)
> -flatten_to_primitivesFL (ComP ps) = concatFL $ mapFL_FL
> flatten_to_primitivesFL ps
> -flatten_to_primitivesFL p = p :>: NilFL
Made fancier in Darcs.Patch.Commute to return primitive patches. The
fancy version looks inside mergers as well. I wonder if this changes
things in a bad way...
> hunk ./src/Darcs/Patch/Core.lhs 204
> - invert (Split ps) = Split $ invert ps
Why are Split patches considered primitive patches?
> hunk ./src/Darcs/Resolution.lhs
> standard_resolution :: Patch C(x y) -> Sealed (FL Prim C(y))
> standard_resolution
> - return [p,merge_list $ map head $ resolve_conflicts p]
> + Sealed $ flatten_to_primitivesFL
> + (merge_list $ map head $ resolve_conflicts p)
Not sure what is going on in these resolution functions. Why do we no
longer need the patch p?
> hunk ./src/Darcs/Resolution.lhs 157
> -external_resolution :: Slurpy -> String -> Patch -> Patch -> Patch -> IO
> [Patch]
> +external_resolution :: Slurpy -> String -> FL Prim -> FL Prim -> Patch
> + -> IO (Sealed (FL Prim))
> external_resolution s1 c p1 p2 pmerged = do
> sa <- apply_to_slurpy (invert p1) s1
> sm <- apply_to_slurpy pmerged s1
> hunk ./src/Darcs/Resolution.lhs 189
> sfixed <- slurp dm
> ftf <- filetype_function
> case smart_diff [LookForAdds] ftf sc sfixed of
> - Nothing -> return [pmerged]
> - Just di -> length (show di) `seq` return [pmerged,di]
> + di -> lengthFL di `seq` return (Sealed di)
Likewise unsure why we no longer need this pmerged patch...
======================================================================
switch over to FL for SelectPatches.
======================================================================
Seems ok overall. I got confused by things getting flipped around,
especially since not everything gets flipped, but I'm assuming you
know what you're doing here.
The increasing use of FL/RL means that we push the unsafeFL/unsafeUnFL
calls further away from the core, which seems nice.
> hunk ./src/Darcs/Patch/Choices.lhs
> -pull_only_firsts :: Patchy p => EasyPC p -> ([TaggedPatch p], [TaggedPatch
> p])
> +pull_only_firsts :: Patchy p => EasyPC p -> (FL (TaggedPatch p) :> FL
> (TaggedPatch p))
> pull_only_firsts easyPC =
> + in (xs :> unsafePerformIO (readIORef r))
> +pull_only_firsts :: Patchy p => EasyPC p -> (FL (TaggedPatch p) :> FL
> (TaggedPatch p))
> - in (xs, unsafePerformIO (readIORef r))
> + in (xs :> unsafePerformIO (readIORef r))
Are these the expected order? (as opposed to r :> xs, or to xs :< r)?
> hunk ./src/Darcs/Patch/Choices.lhs 278
> - not_needed = map thd $ snd $ pull_lasts $ set_simplys xs False e
> + not_needed = case pull_lasts $ set_simplys xs False e of
> + rest :> _ -> mapFL thd rest
Note: contrast with (_ :> rest) in force_matching_first. But looks very
much intentional (pull_lasts returns inverted results)
> +zipWithFL :: (FORALL(x y) a -> p C(x y) -> q C(x y))
> + -> [a] -> FL p C(w z) -> FL q C(w z)
> +zipWithFL f (x:xs) (y :>: ys) = f x y :>: zipWithFL f xs ys
> +zipWithFL _ _ NilFL = NilFL
> +zipWithFL _ [] (_:>:_) = bug "zipWithFL called with too short a list"
Should zipWithFL _ xs NilFL also be an error (as opposed to zipWithFL _
[] NilFL?)
> if All `elem` opts || DryRun `elem` opts
> - then job $ case (islast, isreversed) of
> - (True, False) -> (ps_to_consider, other_ps)
> - (False, False) -> (other_ps, ps_to_consider)
> - (True, True) -> (unsafeUnFL $ invert $ unsafeFL other_ps,
> - unsafeUnFL $ invert $ unsafeFL ps_to_consider)
> - (False, True) -> (unsafeUnFL $ invert $ unsafeFL ps_to_consider,
> - unsafeUnFL $ invert $ unsafeFL other_ps)
> + then job $ if isreversed then invert other_ps :> invert ps_to_consider
> + else ps_to_consider :> other_ps
Ok. We no longer flip arguments around because patches_to_consider
takes care of it. In the old code 'ps_to_consider' and 'other_ps' meant
different things depending on islast. In this new version, they have
consistent meanings, but care is taken to assign them properly
elsewhere...
> hunk ./src/Darcs/SelectChanges.lhs 255
> - where (ps_to_consider, other_ps) = p2c ps
> - (init_pc, init_tps) = patch_choices_tps ps_to_consider
> + where ps_to_consider :> other_ps = p2c ps
> + (init_pc, init_tps) = patch_choices_tps $ if islast then other_ps
> + else
> ps_to_consider
Notably, here.
> hunk ./src/Darcs/SelectChanges.lhs 271
> - ps' = cleanup ps
> - f = if islast
> + let ps' = if isreversed then invert ps else ps
> + f = if islast && not isreversed
> then separate_middle_last_from_first
> else separate_first_middle_from_last
I don't fully understand this change, but I'm sure you do
> hunk ./src/Darcs/SelectChanges.lhs 318
> case separate_last_from_first_middle pc of
> - (xs, ys) -> (map tp_patch xs, other_ps ++ map tp_patch ys)
> + xs :> ys -> other_ps +>+ mapFL_FL tp_patch xs :> mapFL_FL tp_patch ys
Order-flip due to to corresponding change in Darcs.Patch.Choices
> hunk ./src/Darcs/SelectChanges.lhs 324
> - (xs, ys) -> length xs `seq` -- Aaack, ugly hack here...
> - (unsafeUnFL $ invert $ unsafeFL (other_ps ++ map tp_patch
> xs),
> - unsafeUnFL $ invert $ unsafeFL $ map tp_patch ys)
> + xs :> ys -> lengthFL xs `seq` -- Aaack, ugly hack here...
> + invert (mapFL_FL tp_patch ys) :> invert (other_ps +>+
> mapFL_FL tp_patch xs)
do we want lengthFL ys here? Does it matter?
======================================================================
Make PatchInfoAndPatch an abstract type.
======================================================================
Clearly an improvement. Part of a general trend of making the code more
explicit by using our own types. Obscure 'fst' becomes informative
'info'. Interesting to those typeclasses come into so much use as well.
One of darcs 2.0 features is more comprehensible and maintainable code
base, as we ramp up our use of Haskell features.
--
Eric Kow http://www.loria.fr/~kow
PGP Key ID: 08AC04F9 Merci de corriger mon français.
pgpxWwdaiifeU.pgp
Description: PGP signature
_______________________________________________ darcs-devel mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-devel
