Visitar URL original
Introduce Distribution.Compat.Lens by phadej · Pull Request #4701 · haskell/cabal · GitHub
Skip to content

Introduce Distribution.Compat.Lens - #4701

Merged
phadej merged 4 commits into
haskell:masterfrom
phadej:lensify
Aug 17, 2017
Merged

phadej merged 4 commits into
haskell:masterfrom
phadej:lensify

Conversation

@phadej

@phadej phadej commented Aug 16, 2017 •

Copy link
Copy Markdown
Collaborator

This patch introduces Distribution.Compat.Lens, a small portion of lens. The need is two fold:

  • to provide lenses to users of Cabal
  • to use it internally in places where lenses shine (subjectively, see Check.hs)

The design is simple:

  • Distribution.Compat.Lens have some definitions from lens.
  • type modules have sibling .Lens module which exports the type and lenses, with the same name as fields. Users can either
import Distribution.Types.PackageDescription
import qualified Distribution.Types.PackageDescription.Lens as L

or

import Distribution.Types.PackageDescription.Lens

Latter variant is not yet viable, as not everything is lensified. But we can make progress there quite easily.

To better compare lens and "old-school" Haskell approaches see e.g. (I realized later, these are essentially same checks):


This commit includes some lens defintions to show up, how the Cabal-lens
framework could look like.

As an example checks which used ad-hoc lenses use newly introduced
lens-framework.

Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

@23Skidoo

Copy link
Copy Markdown
Member

I haven't reviewed the code in detail, but in general I'm +1 on this idea.

@hvr hvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Maybe we can come up with some scripting to reduce the manual lens-derivation maintenance effort for the large product types, but for now this is good enough for me.

Comment thread Cabal/Distribution/Compat/Lens.hs Outdated
-- Common
-------------------------------------------------------------------------------

_2 :: Functor f => (a -> f b) -> (c, a) -> f (c, b)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't say _2 w/o saying _1, IMO ;-)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is btw a good question. Should we introduce FieldN classes for these, like lens and microlens do? (I have _4 used somewhere already)

--
-- First start a repl with proper-lens dependency
--
-- > cabal new-repl Cabal:lib:Cabal ???

@hvr hvr Aug 17, 2017 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the documented but not yet implemented extra-packages: is supposed to provide just that; but until it is implemented, a cabal flag

if flag(deploy-lens-missiles)
   build-depends: lens >= ... && < ...

is a reasonable temporary workaround.

PS: I already have an idea, how we can generate the code w/o dragging in lens nor requiring TH during the generation time. Just needs an afternoon to implement...

This commit includes some lens defintions to show up, how the Cabal-lens
framework could look like.

As an example checks which used ad-hoc lenses use newly introduced
lens-framework.
@phadej

phadej commented Aug 17, 2017

Copy link
Copy Markdown
Collaborator Author

@23Skidoo seems I cannot restart appveyor builds, can you help?

@23Skidoo

Copy link
Copy Markdown
Member

Restarted the build. IIRC @hvr said that he could work around that by logging in to AppVeyor with my user name somehow.

@hvr

hvr commented Aug 17, 2017

Copy link
Copy Markdown
Member

Yeah, for some reason I get offered a dropdown allowing me impersonate @23Skidoo or just be myself...

@23Skidoo

23Skidoo commented Aug 17, 2017 •

Copy link
Copy Markdown
Member

It's just AppVeyor being weird and having no good equivalent for Github organizations. I think that if @phadej was added to the cabal team on GitHub he also would be able to log in as me.

@23Skidoo

Copy link
Copy Markdown
Member

AppVeyor failure seems legit, BTW.

@phadej

phadej commented Aug 17, 2017

Copy link
Copy Markdown
Collaborator Author

and now it isn't legit:

 ghc (exited 1) - ghc not installed. An error occurred during installation:
 The operation has timed out.

@23Skidoo

Copy link
Copy Markdown
Member

Restarted.

@phadej
phadej merged commit 0db132f into haskell:master Aug 17, 2017
@phadej
phadej deleted the lensify branch August 17, 2017 22:31
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

Successfully merging this pull request may close these issues.

3 participants