[darcs-devel] unsuspend-reify [was: [patch1824] v3: basic integration]

[Ben Franksen]

[re-sending, this time with complete text]

>  - improve/review the rebase unsuspend hack

I think you are referring to the one in
Darcs.Patch.Rebase.Viewing.forceCommutePrim (which is indeed only used
for rebase suspend, via extractRebaseSelect).

I have been thinking about that one. It is interesting to note that
rebase reify does not use forceCommute and thus isn't affected. Indeed,
with rebase reify the fixups remain where they are i.e. before the patch
we unsuspend.

Before I come to my point, let me digress a bit. Rebase unsuspend has
IME one grave practical disadvantage: other than with normal conflicts,
when you get conflicts on unsuspend these aren't stored anywhere and are
irretrievably lost if you revert the conflict markup (unrevert aside).
This is one of the reasons I never unsuspend more than one patch at a
time and are very careful to resolve every conflict immediately, then
amend the patch, but only when I am sure I've got it right, ideally
keeping the marked version in my editor's undo-history during that time.

Ideally I would want the unsuspended patch to be conflicted, so I can
recover the markup. Unfortunately this is impossible, which brings me
back to the issue at hand. because the reason for this is the same as
the one for the hack in forceCommutePrim: We need a proper Named patch
in our history with which we can conflict.

I think we can kill these two birds with one stone.

Let me introduce a new command which is a mixture of the current reify
and unsuspend commands. It works like reify in that we create an
artificial Named patch for the (residual) fixups. And it works like
unsuspend in that we do a forceCommute of them. But we create the patch
from the fixups /first/, and do the forceCommute afterwards with this
Named patch.

How does this forceCommute work, exactly? We have residual fixups f
wrapped up in a Named, and a patch p that depends on f, that is we have
f:>p which don't commute. We want to commute them forcibly. So we merge
(f^:\/:p) = p':/\:f^'. This is allowed, since the patch f we invert is
unconflicted and thus f^ still consists of prims. But re-inverting
(f^')^ and returning p':>(f^')^ would be highly illegal: p' contains
conflictors that refer to inverted prims which aren't in its context;
and (f^')^ even contains rotcilfnocs, inverse conflictors, and these are
pariahs that aren't allowed to live in our repo (except behind firmly
closed doors when we do short-lived deals with them ;-).

But we are rebasing, so we are free to amend patches. All we need is p'
and we amend it by replacing its content with its effect, so it becomes
unconflicted and doesn't refer to anything anymore; let's calls that q.
The freak (f^')^ is thrown away. Instead we do a second merge (q:\/:p) =
p'':/\:_ and the final result is q:>p'', where p'' is now conflicted,
but otherwise an honest, upstanding citizen we can introduce to our
neighbors without shame.

I hope I got that right so far.

The point of all this is that when we unsuspend-reify in this way we get
first the patch q (i.e. the rebased version of our old, formerly
suspended p) and then an artificial but otherwise valid conflicted patch
p''. So the conflict markup can be restored at any time using
mark-conflicts. When we are done with manual resolution we can unrecord
it and then amend everything into p. But we don't necessarily have to;
we might opt to keep it in our repo as is, or only amend it, and not the
q. It makes sense then to improve the PatchInfo we generate for p'', so
it has the correct author (whoever rebases the patch) and date of
creation; we could even let the user edit the description.

The rebase unsuspend hack is not needed for this new command. And the
old behavior of unsuspend can be recovered (for compatibility) by doing
the equivalent of a final 'unrecord --last=1'. I think, though, that the
new command is strictly better, so I would opt for changing the
semantics of unsuspend and adding an option --unrecord-fixups that
restores the old behavior.

Cheers
Ben