-
Notifications
You must be signed in to change notification settings - Fork 72
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
Support pretty diagnostic messages in CLI #908
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay here.
This is looking pretty neat, and surprisingly simple implementation too, I guess everything we need is already there in the 3rd party repo :)
I'm actually curious, maybe we should enable this as the default output. I think it would be useful for most users.
Anything in particular you think this is still missing?
src/AnalyzeCli.cpp
Outdated
// Use Reporter.hpp to create new reporter error | ||
auto file = new reporter::SimpleFile(name); | ||
auto err = reporter::Error( | ||
type, message, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the top line should include the error message, rather than just TypeError
- i think that should go as part of the code.
@JohnnyMorganz There are some terminal incompatibilities so I'd like to see what the results are on different machines. I want to look into other character sets. In my terminal, I had to manually allow it to show certain characters so I wonder if there is a way to detect that. Also, I think ultimately being able to group errors by file will be important. I can come back and take a look at those changes in depth but I think that should be a feature before enabling as default. If you are analyzing an entire project at once I could see this formatting becoming inefficient. |
Closes #495: You can now use the --formatter prettier flag in the CLI to get prettier errors
NOTES: