Pretty type-class compatibility with ansi-wl-pprint

[ony]

It looks like there is no way to annotate with colors when implement Pretty instance since its functions polymorphic on annotation type.

Consider

newtype Amount = Amount Int
instance Pretty Amount where
    pretty (Amount n)
        | n < 0 = annotate (colorDull Red) $ pretty n
        | otherwise = pretty n

Is there any suggestions about migrating in this case?

Suggestion

MultiParamTypeClasses

class Pretty a ann where
    pretty :: a -> Doc ann

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.

[sjakobi]

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?

[ony]

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.

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 separate prettyX functions. Is this not viable in your case @ony?

Yes that is what I tried in hledger

[sjakobi]

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: dhall-lang/dhall-haskell#1557

[ony]

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.

[sjakobi]

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.

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.

[ony]

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.

[quchen]

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.

[ony]

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.

[ony]

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

[sjakobi]

That's pretty fascinating stuff @ony! 👍

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…

[sjakobi]

@ony, what do you think about #256? It would be great to get your review! :)