For discussion initially.

In principle it seems like a 'toPrim' method might be
a worthwhile part of the API of patches in general, and
I have a feeling I introduced ToFromPrim initially when
abstracting the Prim API in the first place.

But in practice it doesn't seem to be used outside V2,
and it's not obvious that other patch types should
implement it for no actual reason.

1 patch for repository darcs-unstable@darcs.net:screened:

patch f5dab200ee7599dfcb8c41c41d8e129ebf2ff559
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Aug 18 19:31:20 BST 2019
  * move class ToFromPrim to RepoPatchV2 code
  
  It's only used there and isn't currently a concept that's
  generally useful in the Darcs code as a whole.

Attachments

• move-class-tofromprim-to-repopatchv2-code.dpatch
• patch-preview.txt
• unnamed

> In principle it seems like a 'toPrim' method might be
> a worthwhile part of the API of patches in general, and
> I have a feeling I introduced ToFromPrim initially when
> abstracting the Prim API in the first place.
> 
> But in practice it doesn't seem to be used outside V2,
> and it's not obvious that other patch types should
> implement it for no actual reason.
> 
> 1 patch for repository darcs-unstable@darcs.net:screened:
> 
> patch f5dab200ee7599dfcb8c41c41d8e129ebf2ff559
> Author: Ganesh Sittampalam <ganesh@earth.li>
> Date:   Sun Aug 18 19:31:20 BST 2019
>   * move class ToFromPrim to RepoPatchV2 code
>   
>   It's only used there and isn't currently a concept that's
>   generally useful in the Darcs code as a whole.

If it weren't for V2-Duplicates we could use toPrim to detect whether a
patch is conflicted or not. This could then be used to refactor
combineConflicts to have a simpler and more intuitive type. But this is
hypothetical and so I came to a similar conclusion some time ago.

However, a class with only one instance makes no sense. A plain function

  toPrim :: RepoPatchV2 prim wX wY -> prim wX wY

is what I'd go for.

On 27/08/2019 17:14, Ben Franksen wrote:

> However, a class with only one instance makes no sense. A plain function
> 
>   toPrim :: RepoPatchV2 prim wX wY -> prim wX wY
> 
> is what I'd go for.

The class/instance combination is needed to break the recursive knot.
The same is true of Nonable. The alternative would be hs-boot files.

>> However, a class with only one instance makes no sense. A plain function
>>
>>   toPrim :: RepoPatchV2 prim wX wY -> prim wX wY
>>
>> is what I'd go for.
> 
> The class/instance combination is needed to break the recursive knot.
> The same is true of Nonable. The alternative would be hs-boot files.

I see. But then I wouldn't move the class to RepoPatchV2, just delete
the instances we don't need and remove ToPrim from the superclasses of
RepoPatch. It is cleaner to have a class that could, in principle, be
defined for many patch types inside the general Darcs.Patch section.
(While Nonable is clearly an implementation detail of RepoPatchV2.) Also
means less churn if later we decide we need toPrim for something after all.

I have a use for toPrim for the upcoming 'convert darcs-3' command.