Skip to content

Conversation

@shoplando
Copy link
Contributor

@shoplando shoplando commented Sep 18, 2025

What are you adding in this PR?

Solves #432

Added a new code action provider that allows users to disable Theme Check rules in the quick fix menu. This feature adds two types of actions:

  1. "Disable [CheckName] for this line" - Adds a {% # theme-check-disable-next-line CheckName %} comment above the line with the offense
  2. "Disable [CheckName] for entire file" - Adds a {% # theme-check-disable CheckName %} comment at the top of the file

What's next? Any followup issues?

What did you learn?

Working with code actions requires careful handling of cursor position and document ranges. It's important to filter duplicate actions when multiple offenses of the same type exist on the same line to avoid cluttering the quick fix menu.

Before you deploy

  • I included a minor bump changeset
  • My feature is backward compatible

Copy link
Contributor Author

shoplando commented Sep 18, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@shoplando shoplando changed the title add command to insert theme-check disable comments and tests added code actions for disabling theme-check rules Sep 18, 2025
@shoplando shoplando marked this pull request as ready for review September 18, 2025 20:24
@shoplando shoplando requested a review from a team as a code owner September 18, 2025 20:24
@shoplando shoplando force-pushed the Add-code-action-to-disable-checks branch 6 times, most recently from eda6b09 to 142ef1c Compare September 25, 2025 20:35
case 'next-line': {
let existingCommentLine = -1;

for (let i = lineNumber - 1; i >= Math.max(0, lineNumber - 5); i--) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's with these numbers? Why are we deducting -5 from lineNumber?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I thought of only looking at maximum 5 lines above the current line because I didn't think people would write more than 5 lines of whitespace, but I now that I think of it, devs should do anything they want.
(same on line 88)

const { textDocument } = document;

const isInLiquidTag = this.isInLiquidTagContext(
document as AugmentedLiquidSourceCode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
document as AugmentedLiquidSourceCode,
document,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shoplando shoplando force-pushed the Add-code-action-to-disable-checks branch 2 times, most recently from 28600e9 to 574b199 Compare October 1, 2025 18:45
@shoplando shoplando force-pushed the Add-code-action-to-disable-checks branch from 574b199 to b0e517b Compare October 1, 2025 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants