Add code actions to disable a check for a line or the whole file#1217
Open
ryandiginomad wants to merge 7 commits into
Open
Add code actions to disable a check for a line or the whole file#1217ryandiginomad wants to merge 7 commits into
ryandiginomad wants to merge 7 commits into
Conversation
…er + drop redundant test cast
…ide {% liquid %} blocks
…r helper name, add outside-liquid test
…ing for disable-check code action
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are you adding in this PR?
ESLint-style quick-fixes for theme-check diagnostics, as requested in #432. When a theme-check offense is under the cursor, two code actions are offered:
<Check>for this line — inserts{% # theme-check-disable-next-line <Check> %}on the line above, indentation-matched<Check>for entire file — inserts{% # theme-check-disable <Check> %}at the top of the fileActions are grouped by check name, so selecting a range containing several offenses of the same check yields a single pair (not one per offense).
The disable-comment consumer already lives in
theme-check-common/disabled-checks; this PR only adds the producer — the code action that writes the comment for you — matching the scope @charlespwd clarified when reopening #432 ("the right click code transformation vs supporting it in code").Implemented as a new
DisableCheckProviderintheme-language-server-common, mirroringSuggestionProvider. UnlikeFixProvider/SuggestionProvider(which dispatch a server command to run an offense's fix function), this provider carries theWorkspaceEditdirectly, since the inserted text is known and needs no server-side computation. A smalltoEditCodeActionhelper was added next totoCodeActionfor that.fixes #432{% liquid %}blocksInside a
{% liquid %}tag, comments are bare#lines rather than{% # %}, so a{% # %}insertion would be invalid there. For now the for this line action is suppressed when the offense is inside a{% liquid %}block (detected viafindCurrentNode); for entire file is still offered (it inserts at the top of the file, always valid).What's next? Any followup issues?
#theme-check-disable-next-lineform for offenses inside{% liquid %}blocks (currently the line-level action is skipped there).{% liquid %}block within one selection, the first-seen offense decides whether the line-level action is offered and where the comment lands. Happy to refine to per-offense handling if you prefer.What did you learn?
theme-check-disable(no-next-line) disables from the comment to EOF, so the file-level action just inserts at the top — no need to track an end marker.Tophatting
Verified with unit tests (mirroring
SuggestionProvider.spec.ts) rather than manual VSCode tophatting — I don't have the extension running locally. Tests assert the exactWorkspaceEditfor both variants, grouping/dedup, multiple distinct checks, the no-offense case, and the{% liquid %}skip + outside-{% liquid %}cases.pnpm testfor the package: 440 passed, type-check and prettier clean.Before / after — choosing "Disable UnusedAssign for this line" on:
{% assign x = 1 %}becomes:
{% # theme-check-disable-next-line UnusedAssign %} {% assign x = 1 %}Before you deploy
changeset🤖 AI assistance disclosure — this PR was written with AI assistance (Claude Code). I directed the design (edit-based approach, the
{% liquid %}v1 skip), reviewed every change, and it follows TDD with unit tests verified locally. I understand the code and can explain why it's correct.Open to feedback on: (1) edit-based vs command-based (chose edit-based as there's nothing to compute server-side); (2) the
{% liquid %}v1 skip vs emitting bare#; (3) the representative-offense behavior noted above.