-
Notifications
You must be signed in to change notification settings - Fork 13
Add the ability to track warnings through re-compilation #368
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
base: main
Are you sure you want to change the base?
Conversation
| if warning_count > 0 { | ||
| tracing::info!( | ||
| "{} Finished {} in {:.2?} ({} warning{} tracked)", | ||
| "Compilation succeeded".if_supports_color(Stdout, |text| text.yellow()), |
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.
One big design change here is that I purposefully picked a new phrase + color to signify "Compilation has finished successfully, but there are warnings present." Yellow + the succinct message here (distinct from the normal "All good!" is meant to signify that.
src/ghci/warning_formatter.rs
Outdated
| // Detect different types of lines and apply appropriate coloring | ||
|
|
||
| // Source code lines with line numbers (e.g., " 28 | import Data.Coerce (coerce)") | ||
| if let Some(pipe_pos) = line.find(" | ") { |
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.
This is kinda hacky, there should at least be a comment here. Ideally we'd integrate this into the parser but I understand that's a more invasive change.
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.
Yeah if there was a struct I could plug into with the various pieces of a GHC message, that would be really helpful. We could store original color information there as well.
I'd like to put that off for a follow up (and left one comment-based pointer for that at the top here). Do you think another comment here would suffice for now?
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.
Added a comment here.
| /// - Files that were directly changed: always update warnings (or clear if none) | ||
| /// - Files that were recompiled due to dependencies: only update if warnings exist | ||
| /// - Files that weren't recompiled: keep existing warnings | ||
| pub fn update_warnings_from_log(&mut self, log: &CompilationLog) { |
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 think this is the only method that uses current_changed_files, would it make sense to make that variable a parameter to this function instead of a field on the struct?
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 could go either way? Do you feel strongly one way or the other?
My thinking here is that the struct keeps track of the things relevant for the feature: warnings and the files currently being changed (to determine which warnings are displayed). The latter could be thought of as an implementation detail but I wasn't sure if we'd want to surface the file list elsewhere externally too in the future.
ec7f5a2 to
adb82ea
Compare
patch,minor, ormajorto request a version bump when it's merged.docs/.tests/.This PR adds support for a "Track warnings" feature which keeps track of warnings that might disappear due to GHC's recompilation processes:

GHC has a behavior that causes "ephemeral warnings" to disappear during development. When a file with warnings is recompiled due to dependency changes (without the file itself
changing), GHC only re-emits warnings for files that were directly modified. This means warnings from unchanged files vanish from the output, making it easy for developers to
lose track of warnings that still need to be addressed.
Example scenario:
Solution
This PR introduces a --track-warnings flag that maintains an in-memory store of warnings per file. The system:
Usage
Enable warning tracking
ghciwatch --track-warnings
Or via environment variable
GHCIWATCH_TRACK_WARNINGS=1 ghciwatch
The feature is off by default to maintain backward compatibility. When enabled, developers get a complete view of all warnings across their codebase, eliminating the
frustrating experience of warnings disappearing during active development.