-
Notifications
You must be signed in to change notification settings - Fork 18
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
Display SonarDelphi errors instead of failing scan #67
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.
Great change.
Surfacing errors and warnings in this way is a nice win for consistency with standard SonarDelphi scans, and much less disruptive when things do go wrong.
I also really like the Analysis Log UI and hope the Report a bug on GitHub
button will drive some useful bug reports to the upstream SonarDelphi project.
Testing went well.
- Should we have more visual feedback on the
Errors
andWarnings
buttons?- The little magnifying glass is cute, but easy to miss.
- Tooltip on hover saying "View analysis log" or similar
- Background color change on hover, like with other buttons.
- The GitHub icon in the Analysis Log is a nice touch, but there's some white pixels around it that look a little funny. BMP antialiasing issue?
Code changes are mostly good to go with a few nitpicks, see below.
01eac33
to
27da72f
Compare
(rebase) |
27da72f
to
616acbc
Compare
Updated now based on feedback. Log description: GitHub icon (dark theme): GitHub icon (light theme):
VCL is surprisingly restrictive with what it lets you do with buttons. I've added a tooltip, but the background color is totally dependent on the theme. In light mode, for example, it actually does darken the buttons: There's not too much I can do without implementing a custom control. I don't think what we have now will be too much of a problem - it's so colourful and angry that I expect most people will click on it anyway. |
616acbc
to
689d992
Compare
689d992
to
fac04a8
Compare
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.
Looks good to me. 🎉
This PR refactors the handling of errors that occur within SonarDelphi during analysis. Previously, any error-level logging would cause the analysis to fail with a fatal error, which is inaccurate - SonarDelphi is quite resilient to errors and can conduct a successful partial analysis in most cases an error appears. This new approach communicates the logs directly to the client along with any results, leaving the user free to determine how important the error is / continue using DelphiLint while they identify the problem.
A new "View Last Analysis Log" option is available in the "Analyze" dropdown:
Clicking this after an analysis shows a log window with options to save a log file or report an issue on integrated-application-development/sonar-delphi:
If the analysis produces errors, then the following shows up (clicking opens the log):
If the analysis produces warnings, then the following shows up (clicking opens the log):
Example of a log with errors: