[darcs-devel] module Darcs.Patch

Ganesh Sittampalam ganesh at earth.li
Wed Aug 28 10:07:16 UTC 2019


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.

> 
> It looks to me as if we can't have the cake and eat it. So this comes
> down to a trade-off.
> 
> 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.

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.

> 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. Perhaps ideally
our patch-using API would not mention the individual classes, but we're
a long way from doing that.

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

Ganesh


More information about the darcs-devel mailing list