Conditionally implement miette::Diagnostic for pest::error:Error

[bobbbay]

Hides an implementation of miette's Diagnostic type under the miette feature flag. Also adds example usage to the parens example.

The only issue I have with this is that the Display of error::Error changes based on this flag. Since miette displays the error and then the diagnostic, the error would be printed twice: once in pest's form, and once in miette's form. Hnce, error.rs:32-33 changes the display value of Error if the miette feature is enabled. In my opinion, it's not a clean solution, so I'd love to hear feedback.

Note: public API change: pest::error::Error::formt(&self) -> String is publicized, so users on the miette feature flag can still read Pest's errors if needed.

Closes #582

[CAD97]

The only issue I have with this is that the Display of error::Error changes based on this flag.

The standard workaround for this would be a .into_miette(self) -> impl Diagnostic adapter. (.as_miette(&self) -> impl '_ + Diagnostic would be nicer, but having 'static is necessary for normal use of miette::Report. You can format non-'static diagnostics, but it's more difficult.)

[bobbbay]

The standard workaround for this would be a .into_miette(self) -> impl Diagnostic adapter.

Sure, but what would the impl Diagnostic be? It can't be Self, because it will have the other, proper formatting as Display. Unless there's another type - MietteError that implements Diagnostic and From<Error> - but that overcomplicates the design.

[CAD97]

Personally, I don't think a non-exposed MietteAdapter(Error) (outside only sees impl Diagnostic) is a significant complication. The only difference is implementing Display/Diagnostic on MietteAdapter instead of Error directly.

For comparison, this is also the pattern used by Path::display.

[tomtau]

errors could also contain path and continued_line -- should those also be included in a Diagnostic (e.g. as additional labels) or would including those make the Miette default formatted print outs look strange?

[bobbbay]

Yes, it would make the miette-fornatted output strange. Adding it to the Display is fine, though.

[CAD97]

So, there's a way around it, but it's a bit hacky:

Provide a custom impl SourceCode which reattaches the line to the span offset information, and then the LabeledSpan can carry the correct span. If the label span is outside SourceCode, weird things can happen (mostly just dropping the source pointer, hopefully).

[CAD97]

I'd want to see a snapshot-style test that shows one or more instances of the miette-formatted error before merging this.

(This could be as simple as assert_eq!(format!("{it:?}"), "expected") or as complex as (waiting for) #661 and using insta or expect_test.)

That said, it's about time to pass the buck on pressing the green button, so...

[CAD97]

(apparently I forgot to push the button to unassign the team when it chooses a load-balanced member)

@pest-parser/maintainers should have write/merge permissions 🎉

[bobbbay]

Great!

(blocked by #666)

[prsabahrami]

Have you considered reopening this? I'm interested in working on this.

[tomtau]

@prsabahrami sure, feel free to go ahead. Do you want to start with the snapshot tests for the errors?

[tomtau]

@prsabahrami or it's also fine if you start directly with this PR and just add a simple test or two with assert_eq!(format!("{it:?}"), "expected") for the error messages