Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record.Lens is not "completely compatible with lens" as claimed #20

Open
aavogt opened this issue Mar 26, 2015 · 5 comments
Open

Record.Lens is not "completely compatible with lens" as claimed #20

aavogt opened this issue Mar 26, 2015 · 5 comments

Comments

@aavogt
Copy link
Contributor

aavogt commented Mar 26, 2015

Record.Lens uses RankNTypes when defining view/set etc. unlike Control.Lens, which means more type signatures are needed

{-# LANGUAGE QuasiQuotes #-}
import Record.Lens
import Record
import qualified Control.Lens

-- figuring out this type signature is too hard (and it's not as general as the inferred type of f)
-- g :: Num a => Lens [r| { x :: a, y :: a } |]  s' a a' -> a
-- g l = view l [r| { x = 1, y = 2 } |]

{- |

>>> f [l| x |]
1

>>> f [l| y |]
2

-}
f l = Control.Lens.view l [r| { x = 1, y = 2 } |]
@nikita-volkov
Copy link
Owner

I don't quite understand what the problem is here and what you propose.

@aavogt
Copy link
Contributor Author

aavogt commented Mar 26, 2015

  1. you could change the comment in that module to
-- A minimal subset of the "lens" library functionality,
-- which is completely compatible* with it.
--
-- * except that unlike Control.Lens.set/over etc., Record.Lens functions use RankNTypes, so that functions taking lenses as parameters also need to use RankNTypes, which makes type signatures mandatory.
  1. you could not use RankNTypes in that module. All they do there is make the types look easier.

@fumieval
Copy link

fumieval commented Apr 2, 2015

Probably he's complaining that the argument of view or over requires too much.

For example, currently the type of over is Lens s s' a a' -> (a -> a') -> s -> s'. It can't be applied to something weaker, like Traversal or just Setter; it turns out incompatible with lens. Instead, it should be over :: ((a -> Identity b) -> s -> Identity t) -> (a -> b) -> s -> t. It accepts any lens-like bros that subsume Setter; in lens, all the setter-related combinators are defined using type ASetter s t a b = ((a -> Identity b) -> s -> Identity t). Similarly, view should be ((a -> Const a a) -> s -> Const s s) -> s -> a.

In terms of compatibility, it'd be better to generalize these combinators.

@aavogt
Copy link
Contributor Author

aavogt commented Apr 2, 2015

@fumieval my complaint would be solved if ghc could infer a rank-2-type. Your examples would not be solved if ghc could infer a rank-2-type.

But they are both arguments for changing the types in Record.Lens.

@nikita-volkov
Copy link
Owner

Okay I finally got @aavogt.

I've been thinking about removing the "Record.Lens" module from the project altogether in future releases anyway, giving preference to the "basic-lens" dependency. Although that library provides exactly the same API as the one in the "Record.Lens" module, its documentation is more involved and does not contain the issues, which @aavogt mentioned.

Concerning the "lens" compatibility, it was never intended to make the lens-combinators of the "record" library exactly compatible with the "lens" library, they were only meant to be interchangeable as per the purposes of the "record" library instead. The most important thing here is that the lenses produced with the [l|..|] quasiquoter are completely compatible with "lens".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants