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

[Eric Kow]

On Tue, Nov 23, 2010 at 22:55:02 +0000, Ganesh Sittampalam wrote:
>   * Remove an unnecessary call to effect in setTentativePending
>   * Get rid of pointless call to effect in Darcs.Resolution
>   * move pending.new reading/writing into Repository.LowLevel
>   * rename Darcs.Hopefully to Darcs.Patch.PatchInfoAnd
>   * get rid of useless instance

I've pushed the part of the bundle that doesn't depend on other stuff
 
> Mon Nov 22 06:32:09 GMT 2010  Ganesh Sittampalam <ganesh at earth.li>
>   * simplify readPatch' for Braced
> 
> Mon Nov 22 06:42:50 GMT 2010  Ganesh Sittampalam <ganesh at earth.li>
>   * split a basic class out of ShowPatch that requires fewer deps
>   This allows a bunch of unreachable and duplicated code to be removed

These are waiting on patch437 and patch426 respectively

Remove an unnecessary call to effect in setTentativePending
-----------------------------------------------------------
> -       setTentativePending r $ effect x
> +       setTentativePending r x

This makes sense because effect just brings patches down to FL Prim.
x comes from readPendingFile which gives use FL Prim already.

Get rid of pointless call to effect in Darcs.Resolution
-------------------------------------------------------
> -              Just (mp' :> _) -> doml (effect p +>+ effect mp') ps
> +              Just (mp' :> _) -> doml (p +>+ mp') ps

Same thing

move pending.new reading/writing into Repository.LowLevel
---------------------------------------------------------
I assume this is just in the interest of layering, pushing
and down hiding them as needed.

> hunk ./src/Darcs/Repository/Internal.hs 323
>      do let newname = pendingName tp ++ ".new"
>         debugMessage $ "Writing new pending:  " ++ newname
>         Sealed sfp <- return $ siftForPending origp
> -       writePendingFile newname sfp
> +       writeNewPending repo sfp
>         cur <- readRecorded repo
> hunk ./src/Darcs/Repository/Internal.hs 325
> -       Sealed p <- readPendingFile newname
> +       Sealed p <- readNewPending repo

This is makeNewPending. Does it belong in LowLevel as well?

Do we care about the "pending.new" name duplication between
LowLevel and Internal? It's unlikely that we'll ever see
these things changing, just curiosity.

Low-level:

> +-- |Read the contents of tentative pending. CWD should be the repository directory.
> +readNewPending :: Repository p C(r u t) -> IO (Sealed (FL Prim C(t)))
> +readNewPending (Repo _ _ _ tp) =
> +    readPendingFile (pendingName tp ++ ".new")

> +-- |Read the contents of new pending. CWD should be the repository directory.
> +writeNewPending :: Repository p C(r u t) -> FL Prim C(t y) -> IO ()
> +writeNewPending (Repo _ _ _ tp) pend =
> +    writePendingFile (pendingName tp ++ ".new") pend

rename Darcs.Hopefully to Darcs.Patch.PatchInfoAnd
--------------------------------------------------
> Ganesh Sittampalam <ganesh at earth.li>**20101122062105
>  Ignore-this: ee6b4444dec3c021be5514b49379ec51
>  The new name much better reflects the role of this module.
>  The Hopefully type could in principle be split out back
>  into a new Darcs.Hopefully module, but it's unlikely it would
>  have any independent value.

I buy it.

> replace ./darcs.cabal [A-Za-z_0-9\-\.] Darcs.Hopefully Darcs.Patch.PatchInfoAnd
> replace ./src/Darcs/Arguments.lhs [A-Za-z_0-9\-\.] Darcs.Hopefully Darcs.Patch.PatchInfoAnd

Note that we tend to ask people to avoid '.' and '-' and replace chars
(I think for conflict avoidance)
  http://wiki.darcs.net/Development/GettingStarted

I think this happened after my attempts at using darcs replace to switch
us to System.FilePath caused a huge conflict mess.  Perhaps good to
revisit that policy and see if it still makes sense

get rid of useless instance
---------------------------
> -instance Nonable Prim where
> -    non = Non NilFL

I guess if it ghc doesn't complain, it's useless

simplify readPatch' for Braced
------------------------------
>  instance ReadPatch p => ReadPatch (Braced p) where
> -    readPatch' =
> -       do mps <- (Just <$> bracketedFL readPatch' '{' '}') <|> return Nothing
> -          case mps of
> -            Just (Sealed ps) -> return $ Sealed $ Braced ps
> -            Nothing -> mapSeal Singleton <$> readPatch'
> +    readPatch' = mapSeal Braced <$> bracketedFL readPatch' '{' '}'
> +                   <|>
> +                 mapSeal Singleton <$> readPatch'

This appears to depend on patch437;

Fri Oct 22 06:49:00 BST 2010  Ganesh Sittampalam <ganesh at earth.li>
  * don't silently throw away remaining parse input
  * drop the unused and often ignored "want eof" parameter to readPatch'
  * drop the nested Maybes in the patch parser

It may be worth mentioning that my workflow now is

1. darcs get --lazy http://darcs.net patched
2. use a tweaked
     http://code.haskell.org/darcs/darcs-team/screened.sed
   to pull -a patches from the bundle into patched
3. darcs get --lazy http://darcs.net unstable
4. cd unstable
5. my usual one-at-a-time darcs apply -i approach to review

split a basic class out of ShowPatch that requires fewer deps
-------------------------------------------------------------
This appears to depend on patch426, particularly

Sun Oct 17 16:41:31 BST 2010  Florent Becker <florent.becker at ens-lyon.org>
  * resolve issue114: allow hunk-splitting in revert

-- 
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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-devel/attachments/20101211/2d1124c1/attachment.pgp>