[darcs-devel] [patch2131] make writeUnrevert take its arguments in... (and 8 more)

[Ganesh Sittampalam]

Ganesh Sittampalam <ganesh at earth.li> added the comment:

>> OK, though I'm not sure if I like seeing yet more patch manipulation
>> code in the Commands layer.
> 
> It is debatable whether this is the case here, but apart from that I am
> not sure I agree. IMO patch manipulation and then passing patches (FL,
> RL, Fork, PatchSet, ...) is clearly superior compared to the
> alternative, which is passing the Repository and then (re-)reading
> patches from disk.

Yes, true.

>> I don't have a good alternative though.
>> Perhaps we need data structures to abstract interactivity.
> 
> Yes, I had a similar idea. Something like passing down a 'decision
> procedure' i.e. something of type IO Bool. One instance where we could
> try that out is with  the interactive parts of tentativelyMergePatches.

Passing down IO Bool wouldn't work when the interaction required is in a
different monad. I was vaguely wondering about some kind of structure like

  data RevertResult = OK (IO ()) | CantUnrevert (IO ())

but that doesn't seem great either.

> Related: we may also want to rename readRecorded to readPristine, which
> is less ambiguous ("recorded" can mean the recorded patches or the
> recorded state). There are many places where we say something like
> 
>   pristine <- readRecorded repository
> 
> Furthermore, readRepo should be named readRecorded or perhaps
> readRecordedPatches.

Sounds good.

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