[darcs-devel] [patch1701] refactorings in the Darcs.Patch subsystem

[Ganesh Sittampalam]

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

[should prim patches implement CommuteNoConflicts]

> Now what about prim patches? Well, their commute really satisfies
> the square commute law, so, from a logical point of view they
> should have an instance of CommuteNoConflicts to express that.
> Indeed a sequential pair of prim patches cannot be in conflict,
> so commuteNoConflicts necessarily coincides with commute.

> The reason we need the more general Commute class at all is *only*
> because of Merge, which requires that both branches of a merge 
> commute to each other. We (have to) weaken the requirements for
> Commute (realtive to CommuteNoConflicts) precisely so that we can 
> state the merge commute law.

> In this sense, the difference between CommuteNoConflicts
> and Commute precisely captures, in an abstract way, the
> difference in behavior between commute for prim patches
> and commute for generalized patches.

Right. I think you've uncovered an important part of the "theory" of
darcs here (at least, I wasn't previously aware of it in these 
terms).

I'm less sure about encoding that in the code in its present form.

Our code does use Commute extensively - even Eq2 for lists relies
on it - whereas commuteNoConflicts is in practice only used in the
internals of specific merge implementations, and for marking 
conflicts. You mentioned mergeNoConflicts in the patch comments but 
I don't see any actual uses of it outside the merge implementations.

I worry about making the code harder to work on by introducing
ambiguity between the two different commutes. Perhaps we should
actually be using commuteNoConflicts more widely, but I don't have
an intuition about where.

I won't push further to drop this, but if you do keep it perhaps
you could add a doc comment to the commuteNoConflicts member itself,
explaining when it should be used? That would complement the
existing nice docs on the class itself that explain how to implement
it.

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