haskell / network-uri

URI manipulation facilities
Other
24 stars 33 forks source link

Add lenses for network uri #42

Closed jappeace closed 5 years ago

jappeace commented 5 years ago

This makes working with network uri simpler.

Once this is merged I can deprecate: http://hackage.haskell.org/package/network-uri-lenses

ezrakilty commented 5 years ago

This is looking good, but I have two comments.

First, I am not very familiar with the use of lenses. Can you show me some example code that uses this library? To the extent I understand them, it seems they'd be most useful for getting into the uriAuthority member and its fields, since that is the nested record. But when I try to use this lens, I can't use it directly, as u ^. uriAuthLens ^. regNameLens, because the authority component is a Maybe and we need to do something extra to get inside the Maybe wrapper. Is there a straightforward way to do that in the lens package? Or should we make changes in the present package to make that easier to do?

Second, I think we should make the lens names be exactly parallel to the field names. So that would mean:

regNameLens becomes uriRegNameLens userInfoLens becomes uriUserInfoLens portLens becomes uriPortLens uriAuthLens becomes uriAuthorityLens

Thoughts?

jappeace commented 5 years ago

Can you show me some example code that uses this library?

Sure okay so the idea is that we make 'getting' and 'setting' of records easy by function composition.

So your example of u ^. uriAuthLens ^. regNameLens may work as u ^. uriAuthLens . _Just . regNameLens. Or as u ^? uriAuthLens . _Just . regNameLens, the first one will only work for monoids (choosing mempty on Nothing) where the second one will just wrap it within a maybe, in other words, if you get a type error with something something monoid, try using ^?, because it's more lax.

Lenses also composes with structures in client code, say we have a record like:

data ClientSettings {
   _hostUri :: URI
  ...
}
makeLenses 'ClientSettings -- this template stuff is available in the lens library, on which you probably don't want to depend as a library.

Now the client can do:

main = do
    settings <- readSettings
    print $ settings ^. hostUri . uriAuthority . _Just . regNameLens
    let replaceRegname = over (hostUri . uriAuthority . _Just) (set regNameLens "jappieklooster.nl") settings
   ...

This is why I want to push them upstream, my existing lenses become more powerfull because they can compose with the upstream ones.

Second, I think we should make the lens names be exactly parallel to the field names.

Yes I agree that consistent names are better, I don't know why I changed them.

ezrakilty commented 5 years ago

Great! The examples are very helpful, thank you. Glad to see there is a way of interacting with Maybes.

Just to make sure I haven't missed anything, in your second example, with main, the occurrences of uriAuthority would actually be uriAuthorityLens, right?

You're right that I'd rather not take a dependency (on the actual lens library) at this point.

jappeace commented 5 years ago

Yes, uriAuthority should be auriAuthorityLens, my bad