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

Labels on multiple source files #193

Open
LukeMathWalker opened this issue Aug 13, 2022 · 7 comments
Open

Labels on multiple source files #193

LukeMathWalker opened this issue Aug 13, 2022 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@LukeMathWalker
Copy link
Contributor

LukeMathWalker commented Aug 13, 2022

miette supports adding multiple labels to an error, but there seems to be a baked-in assumption that all labels refer to the same source file.

This is proving to be a limitation for a usecase of mine: highlight in the error both the usage location (i.e. where a function is called) and the definition location.
To make a practical example: report in an error a function parameter as being of the wrong type (by labelling the value passed in as parameter at the usage location) while reporting, alongside it, the expected parameter type in the definition location (once again using a label).

Is this too niche or would there be an interest in supporting it?
It would probably require a fair amount of breaking changes (e.g. Diagnostic::source_code could have to return an optional iterator, labels will have to store the associated source code, etc.).
You can see a potential implementation approach in ariadne (their multifile code example). They render them sequentially:
image

@LukeMathWalker
Copy link
Contributor Author

LukeMathWalker commented Aug 13, 2022

I tried to work around the issue by using related errors. It could work, but some adjustments would be required to the way related errors are displayed (or, most likely, I'd have to roll my own custom ReportHandler implementation)

Using an example from my project:

Error: 
  × I do not know how to handle the type (`Generic("T")`) of one of the
  │ parameters for `app::stream_file::<std::path::PathBuf>`.
   ╭─[src/lib.rs:7:1]
 7 │ pub fn blueprint() -> AppBlueprint {
 8 │     AppBlueprint::new().route(callable!(crate::stream_file::<std::path::PathBuf>), "/home")
   ·                               ─────────────────────────┬─────────────────────────
   ·                                                        ╰── The callable was registered here
 9 │ }
   ╰────

Error: 
  × The definition
   ╭─[src/lib.rs:2:1]
 2 │     
 3 │ ╭─▶ pub fn stream_file<T>(_inner: T) -> http::Response<hyper::body::Body> {
 4 │ │       todo!()
 5 │ ├─▶ }
   · ╰──── The callable was defined here
 6 │     
   ╰────

It is unclear, from the graphical output, that the latter error is actually a sub-error of the first one. They have the same heading (Error:) and they appear at the same level. Indentation might help, perhaps?

@zkat zkat added enhancement New feature or request help wanted Extra attention is needed labels Aug 14, 2022
@zkat
Copy link
Owner

zkat commented Aug 14, 2022

This seems related to #171! Maybe some input there would be helpful?

@LukeMathWalker
Copy link
Contributor Author

#171 is indeed related to the possible workaround - I left some comments, but it looks quite promising already!

What about the more architecturally-impactful approach (first-class support for label labels spanning multiple source files within the same error)?

@zkat
Copy link
Owner

zkat commented Aug 16, 2022

I think labels themselves spanning multiple files is a tough sell. Part of the point of miette is also to allow all this fancy error reporting with a simple data model, and attaching that to a nice proc macro. Miette used to be a bit more flexible about this stuff, but it was way more complex, and I'm more happy where it sits now.

@LukeMathWalker
Copy link
Contributor Author

LukeMathWalker commented Aug 16, 2022

Rephrasing, just to make sure I understand the concern: you are worried that supporting labels across multiple files (a niche-r usecase) will impact the ergonomics of the most common usecase (single file), therefore making this a net-negative tradeoff?

@zkat
Copy link
Owner

zkat commented Aug 16, 2022

correct. Older versions of miette supported this, and it was more complicated than it was worth. I think exploring things like #171 and such is a better alternative here.

@LHolten
Copy link
Contributor

LHolten commented Jan 11, 2024

You can actually have labels in multiple files with current miette.
To make it work you need to create a SourceCode implementation for one big "virtual" file that maps different ranges to the actual files. Then the SourceSpan in your Diagnostic should be offset + len in this "virtual" file and it will just work.
For example implementations you can look at nushell or my own compiler here.

When #324 is merged it will even show the correct file header above each context.

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

No branches or pull requests

3 participants