[darcs-devel] [patch479] Tighten up type in Darcs.Rollback (and 14 more)

Eric Kow bugs at darcs.net
Fri Dec 10 19:08:19 UTC 2010


Eric Kow <kowey at darcs.net> added the comment:

I'm trying to chip away at the backlog, unfortunately only tackling the
most obvious-looking stuff for now.

On Tue, Nov 23, 2010 at 22:55:02 +0000, Ganesh Sittampalam wrote:
>   * Tighten up type in Darcs.Rollback
>   * Tighten up type of applyToWorking
>   * tighten up type of handlePendForAdd
>   * generalise type of xmlSummary
>   * Remove an unnecessary call to effect in setTentativePending
>   * Get rid of pointless call to effect in Darcs.Resolution
>   * Darcs.Patch.Apply doesn't have orphans any more
>   * improve name
>   * get rid of some spurious arguments to impossible
>   * move writePatch and gzWritePatch outside ShowPatch

I've applied these as they seem fairly straightforward

>   * simplify readPatch' for Braced
>   * move pending.new reading/writing into Repository.LowLevel
>   * rename Darcs.Hopefully to Darcs.Patch.PatchInfoAnd
>   * get rid of useless instance
>   * split a basic class out of ShowPatch that requires fewer deps

I'll hopefully work on this Saturday afternoon or if I'm too tired
Wednesday


Tighten up type in Darcs.Rollback
---------------------------------
> -rollItBackNow :: (RepoPatch p1, RepoPatch p) =>
> -                [DarcsFlag] -> Repository p1 C(r u t) ->  FL (PatchInfoAnd p) C(x y)
> +rollItBackNow :: (RepoPatch p) =>
> +                [DarcsFlag] -> Repository p C(r u t) ->  FL (PatchInfoAnd p) C(x y)

Is this just a matter of principle, not making types more general than
we have evidence that they need to be?

Or are we actively asserting that they have the same type here?

Tighten up type of applyToWorking
---------------------------------
> Ganesh Sittampalam <ganesh at earth.li>**20101121153126
> -applyToWorking :: Patchy p => Repository p1 C(r u t) -> [DarcsFlag] -> p C(u y) -> IO (Repository p1 C(r y t))
> +applyToWorking :: Repository p C(r u t) -> [DarcsFlag] -> FL Prim C(u y) -> IO (Repository p C(r y t))

tighten up type of handlePendForAdd
-----------------------------------
> -handlePendForAdd :: forall p q C(r u t x y). (RepoPatch p, Effect q)
> -                    => Repository p C(r u t) -> q C(x y) -> IO ()
> +handlePendForAdd :: forall p C(r u t x y). (RepoPatch p)
> +                    => Repository p C(r u t) -> PatchInfoAnd p C(x y) -> IO ()

Similar questions for the patches above?

generalise type of xmlSummary
-----------------------------
> -import Darcs.Patch.Named ( Named(..), patchcontents )
> +import Darcs.Patch.Named ( Named(..) )

> -xmlSummary :: (Effect p, Conflict p) => Named p C(x y) -> Doc
> +xmlSummary :: (Effect p, Conflict p) => p C(x y) -> Doc
>  xmlSummary p = text "<summary>"
> hunk ./src/Darcs/Patch/Viewing.hs 131
> -             $$ (vcat . map summChunkToXML . genSummary . conflictedEffect . patchcontents $ p)
> +             $$ (vcat . map summChunkToXML . genSummary . conflictedEffect $ p)

I'm trusting the patch summary that conflictedEffect on Named
grabs the patchcontent

Remove an unnecessary call to effect in setTentativePending
-----------------------------------------------------------
> ] hunk ./src/Darcs/Repository/Internal.hs 637
> -       setTentativePending r $ effect x
> +       setTentativePending r x

Seems like this needed a bit more studying to see that it's
unnecessary so I postponed.

Get rid of pointless call to effect in Darcs.Resolution
-------------------------------------------------------
> Ganesh Sittampalam <ganesh at earth.li>**20101122061049
> hunk ./src/Darcs/Resolution.lhs 70
>                case commute (invert p :> mp) of
> -              Just (mp' :> _) -> doml (effect p +>+ effect mp') ps
> +              Just (mp' :> _) -> doml (p +>+ mp') ps

Likewise, though I'm guessing that's because Prims are their own
effects.

Darcs.Patch.Apply doesn't have orphans any more
-----------------------------------------------
> Ganesh Sittampalam <ganesh at earth.li>**20101122061806
>  Ignore-this: 1f16110aa0724103970b586885e4bba6
> ] hunk ./src/Darcs/Patch/Apply.lhs 20
>  
>  
>  \begin{code}
> -{-# OPTIONS_GHC -fno-warn-orphans #-}


improve name
------------
> -applyToPop'
> +applyToPopPrim

Yes

---------------------------------------------------------
move pending.new reading/writing into Repository.LowLevel
rename Darcs.Hopefully to Darcs.Patch.PatchInfoAnd
get rid of useless instance
simplify readPatch' for Braced
split a basic class out of ShowPatch that requires fewer deps
---------------------------------------------------------
postponed


get rid of some spurious arguments to impossible
------------------------------------------------
> -fromNons :: [Non RealPatch C(x)] -> Maybe (Sealed (FL Prim C(x)))

> -pullInContext :: [Non RealPatch C(x)] -> Maybe (Sealed (Prim C(x)), [Non RealPatch C(x)])

> -            Nothing -> impossible pullInContext fromNons
> +            Nothing -> impossible

Heh, I wonder if this was some kind of attempt to semi-comment-out code.

move writePatch and gzWritePatch outside ShowPatch
--------------------------------------------------
> hunk ./src/Darcs/Patch/Show.lhs 71
> -    writePatch :: FilePath -> p C(x y) -> IO ()
> -    writePatch f p = writeDocBinFile f $ showPatch p <> text "\n"
> -    gzWritePatch :: FilePath -> p C(x y) -> IO ()
> -    gzWritePatch f p = gzWriteDocFile f $ showPatch p <> text "\n"

> +writePatch :: ShowPatch p => FilePath -> p C(x y) -> IO ()
> +writePatch f p = writeDocBinFile f $ showPatch p <> text "\n"
> +gzWritePatch :: ShowPatch p => FilePath -> p C(x y) -> IO ()
> +gzWritePatch f p = gzWriteDocFile f $ showPatch p <> text "\n"

I can buy that we never want to override this.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, try +44 (0)1273 64 2905 or
xmpp:kowey at jabber.fr (Jabber or Google Talk only)

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch479>
__________________________________


More information about the darcs-devel mailing list