[darcs-devel] module Darcs.Patch

Ben Franksen ben.franksen at online.de
Wed Aug 28 11:57:29 UTC 2019


Am 28.08.19 um 12:07 schrieb Ganesh Sittampalam:
> On 28/08/2019 10:49, Ben Franksen wrote:
> 
>> (a) Either we delete it and always import everything explicitly.
>>
>> (b) Or we extend it to cover everything from under Darcs.Patch that we
>> consider part of the external API and from the outside import only from
>> that module.
> 
> My initial thought is that we should do (b), though let's not try to do
> it as a big bang in a single patch.
> 
>> The problem with (b) is that in the importing modules we loose the
>> information about where the items we import are defined.
> 
> Well, we lose the direct information. We should be able to find the
> information just by looking in Darcs.Patch.
> [...]
>> If we decide on (b) we should re-write Darcs.Patch to replace explicit
>> exports of single items with module exports. If we want to exclude an
>> item because it is only meant for internal use inside of Darcs.Patch.*
>> we can do that by restricting the import.
> 
> Agreed, though let's always restrict the import so we don't accidentally
> add things to the Darcs.Patch API. If we're going to the trouble of
> having one we should have to think at least briefly before we expand it.

Hm, yes. This would also resolve at least some of the tension as we
could always see then where the item is defined.

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

>> I would also export all the
>> classes with all associated methods and types instead of only the
>> methods (we need the class names often enough that this makes no sense).
> 
> i.e. Commute(..) rather than commute? Yes, I think in practice this will
> be necessary as we mention them in constraints so often.

Yes, that was my thinking.

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

>> If we decide on (a) we should perhaps be a bit more liberal with
>> re-exports. For instance, Darcs.Patch.RepoPatch could re-export all its
>> superclasses including their methods, so we could at least import things
>> like Commute(..), Invert(..), etc from a single module.
> 
> 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?



More information about the darcs-devel mailing list