search

well-typed / optics

Optics as an abstract interface
375 stars 24 forks source link

Port selected functions from Control.Lens.Plated #379

Closed arybczak closed 2 years ago

arybczak commented 3 years ago

It needs a bit of polish wrt. documentation. Do we want to have it in 0.4?

Boarders commented 3 years ago

Any updates on this PR? I'd be happy to help get it over the line.

mikeplus64 commented 3 years ago

I wonder if Plated would be better off in its own package or in recursion-schemes where it IMO fits in very well. It's a really useful typeclass and it would be nice if lens and optics had compatibility with it as well.

class Plated a where
  plate :: Applicative f => (a -> f b) -> s -> f t

I don't know if that's impossible with optics' Traversal.

arybczak commented 2 years ago

I've updated the PR and stripped the API down. Could you tell me if that's ok? I've never used Plated so can't really tell if the API is now much harder to use.

I'm even wondering about removing Plate and all functions without the Of suffix that just apply the Of variant to plate. But maybe that's too much?

arybczak commented 2 years ago

I'm even wondering about removing Plate and all functions without the Of suffix

The last commit performs that experiment. It looks the most reasonable to me :thinking: Thoughts?

arybczak commented 2 years ago

@mikeplus64 this is a good idea. optics can reuse Plated in the form you've written by simply wrapping it in traversalVL.

The only problem is the default instance would be missing, which might or might not be a big deal.

phadej commented 2 years ago

In my opinion Plated type-class shouldn't be in lens and shouldn't be copied to optics either. It's just a traversal for immediate children. (It would be silly to have Traversable in lens, and we pulled TraversableWithIndex out of it too).

EDIT: I'm :+1: with adding non Plated specific combinators. I'm :-1: for adding the class.

arybczak commented 2 years ago

@phadej I'm with you on this one. I don't see the point of having Plated and *Of variants that are independent of it.

I think this PR can be merged as-is.