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

Derive macro: single field for multiple snippets #43

Closed
Tracked by #45
Schniz opened this issue Sep 7, 2021 · 7 comments
Closed
Tracked by #45

Derive macro: single field for multiple snippets #43

Schniz opened this issue Sep 7, 2021 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Schniz
Copy link

Schniz commented Sep 7, 2021

Allowing dynamic list of spans to provide highlights you can be good for using miette to wrap tools like TypeScript (tsc).

Looking at the source code, maybe Vec<SourceSpan> might not cut it, but introducing a new LabelledSourceSpan and allowing everything that is Into<Vec<LabelledSourceSpan>> could also make sense.

What do you think?

@zkat
Copy link
Owner

zkat commented Sep 7, 2021

You can actually provide as many snippets as you want if you use the Diagnostic protocol directly, which is probably a good idea if you're wrapping a tool that spits out many snippets: https://docs.rs/miette/2.0.0/miette/trait.Diagnostic.html#method.snippets

Is this what you're looking for?

@Schniz
Copy link
Author

Schniz commented Sep 7, 2021

Ha! Indeed! This issue can be closed if you don't think the drive micro should support this use case (but I'd be happy to know and learn why!)

Anyway, thanks for your work! It works great :)

@zkat zkat added enhancement New feature or request help wanted Extra attention is needed labels Sep 7, 2021
@zkat
Copy link
Owner

zkat commented Sep 7, 2021

I mean, I guess the derive macro could support this! I'll leave this issue open in case someone wants to contribute it, but I have no drive to do this myself. Maybe you can nerd snipe @cormacrelf into wanting to do it? ;)

I'm thinking maybe something like a #[all-snippets] foo: Vec<impl Into<DiagnosticSnippet>> or something of the sort?

@zkat zkat changed the title Allow Vec<SourceSpan> for highlights Derive macro: single field for multiple snippets Sep 7, 2021
@zkat zkat added the good first issue Good for newcomers label Sep 7, 2021
@cormacrelf
Copy link
Contributor

Ah yes, @, the nerd sniper's targeting reticle. Good shot.

First off, I am pretty sure the lifetime on DiagnosticSnippet<'a> makes Into a bit awkward, maybe even a for<'a> kind of deal. You don't want to foray too far into that if you can avoid it ;) At least with macros you don't usually need to write down a trait's name to use it. Generally the Diagnostic trait itself with &self receivers is better suited to the task of implementing one of its methods than Into with a self receiver. Telling people to impl<'a> Into< box dyn gore > for &'a MySnippet (?) is not exactly the pinnacle of usability.

With that in mind, this is maybe a use case for my pet #41. If you can implement the Diagnostic::snippets function on a struct SnippetList(Vec<_>); then you could reuse it on other diagnostic types via the macro, no extra traits required. I figured there'd be a need for some composability of the snippets, but I would have thought it would be when building tons of similarly shaped diagnostics. As you have observed, for wrapping another compiler's output, you probably want to implement Diagnostic manually with all the methods in one spot on one type. So not quite a solution to the "solution in search of a problem" problem yet. Not specifically this, anyway, but there is definitely crossover and 41 would help anyone who wanted what was requested here.

Another option is adding an OwnedSnippet type to store in vectors, almost identical to DiagnosticSnippet except the source, which would be owned; then this all becomes very easy. It loses flexibility and I don't know who would miss that but the most plausible idea would be localisation of error messages, hints and such. Miette seems to be uniquely positioned for accessibility right now so I would want to keep that kind of thing open. I hope one day more compilers can have multilingual UIs.

@cormacrelf
Copy link
Contributor

Also just noticed 2.0, nice work! You SEMVER BUMP miette? You interpret her MUST and MAY NOT as described in the RFC 2119?

@zkat zkat mentioned this issue Sep 10, 2021
14 tasks
@zkat zkat closed this as completed Sep 22, 2021
@wizard-28
Copy link

wizard-28 commented Nov 15, 2022

Was this attribute ever added to derive macro? I can't find it in the docs.

@zkat
Copy link
Owner

zkat commented Nov 15, 2022

An attribute was not added, no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants