Add functionality to render diagnostics with user provided Docs

[expipiplus1]

I originally implemented it with the user supplying a function Annotation -> ann along with their Doc anns. This made it certain that Diagnose wasn't messing with the user's types, however the implementation was much more clunky so I went with the simpler solution of adding another case to Annotation.

Here are two examples of: colorful user portions of messages and newlines + indentation in one of the messages

Also included:

• Some qol instances: Functor, Foldable, Traversable instances where appropriate, Eq and Ord for Note (Add instances for Error.Diagnose.Style #17)
• making withUnicode and tabSize their own types to make the printDiagnostic interface slightly less difficult.
• Major version bump because of that breaking change.

[Mesabloo]

This issue was already brought up to my attention in #15, and at that time we didn't think there was a good solution for this.
I have yet to review your PR (which I can't right now), but right from the beginning, some things are making me tick:

however the implementation was much more clunky so I went with the simpler solution of adding another case to Annotation.

I think this is not a good solution as we lose the semantics of the annotation type: instead of just describing the style of the document, we now have to worry about things which aren't style (and user-defined Doc fall into this category).

making withUnicode and tabSize their own types to make the printDiagnostic interface slightly less difficult.

I think we could do the same for the colors switch too, but this is a very minor change as the colors are already separated from the rendering logic.

Now here are my thoughts: originally, in #15, we debated whether it would be a good thing to have functions converting message type into Docs:

One fix (which is very dirty and completely voids the purpose of the Pretty typeclass, which is flawed anyway) would be for diagnose to internally pass msg -> Doc ann functions to every rendering function. But this is very cumbersome and error-prone. And this also bloats the function interfaces.
(#15 (comment))

This should work, and we could have two versions of printDiagnostic involved:

• printDiagnostic would simply use printDiagnosticNoPretty pretty (where pretty comes from the Pretty typeclass);
• printDiagnosticNoPretty f would require f :: msg -> Doc AnsiAnnotation (or some other type of annotation, in which case we would need to convert to the final type of annotation with another user-passed function). This allows us to use Docs inside reports pretty easily. Unfortunately, this solution does not quite get us to a clear semantics division, but this becomes the user's fault (kinda) instead of our fault.

This seems to me a better solution to the problem than the one you propose, and also should involve less changes in the codebase (basically, we only have to pass the prettifying function everywhere, why not as an implicit parameter, and it should mostly be all the changes required).

---

Let's discuss this more thoroughly before merging this PR (I also don't like merging PR knowing that we have a very big change coming soon, see #14; there are still a few more things to change before publishing a new version unfortunately).

[expipiplus1]

Thank you for taking the time to write up your thoughts.

I think this is not a good solution as we lose the semantics of the annotation type: instead of just describing the style of the document, we now have to worry about things which aren't style (and user-defined Doc fall into this category).

Specifically what semantics are these? Whatever they are can't they be retained by a clause saying "If the Annotation is OtherStyle then its provenance is the user code, and the user should be prepared to deal with whatever they put in"?

I think we could do the same for the colors switch too, but this is a very minor change as the colors are already separated from the rendering logic.

Indeed, however I think that being able to pass in unadordnedStyle (or whatever other colorless style) is a better solution here as we can remove this boolean for free.

One fix (which is very dirty and completely voids the purpose of the Pretty typeclass, which is flawed anyway) would be for diagnose to internally pass msg -> Doc ann functions to every rendering function. But this is very cumbersome and error-prone. And this also bloats the function interfaces.

It's true that we're only using pretty as a glorified fromString. However there's an additional (unenforced) requirement for input msgs we're relying on: which is that it doesn't insert any Nesting or Nest. One way or another, I think we'll have to either

• Implement the "denesting" logic I've implemented here in replaceLinesWith
• Disallow passing in Docs altogether (including smuggling them in via a Pretty instance or msg -> Doc ann)

(The specific issue with these constructions is that if one renders nest 10 foo then the pipes and other flourishes are also indented, oh no!)

(The absolute correct fix here would be to remove prettyprinter from the layouting, instead using it to layout individual messages and other ornamentation, and then proceeding to forward the rendered blocks to some compositing engine capable of overlaying blocks of text sensibly, although convenient for rendering the Doc that one gets back from diagnose is nonsense, as semantically related parts of text (for example multiple lines in the user message) have been interleaved with the diagnose's ornamentation)

Unfortunately, this solution does not quite get us to a clear semantics division, but this becomes the user's fault (kinda) instead of our fault.

(#15 (comment))

This should work, and we could have two versions of printDiagnostic involved:

* `printDiagnostic` would simply use `printDiagnosticNoPretty pretty` (where `pretty` comes from the `Pretty` typeclass);

* `printDiagnosticNoPretty f` would require `f :: msg -> Doc AnsiAnnotation` (or some other type of annotation, in which case we would need to convert to the final type of annotation with another user-passed function). This allows us to use `Doc`s inside reports pretty easily. 

TBH I don't think such an interface should be exposed if it's not possible to correctly use it, in that case only allow passing in Text.

This seems to me a better solution to the problem than the one you propose, and also should involve less changes in the codebase (basically, we only have to pass the prettifying function everywhere, why not as an implicit parameter, and it should mostly be all the changes required).

I should probably split this one up into the miscellaneous tidyings, the actual relevant change is not big (just changing the type sigs throughout Internal and the change to replaceLinesWith

[Mesabloo]

Specifically what semantics are these? Whatever they are can't they be retained by a clause saying "If the Annotation is OtherStyle then its provenance is the user code, and the user should be prepared to deal with whatever they put in"?

Alright, a took a better look at the code (more specifically how OtherStyle is being used) and this completely voids my complaint. In fact, it is way better than what I imagined/understood initially.

Indeed, however I think that being able to pass in unadordnedStyle (or whatever other colorless style) is a better solution here as we can remove this boolean for free.

That's something that I never considered, hence the simpler interface with booleans (which, in the case of colors, was only useful to remove all annotations, which is what a style can do).

It's true that we're only using pretty as a glorified fromString. However there's an additional (unenforced) requirement for input msgs we're relying on: which is that it doesn't insert any Nesting or Nest. One way or another, I think we'll have to either

• Implement the "denesting" logic I've implemented here in replaceLinesWith
• Disallow passing in Docs altogether (including smuggling them in via a Pretty instance or msg -> Doc ann)

Fortunately, we require msg to be Pretty in all the rendering spots. This means that (unless somebody is willing to write a very sub-optimal instance Pretty (Doc ann)) we cannot pass Docs currently, hence any msg cannot contain such constructs (Nesting and Nest).
The problem specifically manifests itself when Docs are embed within other docs.

(The absolute correct fix here would be to remove prettyprinter from the layouting, instead using it to layout individual messages and other ornamentation, and then proceeding to forward the rendered blocks to some compositing engine capable of overlaying blocks of text sensibly, although convenient for rendering the Doc that one gets back from diagnose is nonsense, as semantically related parts of text (for example multiple lines in the user message) have been interleaved with the diagnose's ornamentation)

I'm not sure that I entirely follow this, but from what I understand, having a two-layer rendering (rendering each individual block, e.g. the source code, the messages with the "arrows", the side rule, etc.) would be a lot better to create new layouts (which is the point of the issue/PR linked earlier, to allow for user-defined layouts), as one would simply need to write the logic for placing blocks and some default color style, and all blocks would be placed accordingly by the "compositing engine".

[expipiplus1]

Alright, a took a better look at the code (more specifically how OtherStyle is being used) and this completely voids my complaint. In fact, it is way better than what I imagined/understood initially.

Great :)

It's true that we're only using pretty as a glorified fromString. However there's an additional (unenforced) requirement for input msgs we're relying on: which is that it doesn't insert any Nesting or Nest. One way or another, I think we'll have to either

• Implement the "denesting" logic I've implemented here in replaceLinesWith
• Disallow passing in Docs altogether (including smuggling them in via a Pretty instance or msg -> Doc ann)

Fortunately, we require msg to be Pretty in all the rendering spots. This means that (unless somebody is willing to write a very sub-optimal instance Pretty (Doc ann)) we cannot pass Docs currently, hence any msg cannot contain such constructs (Nesting and Nest). The problem specifically manifests itself when Docs are embed within other docs.

What I mean is that even the Pretty instance is too much. For example this breaks in the current version of Diagnose

{-# LANGUAGE OverloadedStrings #-}

module A where

import Error.Diagnose
import Prettyprinter

data MyMsg = MyMsg

instance Pretty MyMsg where
  pretty MyMsg = nest 2 (vsep ["foo", "bar", "baz"])

main :: IO ()
main =
  printDiagnostic stderr True True 2 defaultStyle $
    def `addReport` Err Nothing MyMsg [(Position (1, 3) (1, 8) "some_test.txt", This MyMsg)] [Note MyMsg]

If nothing else, then this patch fixes this bug.

[Mesabloo]

Ah yeah, indeed. I never thought about this case.

---

Instead of continuing on a half broken base, perhaps we should investigate directly on the alternative of the compositing engine?
I think this is the way of continuing forward, and should fix all those problems (both newlines and nests and all other problems that were not found yet) because everything is pre-rendered in isolation, and put together afterwards. Not sure how one would create such engine though.
If you have any idea, let me know. :)

[expipiplus1]

Honestly, I would rather see this merged as-is, as I think it's a clear improvement over the status-quo.

Having said that, the way I would implement such a compositing engine would be:

• Determine widths for all sub-documents, this must be an easy task for this library width = totalWith - maximum (myOffset : offsetsOfMsgsAboveMe)
• Render msgs to SimpleDocStream using those widths
• Split on lines SimpleDocStream -> [SimpleDocStream] (Care must be taken to preserve annotations across line breaks!)
• Render the ornamentation according to the heights of the earlier rendered msgs, and split to [SimpleDocStream]
• Vertically pad with empty lines or whatever our msgs according to where they appear vertically
• Zip lines together with appropriate horizontal padding (outerline <> replicate ' ' (horizontalMsgPos - length outerline) <> horizontalMsgLine) (No reason for a little more complication, one could support true overlapping here)

[Mesabloo]

Hey, is there anything else that you'd be willing to include in the PR or can it be merged?
Overall it looks good to me.
I'll try and work on the compositing engine at some point in the future, to see what I can come up with and how easier it makes writing layouts.

[expipiplus1]

I think it should be good to go. I've been using it for a bit without any obvious issues.

[expipiplus1]

Thanks!