[darcs-devel] [patch2131] make writeUnrevert take its arguments in... (and 8 more)
Ganesh Sittampalam
bugs at darcs.net
Tue Dec 22 00:46:13 UTC 2020
Ganesh Sittampalam <ganesh at earth.li> added the comment:
> * make writeUnrevert take its arguments in a more natural order
OK
> * unrevert: rename some local variables
OK
> * writeUnrevert: remove the 'pending' argument
>
> This is a preparation for moving writeUnrevert to the Repository layer. The
> code we move from writeUnrevert into the command implementation is asking
> the user, so belongs to the UI layer.
OK, though I'm not sure if I like seeing yet more patch manipulation
code in the Commands layer. I don't have a good alternative though.
Perhaps we need data structures to abstract interactivity.
> * move writeUnrevert and unrevertPatchBundle to Darcs.Repository.Unrevert
I like this direction much better :-)
> * cleanup imports in Darcs.UI.Commands.Unrevert
OK
> patch 39ef756740bf8770f8bc119bb8314e935e7e1500
> Author: Ben Franksen <ben.franksen at online.de>
> Date: Fri Oct 23 16:34:26 CEST 2020
> * move call to readRepo out of writeUnrevert
>
> This prepares for moving removeFromUnrevertContext from D.R.Hashed to
> D.R.Unrevert. It also makes it utterly clear that unrevert is unsafe: we
> read the repo and then modify the unrevert bundle /after/ it the changes
> have been finalized.
> hunk ./src/Darcs/Repository/Unrevert.hs 27
> -writeUnrevert repository pristine ps = do
> - rep <- readRepo repository
> +writeUnrevert recorded pristine ps = do
> hunk ./src/Darcs/UI/Commands/Revert.hs 176
> - deps :> torevert' :> _ ->
> - writeUnrevert repository recorded (deps +>>+ torevert')
> + deps :> torevert' :> _ -> do
> + ps <- readRepo repository
> + writeUnrevert ps recorded (deps +>>+ torevert')
The parameter naming is confusing here. In the old code, something
called 'recorded' was being passed to a parameter named 'pristine'.
In the new code, that still happens but we also pass 'ps' to a parameter
named 'recorded'. Could you clean that up a bit, as you already did
further down? [I haven't read ahead yet, so perhaps you do so in a later
bundle]
> * unrevert: use promptYorn instead of askUser
OK
> * simplify removeFromUnrevertContext
OK (I suspect this came from a time when GADT type inference/matching
didn't work so well)
> * move removeFromUnrevertContext to D.R.Unrevert
OK
__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch2131>
__________________________________
More information about the darcs-devel
mailing list