[darcs-devel] module Darcs.Patch

[Benjamin Franksen]

>> 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!)

What exactly makes you nervous about it? I would like to understand.

>>> 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.

Ah, I see.

Indeed, it definitely can't be Darcs.Patch. Nothing named Darcs.Patch.X
should import Darcs.Patch! (We do it in a few places but this can easily
be cleaned up.) Besides, we'd immediately get circular dependencies.

I think Darcs.Patch.RepoPatch should re-export all its superclasses
(with methods). That would make it the perfect candidate for all three
RepoPatch types. For prim patches we already have Darcs.Patch.Prim which
does exactly that.

I also think that both RepoPatch and PrimPatch should become constraint
synonyms instead of classes with lots of superclasses. This avoids
syntax overhead and means less potential for cyclic module dependencies.
(And a few less orphans, too.)

BTW, these are all action items that are largely independent of whether
we do something about Darcs.Patch or not. In fact I think we should do
them before we start to extend Darcs.Patch (assuming we go for (b)).

Cheers
Ben