miette
miette copied to clipboard
Render inner diagnostics
Previously #[diagnostic_source]
did not correctly render the inner diagnostic. This PR fixes that.
There's a change here that can be considered controversial:
- Only print newlines when there is an actual header (in ad9d0378ddd6dd4a6b9e650dd6aa62116da65910)
I am fine with removing it, but then the nested diagnostic source will be offset by one line and appear floating when it does not print a header.
PS: This PR also includes things from #169. I will rebase once that is merged (hopefully?)
I'm a little concerned about all the extra whitespace, and the potential complexity of displaying multiple errors in their entirety (as opposed to just the header, which I think was ideal).
I am not sure why miette
would need to be aware of this? If a user wishes to not have it displayed, they can choose not to use the features that miette
gives them. Since with an empty impl of Diagnostic
it looks exactly how the Error
output.
I'm not sure this matches up with what I'd like to see.
I absolutely understand that this is what I'd call a 'controversial' change, so I def want this to evolve to help everyone using miette
. Could you elaborate a bit more on what parts you think do not fit? (Unless they are those you've already mentioned)
I'd love to come up with some more examples/combinations of how this could look like in some situations. (I've also gone one step further regarding the related
questions in #171 but I'd say that is even more controversial and can wait :P)
If a user wishes to not have it displayed, they can choose not to use the features that miette gives them.
At scale, a dev does not necessarily have control over whether the sources they display are Diagnostics or not, and they might be surprised when they suddenly get a ton more output.
Could you elaborate a bit more on what parts you think do not fit? (Unless they are those you've already mentioned)
I just think we can do better than just rendering the entire diagnostic inline like this.
I'd love to come up with some more examples/combinations of how this could look like in some situations.
Ultimately, this is what I really want (for this and for #171). I'm not sure I like how they look right now, and I'd like to explore some more ideas of how things look before we move on to implementation. Just figure out the best end result, regardless of current implementation, and work backwards from there. Heck, we can even change the general structure of error printing right now if it makes sense!
At scale, a dev does not necessarily have control over whether the sources they display are Diagnostics or not, and they might be surprised when they suddenly get a ton more output.
Hm, I'd agree if source
and diagnostic_source
would be the same annotation, but if a dev puts diagnostic_source
into their code then I do believe they should expect it to be printed as a diagnostic! (Otherwise, they could just use it as an Error
& source
)
I just think we can do better than just rendering the entire diagnostic inline like this.
Alright! I am not exactly sure what the alternatives are haha, but I'd love to discuss it.
Ultimately, this is what I really want (for this and for https://github.com/zkat/miette/pull/171). I'm not sure I like how they look right now, and I'd like to explore some more ideas of how things look before we move on to implementation. Just figure out the best end result, regardless of current implementation, and work backwards from there. Heck, we can even change the general structure of error printing right now if it makes sense!
Sounds good to me, I'll be able to get back to it probably next week, so I'll try to hash out some nice error structures then, so we can compare/discuss.
if a dev puts
diagnostic_source
into their code then I do believe they should expect it to be printed as a diagnostic!
Yes, definitely agree here. It’s opt-in and I would expect (as seen in #172) the diagnostics to be included in this case.
So, I've been thinking about this, and I would suggest the following path forward:
- Create a recursive struct of all the information the errors contain (or don't contain)
struct ErrorDescription {
error: String,
causes: Vec<ErrorInformation>,
related: Vec<ErrorInformation>,
source_code: ...
}
- Make the reporter accept that ErrorDescription and print its respective representation
- Now that it is hopefully easier to refactor printing, discuss with more datapoints!
This would also make it easier for other crates to replace the reporter since they don't have to gather the information in the first place and simply use the given information.
What do you think?
Ok I think we can merge this. Do you mind rebasing/resolving conflicts?
@zkat there we go!
One last thing: do you mind writing a quick test to show what these nested diagnostics actually look like?
Duh, totally forgot that one haha
It prints them indented as before, but simply prints them as a whole rather than just their debug info
Thanks all for getting this in! 🎉