[WIP] microlens-pro

[crumbtoo]

Draft PR to track progress on microlens-pro.
Original issue: #105
Original WIP PR: #107 (which will serve as reference if not the base of this)

Are there any objections to following the path described in the original thread; that is, re-exporting microlens-platform with the imposter Prisms hidden and replaced with the lens-accurate definitions?

[crumbtoo]

Last commit is a nice starting point for discussion. Included are at least replacements for the faux Prisms and Isos from microlens, as well as a number of other iso/prism-related definitions.

The main points of concern from the original thread included handling of non, _Just, _Left, etc. from Lens.Micro. Currently, they are redefined as proper Isos and Prisms in Lens.Micro.Pro. An accompanying module Lens.Micro.ProCompat re-exports the entirety of Lens.Micro with the exception of _Just and friends, which are sourced from Lens.Micro.

This scheme of wrapping Lens.Micro (or potentially even microlens-platform) was proposed by neongreen in the original issue, and alternate solutions are very welcome.

Another point of note is the obvious lack of documentation. microlens' documentation is leagues more approachable than lens', and thus should be handled with more care; i.e. not cut-and-pasted. a loose to-do list includes Template Haskell, and Haddocks.

[stevenfontanella]

Thanks for opening! Looks like you have good progress already.

Are there any objections to following the path described in the original thread; that is, re-exporting microlens-platform with the imposter Prisms hidden and replaced with the lens-accurate definitions?

No objections, that sounds good to me. It sounds like the idea is that users would either import Lens.Micro / Lens.Micro.Platform or Lens.Micro.Pro depending on their situation. I think that's good.

Re: the rest of the package: I'm less familiar with these lens features. Can you help explain your thought process for what definitions to include and not include? I see that most come from Control.Lens.Combinators but naturally not everything. From #105 I understand that we want prisms and isomorphisms but it looks like there are several related definitions as well. Basically, I'm wondering how we should choose whether to include a definition here or not? That will help me review this.

[crumbtoo]

In honesty, the decision on what definitions to keep was rather arbitrary. I went for at least what was included in microlens and microlens-platform, as well as most of #107.

In general, I'm not quite sure how to rule on a given definition's inclusion besides the vague notion of "microlens only needs the general combinators and a few optics for the types in base"

[stevenfontanella]

That sounds good then! The PR looks good so far, feel free to send it out when you're ready.

[crumbtoo]

I think all that I'm confident doing is done. About updating the README:

• Build times - I don't know what machine these measurements were taken on, nor the methods used to take them.
• There's a significant amount of text that explicitly discusses the absence of prisms and isomorphisms, and I'm not sure how to appropriately go about updating it.

[stevenfontanella]

Sounds good, I’ll take a look tonight.

Build times

I can measure these again for everything, it’s probably best to get more up to date numbers anyway.

updating the README

I can do this separately, no need to wait for that.

[stevenfontanella]

I rebased on master and force-pushed. If you make any additional changes, please just back them up to a separate branch, force-pull (please be careful not to lose any work that wasn't pushed here) and then cherry-pick again before pushing here.

I updated the CI to include microlens-pro, let's try to get it working before merging. I see failures now that I'll take a look at.

[crumbtoo]

overly-strict cabal version bounds might be behind the CI failures. They were generated with cabal gen-bounds under GHC 9.6.4 if I remember correctly

[crumbtoo]

Couldn't get working on ghc-7.10 :(. Main obstacle is GHC.Exts.TYPE, which is seemingly not exported despite docs suggesting it has been since at least base-2.1...

[stevenfontanella]

I left some non-blocking comments. I'll merge and publish the v1 when the CI passes. Thanks a lot for this big contribution!

[stevenfontanella]

I tried using the library a little bit. @crumbtoo, what do you think about changing Lens.Micro.ProCompat into Lens.Micro.Pro? i.e. perform the same exports directly in Lens.Micro.Pro and remove Lens.Micro.ProCompat? My thinking is that Lens.Micro.Pro is currently incompatible with both Lens.Micro and Lens.Micro.Platform, and when using Lens.Micro.Pro, you would likely end up needing definitions defined in Lens.Micro / Lens.Micro.Platform.

Basically I don't see a reason to import Lens.Micro.Pro instead of Lens.Micro.ProCompat. In the former case, you would likely need to import Lens.Micro / Lens.Micro.Platform and add several hiding clauses. So I feel we can just only define Lens.Micro.Pro and let it do the same re-exports. Does that sound ok to you, or is there a reason not to do this?

[crumbtoo]

That's true :p. I don't have any objections.