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

Lifting Pretty to * -> * & * -> * -> * #39

Closed
robrix opened this issue Aug 22, 2017 · 5 comments
Closed

Lifting Pretty to * -> * & * -> * -> * #39

robrix opened this issue Aug 22, 2017 · 5 comments

Comments

@robrix
Copy link

robrix commented Aug 22, 2017

Would you be open to a PR defining liftings of Pretty to * -> * and * -> * -> *, a la Show1 &c. from Data.Functor.Classes? It makes (decidable) recursive instances for Fix, Free, Cofree &c. much easier to define:

instance Pretty1 f => Pretty (Fix f) where
  pretty (Fix f) = liftPretty pretty prettyList f

If so, I’ll be happy to submit one 😊

@robrix
Copy link
Author

robrix commented Aug 22, 2017

For reference, here’s the implementation I’m working with currently:

https://github.com/robrix/abstract-interpretation/blob/b90ea99a977872fd1e31560b579c6cdfec33f6f7/src/Data/Functor/Classes/Pretty.hs

(I’d document everything before submission, of course.)

@quchen
Copy link
Owner

quchen commented Aug 22, 2017

Hm, the code doesn’t really look very nice, but since we now have Show1 etc. in Base, I think this is something we could include. I’d like to keep it minimally invasive though – from a Haddock standpoint, there shouldn’t be too many new exports, the (single exposed main) module is long enough as it is.

Can you explain why Pretty1Of is necessary a bit? I’m familiar with Show1, but not with Show1Of.

@robrix
Copy link
Author

robrix commented Aug 23, 2017

Hm, the code doesn’t really look very nice, but since we now have Show1 etc. in Base, I think this is something we could include.

I’ll do my best to match project style 😊

I’d like to keep it minimally invasive though – from a Haddock standpoint, there shouldn’t be too many new exports, the (single exposed main) module is long enough as it is.

Okay, you bet 👍

Would you be interested in including a generically-derivable implementation of Pretty1? This really isn’t make-or-break for me; we can always include it in our project and replace deriving Pretty1 with instance Pretty1 F where liftPretty = genericLiftPretty.

I’m guessing not given the above, but wanted to double-check since it wouldn’t necessarily have to increase the API’s surface area.

Can you explain why Pretty1Of is necessary a bit? I’m familiar with Show1, but not with Show1Of.

Oh, my bad—I don’t think we should include it at all. It’s a convenience to give you a Pretty instance if you have a Pretty1 instance, and helps to avoid orphan instances. I see little benefit to including it here, because presumably we’d just write a (non-orphan) Pretty instance in the first place.

@quchen
Copy link
Owner

quchen commented Aug 25, 2017

I don’t think Pretty1 should be derivable (likewise for Pretty), since prettiness is not something that follows any laws. If push comes to shove and we really need a quick Pretty instance, writing the instance without any definitions will just fall back to Show.

@quchen
Copy link
Owner

quchen commented Oct 17, 2017

I’m closing this issue since the corresponding PR (#40) looks good enough to subsume it.

@quchen quchen closed this as completed Oct 17, 2017
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

2 participants