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: better use of spans and file information in error messages #102

Merged
merged 7 commits into from
Jul 30, 2024

Conversation

Kijewski
Copy link
Collaborator

No description provided.

@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from 4aea79a to 380ecc1 Compare July 29, 2024 03:05
error: failed to parse template source at row 1, column 27 near:
" %}\nHELLO\n{{v}}\n{%- endfilter %}"
error: failed to parse template source
--> <source attribute>:1:28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this one we could actually get the current file name.

Copy link
Collaborator Author

@Kijewski Kijewski Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we know from which file we were called, do we? Not until rust-lang/rust#54725 lands. And the path is written just below this error message, so I don't think that is matters much.

@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from 1110a8b to fa8c936 Compare July 29, 2024 11:46
@Kijewski
Copy link
Collaborator Author

It's a huge diff, but I think it's worth it. At least to me error messages are now much more readable.

I cannot get vmware to run my windows VM for quite some time, so I cannot test if it works in windows, too.

@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from ec9cffd to 6af5220 Compare July 29, 2024 13:43
@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from 6af5220 to c3639ec Compare July 29, 2024 13:48
|
3 | #[derive(Template)]
| ^^^^^^^^
1 | {{ 1234 as 4567 }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, is it normal to have this super wide space starting here?

Copy link
Collaborator Author

@Kijewski Kijewski Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is aligned to start after "error: ", because it belongs to the rust source that is printed below. At first, I did not find out how to remove the indentation, now I kinda like it, because it gives you a visual "this happened because of that" hint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit I hate it. ^^'

I'd prefer if we kept coherency with rustc (with columns aligned to the one with the biggest line error number):

$ rustc foo.rs 
error[E0425]: cannot find value `Ba` in this scope
  --> foo.rs:10:13
   |
7  | pub struct Bar;
   | --------------- similarly named unit struct `Bar` defined here
...
10 |     let x = Ba;
   |             ^^ help: a unit struct with a similar name exists: `Bar`

error: aborting due to 1 previous error

@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from 3b2c074 to cf75d27 Compare July 29, 2024 14:46
@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from cf75d27 to 2e887a5 Compare July 29, 2024 14:50
@Kijewski
Copy link
Collaborator Author

821a147. In my terminal I can see the messages, but they aren't captured by trybuild or vs code. I guess that proc_macros aren't supposed to just print something into stderr. Maybe I have to wrap the output in some magic message, but if so, I have no idea what.

}) => {
let msg = if rendered {
eprintln!("{msg}");
"the previous template error derives from"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny that only this part of the error message is displayed. Dark magic.

@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from 2e41056 to 08674bd Compare July 29, 2024 19:57
@Kijewski
Copy link
Collaborator Author

I removed the formatting/pretty printing of the annotation in the last commit. Maybe we can merge this stuff for now? Even without the pretty output, it has a ton of minor improvements. I would then open a new PR to get the pretty rendering going.

@Kijewski Kijewski marked this pull request as ready for review July 29, 2024 20:00
@Kijewski Kijewski changed the title derive: properly display error locations derive: better use of spans and file information in error messages Jul 29, 2024
@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from c280cd1 to 7552ed1 Compare July 30, 2024 10:57
@Kijewski Kijewski force-pushed the pr-annotate-snippets branch from 7552ed1 to 5e98580 Compare July 30, 2024 12:04
@GuillaumeGomez
Copy link
Contributor

Looks good, thanks a lot!

@GuillaumeGomez GuillaumeGomez merged commit 4332dda into rinja-rs:master Jul 30, 2024
17 checks passed
@Kijewski Kijewski deleted the pr-annotate-snippets branch July 30, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants