[darcs-devel] module Darcs.Patch

[Ganesh Sittampalam]

On 28/08/2019 12:57, Ben Franksen wrote:
> Am 28.08.19 um 12:07 schrieb Ganesh Sittampalam:

>> I'm not sure what to do about Darcs.Patch.Witnesses.* - they are somehow
>> more fundamental than the patch API itself though clearly they are
>> central to it. They used to be in the Darcs.Witnesses namespace and
>> maybe we could move them back there rather than just re-exporting them
>> all through Darcs.Patch.
> 
> That sounds good, except for Witnesses.Eq which I think should go into
> the Darcs.Patch namespace directly (i.e. rename it to Darcs.Patch.Eq).

Makes sense, given the FL instance. (Which still makes me nervous!)

>> Perhaps ideally
>> our patch-using API would not mention the individual classes, but we're
>> a long way from doing that.
> 
> I think attempting to hide the classes is misguided. We should embrace
> the fact that we have all these different interfaces and I think it is a
> good idea to add only the minimal set of constraints to type
> signatures... within reason, of course. Using synonyms like RepoPatch is
> fine if the number of constraints becomes large; but if we know we need
> only, say, Commute and Invert, why not say so?

I go back on forth on this. But I think you're probably right,
particularly if we anyway have a patch-defining API that anyone could
implement if they follow the laws.

>> Related to this, I think we should have an "internal" API for *defining*
>> patch types. Every time we define a new patch type we have to import
>> something like 10 different modules. If we are just going to export all
>> the necessary classes from Darcs.Patch anyway then maybe this API could
>> just be Darcs.Patch, but it seems to me that the two are logically
>> slightly different things.
> 
> I am not sure I get this. Can you give an example?

Take for example this chunk of imports in Darcs.Patch.Prim.WithName (or
Darcs.Patch.Prim.Named in reviewed):

import Darcs.Patch.Annotate ( Annotate(..) )
import Darcs.Patch.Apply ( Apply(..) )
import Darcs.Patch.Commute ( Commute(..) )
import Darcs.Patch.CommuteNoConflicts ( CommuteNoConflicts(..) )
import Darcs.Patch.Format ( PatchListFormat(..) )
import Darcs.Patch.Ident ( PatchId, Ident(..), SignedId(..),
StorableId(..), IdEq2 )
import Darcs.Patch.Inspect ( PatchInspect(..) )
import Darcs.Patch.FileHunk ( IsHunk(..) )
import Darcs.Patch.FromPrim ( PrimPatchBase(..) )
import Darcs.Patch.Prim.Class ( PrimPatch, PrimApply(..),
PrimClassify(..), PrimDetails(..) )
import Darcs.Patch.Invert ( Invert(..) )
import Darcs.Patch.Read ( ReadPatch(..) )
import Darcs.Patch.Repair ( RepairToFL(..) )
import Darcs.Patch.Show
    ( ShowPatchBasic(..)
    , ShowPatch(..)
    , ShowContextPatch(..)

I think we should have one module that exports all of them, and it
shouldn't necessarily be just Darcs.Patch.

Ganesh