Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR reapplies functionality for displaying unused variable warnings in the Hazel editor. The feature adds visual indicators and user interface elements to show warnings about unused variables, with a toggle to enable/disable warning display.
Key Changes
- Adds a warnings system with visual styling to differentiate warnings from errors
- Implements unused variable detection for pattern variables
- Adds a user settings toggle to control warning display visibility
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/web/www/style/variables.css | Defines warning-specific CSS color variables and z-index values |
| src/web/www/style/editor.css | Adds styling for warning decorations with dashed borders |
| src/web/www/style/dynamics.css | Updates error message selector syntax |
| src/web/www/style/cursor-inspector.css | Styles cursor inspector warning states |
| src/web/view/NutMenu.re | Adds warnings toggle to developer menu |
| src/web/app/inspector/CursorInspector.re | Implements warning display logic in cursor inspector |
| src/web/app/editors/decoration/Deco.re | Adds warning decorations to editor |
| src/web/app/common/ProjectorView.re | Adds warning state to projector status |
| src/web/Settings.re | Adds display_warnings setting |
| src/language/statics/Warning.re | Defines warning types and unused variable detection |
| src/language/statics/Statics.re | Integrates warning detection into static analysis |
| src/language/statics/Info.re | Adds warning field to info types |
| src/language/statics/CoCtx.re | Adds hole detection utility |
| src/language/CoreSettings.re | Adds display_warnings to core settings |
| src/haz3lcore/derived/CachedStatics.re | Tracks warning IDs alongside error IDs |
| } | ||
|
|
||
| .cell-item.cell-result .error-msg { | ||
| .cell-item.cell-result .error-msg .warning-msg { |
There was a problem hiding this comment.
The CSS selector is malformed. It should use a comma to separate selectors for both error and warning messages: .cell-item.cell-result .error-msg, .cell-item.cell-result .warning-msg {
| .cell-item.cell-result .error-msg .warning-msg { | |
| .cell-item.cell-result .error-msg, .cell-item.cell-result .warning-msg { |
| let e_co_ctxs = | ||
| List.map2(CoCtx.mk(ctx), p_ctxs, List.map(Info.exp_co_ctx, es)); | ||
| List.map2( | ||
| ((p_ctx, _ctx)) => CoCtx.mk(p_ctx, p_ctx), |
There was a problem hiding this comment.
The CoCtx.mk call uses p_ctx for both arguments, but the original code used ctx for the first argument. This change may affect context handling incorrectly.
| ((p_ctx, _ctx)) => CoCtx.mk(p_ctx, p_ctx), | |
| ((p_ctx, _ctx)) => CoCtx.mk(p_ctx, ctx), |
There was a problem hiding this comment.
I've been scrutinizing these changes flagged by Copilot and am confused. Commenting them out makes unused variable warnings appear even when they are used.
But I don't understand why it would make sense to pass in p_ctx for both ctx_before and ctx_after args to CoCtx.mk. Separately, seems weird to zip together the constant ctx with all the p_ctxs only to ignore it later, assuming that isn't needed at all. Finally, while I don't follow why this change makes sense, I'm pretty sure this would mess up the co-ctx calculation for the overall case expression.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #1832 +/- ##
==========================================
- Coverage 50.15% 50.11% -0.04%
==========================================
Files 231 232 +1
Lines 25513 25575 +62
==========================================
+ Hits 12796 12818 +22
- Misses 12717 12757 +40
🚀 New features to boost your workflow:
|
|
@cyrus- @disconcision fixed merge conflicts, seems to work as expected 
|
Resolve merge conflicts keeping both warning support and dev features (targets/sampling, meta-down, line numbers, selected-expanded styles). Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
@cyrus- thoughts here? #1832 (comment) |
3 participants
This reverts commit 2bdb606, reversing changes made to 5328152. See original PR at #1033.