Emphasize similarities between traversalVL and foldVL

[arybczak]

Somewhat related to #426.

[phadej]

I don't like this. We have unfolded FoldVL definition for some reason (and it's not correct). Yet we very much could, as Contravariant is in base since 4.12 (we'd need to drop ghc-8.2 support, that's imo fine). We also use synonyms in optics-vl and in Optics.Lens.

[arybczak]

What do you mean by "it's not correct"?

[arybczak]

Ok, I think you mean that it's not how Fold from lens looks like.

This is fine. Contravariant in Fold from lens is a quirk emerging from their implementation. We don't need Contravariant because we have Bicontravariant internally to do the same thing it does, just behind the scenes.

You mentioned in #426 that foldVL traverse works, but that's just because foldVL type is too permissive. I actually changed it here so that foldVL traverse no longer works, but foldVL traverse_ works just fine.

[phadej]

Not fine. I cannot convert lens Fold into optics Fold using current, or new foldVL.

import qualified Control.Lens as L
import qualified Optics as O

ex1 :: Foldable f => L.Fold (f a) a
ex1 = L.folding id

ex2 :: Foldable f => O.Fold (f a) a
ex2 = O.foldVL ex1

{-

fold.hs:8:16: error:
    • Could not deduce (L.Contravariant f1) arising from a use of ‘ex1’
      from the context: Foldable f
        bound by the type signature for:
                   ex2 :: forall (f :: * -> *) a. Foldable f => O.Fold (f a) a
        at fold.hs:7:1-35
      or from: Applicative f1
        bound by a type expected by the context:
                   forall (f1 :: * -> *).
                   Applicative f1 =>
                   (a -> f1 a) -> f a -> f1 (f a)
        at fold.hs:8:7-18
      Possible fix:
        add (L.Contravariant f1) to the context of
          a type expected by the context:
            forall (f1 :: * -> *).
            Applicative f1 =>
            (a -> f1 a) -> f a -> f1 (f a)
    • In the first argument of ‘O.foldVL’, namely ‘ex1’
      In the expression: O.foldVL ex1
      In an equation for ‘ex2’: ex2 = O.foldVL ex1
  |
8 | ex2 = O.foldVL ex1

-}

We can have the foldVL-like introduction form, but it cannot be named foldVL, that is misleading.

[arybczak]

I mentioned at least once (#179 and #183) before the initial release the discrepancy you're pointing and we agreed it was fine.

It seems it's just a case of insufficient documentation.

In any case foldVL can't "easily" take Contravariant because wander doesn't.

It seems we just need to communicate the fact that foldVL is not compatible with lens. I had a patch lying around, so if you really want this, I rebased and pushed it: #430.

[phadej]

There's not much discussion in #183, and in light of new knowledge I think it wasn't that good of the move. mkFold is not great name either, but foldVL is simply wrong and with #430 in it would be misleading and confusing.

[arybczak]

I'd like to resolve this situation, otherwise these two PRs (this and #430) will keep hanging.

Let's just rename foldVL in Optics.Fold back to mkFold. Yes, it's not a great name, but it's better to rename it than be in the current situation.

Unless you have propositions for a better name 🙂

[arybczak]

I'd like to merge this. Any objections?