Open ony opened 4 years ago
I think this is an intriguing idea – I've run into similar issues in dhall
where many Pretty
instances currently look like
instance Pretty X where
pretty = unAnnotate . prettyX
Instances like this one are also suboptimal, because unAnnotate
can be rather expensive.
The obvious cost of this change is that every existing Pretty
instance would break, and many uses of pretty
too. A convincing case that argues that this cost is worthwile should involve a fair amount of smoke-testing in the ecosystem.
The obvious alternative is simply not to use Pretty
instances, and to rely on separate prettyX
functions. Is this not viable in your case @ony?
I think this is an intriguing idea – I've run into similar issues in
dhall
where manyPretty
instances currently look likeinstance Pretty X where pretty = unAnnotate . prettyX
Instances like this one are also suboptimal, because
unAnnotate
can be rather expensive.
Not sure about purpose of that. Are you using rewrite rules to fuse annotate . unAnnotate = id
that breaks laws and keeps annotation?
The obvious alternative is simply not to use
Pretty
instances, and to rely on separateprettyX
functions. Is this not viable in your case @ony?
Yes that is what I tried in hledger
Not sure about purpose of that. Are you using rewrite rules to fuse
annotate . unAnnotate = id
that breaks laws and keeps annotation?
What are you referring to with "that"? dhall
doesn't use rewrite rules. Here's some context in case it helps: https://github.com/dhall-lang/dhall-haskell/issues/1557
What are you referring to with "that"?
Sorry. I'm referring to this:
instance Pretty X where pretty = unAnnotate . prettyX
I don't understand purpose of using Pretty
and annotations together since you cannot implement pretty
only for one annotation and you cannot restrict annotations to some class in terms of which you'll construct them.
But I just thought about other crazy compatible approach. Just remembered how exceptions handled. I.e. you can "select" content on unpack and you can build chain of handlers.
instance Monoid SomeAnn -- this also allows to mix annotations of different types?
class Monoid ann => PrettyAnn ann where
toAnn :: ann -> SomeAnn
fromAnn :: SomeAnn -> ann -- mempty if no type match
-- (fromAnn . toAnn) == id (include re-write rules to collapse)
class Pretty a where
pretty :: PrettyAnn ann => a -> Doc ann
pretty = fmap fromAnn . prettySome
prettySome :: a -> Doc SomeAnn
prettySome = fmap toAnn . pretty
instance Pretty X where
prettySome = fmap toAnn . prettyX
prettyAnsi :: Pretty a => a -> Doc AnsiStyle
prettyAnsi = pretty -- doesn't loose AnsiStyle if prettySome provides it
handleMyStyle :: Pretty a => a -> Doc AnsiStyle
handleMyStyle = fmap remap . prettySome where
remap = \case
(SomeAnn ansi t) -> ansi <> remap t -- AnsiStyle as is
(SomeAnn Keyword t) -> color White <> bold <> remap t -- our own markup
(SomeAnn Comment t) -> italicized <> remap t
(SomeAnn _ t) -> remap t -- skip unknown annotation
EmptyAnn -> mempty
This shouldn't break anything since previous implementation of pretty
were required to implement any type including those restricted by PrettyAnn
.
Though I still believe indexing on both is more appropriate and probably efficient.
I don't understand purpose of using
Pretty
and annotations together since you cannot implementpretty
only for one annotation and you cannot restrict annotations to some class in terms of which you'll construct them.
Actually I'm not sure why dhall
has these Pretty
instances at all. Any code that uses them could just rely on the individual prettyX
functions, and would probably be clearer and possibly more efficient that way.
So I wonder whether prettyprinter
users should actually define their own Pretty
instances. I think the main value of that class is probably in the instances that prettyprinter
provides for the basic types.
The classes you propose, @ony, seem superior – however I'm wondering whether it would be best not to have a Pretty
class at all.
I think the main value of that class is probably in the instances that
prettyprinter
provides for the basic types.
In this case it is not clear why it is not defined as pretty :: a -> Doc Void
or something similar and then unAnnotate . pretty :: a -> Doc ann
, I guess.
The classes you propose, @ony, seem superior – however I'm wondering whether it would be best not to have a
Pretty
class at all.
Yes. I agree that maybe there should be no Pretty
, but PrettyAnsiStyle
and others in their individual packages.
I see the point, but it’s a huge breaking change for a very specific use case that I don’t think flies without a significant, popular use case. A new, type class (implemented by the consumer) is the better option I think.
I see the point, but it’s a huge breaking change for a very specific use case that I don’t think flies without a significant, popular use case.
What is a canonical implementation/use of class Pretty
from prettyprinter
?
As per
Annotation support
... Idris is a project that makes extensive use of such a feature. ...
But search points to class indexed by both type and annotation.
Unfortunately there is no much of examples for that part to grasp idea.
Is there a reason why pretty
and other functions are polymorphic on annotations without any restrictions? Is there a popular use case for that which is not caused by library itself?
I suspect that there is some decision behind this approach, which I don't understand at the moment and it may impair effective use of this library.
A new, type class (implemented by the consumer) is the better option I think.
Yes. It seems at least two people came to this conclusion independently. I raised this issue to either find out that I'm doing something wrong or that there is some improvement can be done in prettyprinter
. What I suggested is just a things that I come up with after not being able to implement Pretty
.
Mock-up of how it could look with Void
:
data Doc ann where
Empty :: Doc Void
Text :: String -> Doc Void
Unannotated :: Doc Void -> Doc ann
Annotated :: !ann -> Doc ann -> Doc ann
Cat :: Doc ann -> Doc ann -> Doc ann
instance Functor Doc where
fmap f = \case
Empty -> Unannotated Empty
Text s -> Unannotated (Text s)
Unannotated d -> Unannotated d -- no need to traverse deeper
Annotated x d -> Annotated (f x) (fmap f d)
Cat a b -> Cat (fmap f a) (fmap f b)
class Pretty a where
pretty :: a -> Doc Void
default pretty :: Show a => a -> Doc Void
pretty = Text . show
instance Pretty Int
class PrettyStr a where prettyStr :: a -> Doc String
instance PrettyStr Int where
prettyStr x
| x < 0 = Annotated "red" . Unannotated $ pretty x
| otherwise = Unannotated $ pretty x
-- subject for re-write: unAnnotate :: Doc Void -> Doc Void
unAnnotate :: Doc ann -> Doc Void
unAnnotate = \case
Empty -> Empty
Text s -> Text s
Unannotated d -> d -- early termination of re-writing annotations
Annotated _ d -> unAnnotate d
Cat a b -> Cat (unAnnotate a) (unAnnotate b)
renderSimple :: Doc Void -> String
renderSimple = \case
Empty -> ""
Text s -> s
Unannotated d -> renderSimple d
Annotated _ d -> renderSimple d -- dead code since no way to build Void
Cat a b -> renderSimple a ++ " " ++ renderSimple b
renderStr :: Doc String -> String
renderStr = \case
Unannotated d -> renderSimple d
Annotated x d -> "[" ++ x ++ "]" ++ renderStr d
Cat a b -> renderStr a ++ " " ++ renderStr b
main = do
let sample = prettyStr (-41 :: Int)
putStrLn . renderSimple . unAnnotate $ sample
putStrLn $ renderStr sample
That's pretty fascinating stuff @ony! :+1:
But I agree with @quchen that prettyprinter
is probably too mature for big changes like this. Maybe you can experiment further in a fork?!
Regarding the existing Pretty
class, I wonder whether we should add a sentence to discourage from defining instances if users may want to use annotations…
It looks like there is no way to annotate with colors when implement
Pretty
instance since its functions polymorphic on annotation type.Consider
Is there any suggestions about migrating in this case?
Suggestion
MultiParamTypeClasses
This seems to model
pretty
better since it is indexed by both what we render and which annotations we want to collect.In case of simple types we can keep
ann
unrestricted and allow rendering for any annotation type.