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

[feat] allow passing list literals to mod builders #488

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MangoIV
Copy link

@MangoIV MangoIV commented May 24, 2024

I have found myself writing things like strOption $ mconcat [... quite often and upon discussion, @Qqwy suggested just using list literals, this PR implements this idea, it works like a charm. the IsList instance even respects its laws!

It allows writings things like

... prefs [shoHelpOnEmpty, showHelpOnError] 
... strOption [long "bla", short 'b']

etc

@HuwCampbell
Copy link
Collaborator

That's an interesting approach.

I try to keep optparse-applicative very conservative with regards to its extensions (we're just Haskell 98 + Existential Types at the moment), so I don't think it's very likely this will be merged as is.

But I'll keep this open now so it can be discussed further.

@MangoIV
Copy link
Author

MangoIV commented May 27, 2024

well the extension is required to write the instance, it’s not that I’m making excessive use of TypeFamilies, unfortunately there’s no other good place for this to go, orphan instances are sad :(

Alright, let’s see :)

@MangoIV
Copy link
Author

MangoIV commented May 28, 2024

@HuwCampbell would you be happy to exclude the change from older GHC by the use of CPP or would that be too much of a complexity increase?

@MangoIV
Copy link
Author

MangoIV commented Jun 19, 2024

@HuwCampbell what kind of improvements could I do to this PR such that it can be merged?

@HuwCampbell
Copy link
Collaborator

HuwCampbell commented Jul 9, 2024

Look, I'm not sure right now. While it's a minor inconvenience to add mconcat, and I do think that it's probably a better API to accept a list.

But unfortunately I don't think I can't make breaking changes like that in the next release.

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.

2 participants