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

How to deal with big content #15

Closed
aboeglin opened this issue Oct 22, 2022 · 7 comments
Closed

How to deal with big content #15

aboeglin opened this issue Oct 22, 2022 · 7 comments
Labels
bug Something isn't working question Further information is requested

Comments

@aboeglin
Copy link

aboeglin commented Oct 22, 2022

Hello,

I'm currently testing your library and the diagnostics look really good!

I am just having one issue and it's unclear to me how to solve it from the docs. If I have content that spans over multiple lines and it breaks the layout. Is it possible to use say Docs instead of Text/String to create a Report? Or what would be the way of doing something like this? Say showing actual/expected types in a type error, or having a type that is long ( record type or function types for example ).

@Mesabloo
Copy link
Owner

Mesabloo commented Oct 22, 2022

Hi there.

Could you please share an example (ideally with a simple way of reproducing) that breaks the current layout?
Also, yes it is possible to use Docs. In fact, it is possible to use any type t which has a Pretty t instance. (this is the only requirement)

I have personally not have errors which broke the layout. If the input Text/String is long and unbroken (meaning that there is no \n within), then the layout will not try to break it for you (this is the default expected behavior of prettyprinter). This means that if you output a type which is very long, it is your responsibility to make it fit within the acceptable boundaries of a text. (usually, this lis between 80 and 100 characters per line, but this may vary)

If the docs are unclear on that matter, I'll happily change them so that they're easier to understand! (this will wait until #14 is merged though, which changes a whole lot of things)

@Mesabloo Mesabloo added the question Further information is requested label Oct 22, 2022
@aboeglin
Copy link
Author

Well I believe the example is precisely what you highlighted. Like it's not breaking the layout per se but it's more that the line goes on and visually starts column one. When trying it with a Report (Doc ann) I get the following issue:

No instance for (Pretty.Pretty (Pretty.Doc ann0))
        arising from a use of ‘Diagnose.prettyDiagnostic’

Do you have an example somewhere using Doc as the msg type?

@Mesabloo Mesabloo added the bug Something isn't working label Oct 22, 2022
@Mesabloo
Copy link
Owner

I see.
I am facing the same issue, because Doc is not prettyprintable (I was unaware of that).
Unfortunately, I don't really know whether there is an easy fix for this (other than the “hacks” present in the issue linked, which in this case rely on defining orphan instances) or not.

@aboeglin
Copy link
Author

I see. So the best option for now would be to render it to Text beforehand to have the line returns in place with a width of like 50-60?

@Mesabloo
Copy link
Owner

Pre-rendering should work, provided that there are lines (or hardlines) within the Document to render.
Otherwise, it would pretty much have no effect (prettyprinter does not break long lines for you if there are no lines within, but it is able to otherwise).

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.
If GHC supported local instances (as e.g. Agda), we could define a printDiagnostic' which would internally create an instance given the type of a pretty-printing function passed as parameter. Unfortunately this is not the case, and I don't see it added soon (because it adds a lot of complexity to a system which is already very complicated).
The other solution is the one present in the issue, that is to say to define this instance:

instance Pretty (Doc ann) where
  pretty = unAnnotate

But this greatly decreases performance (I am confident that using unsafeCoerce in place of unAnnotate will not work) so it should be avoided as much as possible.

@aboeglin
Copy link
Author

Thanks for taking the time to give a detailed answer! I’ll try my chance with pre-rendering it then. Sounds like the best compromise to me.

@Mesabloo
Copy link
Owner

No problem! I'll close this issue as there is pretty much nothing I can really do to solve this issue (because it originates from prettyprinter itself).
Thanks for the report too! Now I know that Doc ann cannot be pretty-printed (for good reasons though). :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants