Skip to content

fix(oxlint/lsp): "ignore this" actions merge with existing directive#22604

Merged
graphite-app[bot] merged 1 commit into
mainfrom
05-19-fix_oxlint_lsp_ignore_this_actions_merge_with_existing_directive
May 20, 2026
Merged

fix(oxlint/lsp): "ignore this" actions merge with existing directive#22604
graphite-app[bot] merged 1 commit into
mainfrom
05-19-fix_oxlint_lsp_ignore_this_actions_merge_with_existing_directive

Conversation

@Sysix

@Sysix Sysix commented May 19, 2026

Copy link
Copy Markdown
Member

Pull request overview

This PR updates oxlint’s LSP “ignore this” code actions so that when a relevant disable directive already exists (oxlint or ESLint form), the action appends the new rule to the existing directive instead of inserting a new comment.

Now it will check for oxlint-disable-line first and searched oxlint-disable-next-line if no comment after the diagnostic is found.

closes #16060

Generated by GPT-5.3-Codex 🤖
Reviewed & manually tested by me.

Sysix commented May 19, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

@github-actions github-actions Bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server labels May 19, 2026
@Sysix Sysix force-pushed the 05-19-fix_oxlint_lsp_ignore_this_actions_merge_with_existing_directive branch 3 times, most recently from 4551be8 to db9504a Compare May 19, 2026 17:10
@Sysix Sysix requested a review from Copilot May 19, 2026 17:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates oxlint’s LSP “ignore this” code actions so that when a relevant disable directive already exists (oxlint or ESLint form), the action appends the new rule to the existing directive instead of inserting a new comment.

Changes:

  • Reuse existing // (oxlint|eslint)-disable-next-line … directives by inserting " {rule}" at the end of the directive line.
  • Reuse existing // (oxlint|eslint)-disable … “section/file” directives similarly.
  • Update LSP snapshot expectations and expand unit tests (including framework/section-offset cases like Vue script blocks).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
apps/oxlint/src/lsp/error_with_position.rs Adds detection/parsing to find an existing disable directive near the insertion point and appends the new rule instead of inserting a new directive; adds tests for merge behavior.
apps/oxlint/src/lsp/snapshots/fixtures_lsp_unused_disabled_directives@test.js.snap Updates snapshot to reflect the new TextEdit behavior (append rule text instead of inserting a new comment).

Comment thread apps/oxlint/src/lsp/error_with_position.rs Outdated
@Sysix Sysix force-pushed the 05-19-fix_oxlint_lsp_ignore_this_actions_merge_with_existing_directive branch 2 times, most recently from 3d0bc79 to 4fcec7a Compare May 19, 2026 18:41
@Sysix Sysix marked this pull request as ready for review May 19, 2026 18:44
@Sysix Sysix requested a review from camc314 as a code owner May 19, 2026 18:44

@camc314 camc314 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💪

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 20, 2026

camc314 commented May 20, 2026

Copy link
Copy Markdown
Contributor

Merge activity

…22604)

> ## Pull request overview
> This PR updates oxlint’s LSP “ignore this” code actions so that when a relevant disable directive already exists (oxlint or ESLint form), the action appends the new rule to the existing directive instead of inserting a new comment.

Now it will check for `oxlint-disable-line` first and searched `oxlint-disable-next-line` if no comment after the diagnostic is found.

closes #16060

Generated by GPT-5.3-Codex 🤖
Reviewed & manually tested by me.
@graphite-app graphite-app Bot force-pushed the 05-19-fix_oxlint_lsp_ignore_this_actions_merge_with_existing_directive branch from 4fcec7a to 36fc0ec Compare May 20, 2026 07:54
@graphite-app graphite-app Bot merged commit 36fc0ec into main May 20, 2026
27 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 20, 2026
@graphite-app graphite-app Bot deleted the 05-19-fix_oxlint_lsp_ignore_this_actions_merge_with_existing_directive branch May 20, 2026 07:58
Dunqing added a commit that referenced this pull request May 26, 2026
# Oxlint
### 🚀 Features

- 10da26b linter: `no-misleading-character-class` add suggestions for
regex literal (#22681) (Sysix)
- b84941e linter/vue: Implement no-expose-after-await rule (#22675)
(bab)
- 98b98c1 linter/vue: Implement no-computed-properties-in-data rule
(#22674) (bab)
- 868e2e8 linter: Add suggestion for `no-misleading-character-class`
(#22631) (Sysix)
- 2d4c919 oxlint: Support `vite-plus/resolveConfig` for vite.config.ts
(#22456) (leaysgur)
- 2a60012 linter/vue: Implement require-render-return rule (#22613)
(bab)
- 9f227fd linter/vue: Implement no-deprecated-props-default-this rule
(#21892) (bab)
- 9cd28b3 linter: Add debug option to print files to be linted (#22546)
(camchenry)
- 87f065e linter/vue: Implement return-in-emits-validator rule (#21935)
(bab)
- ea0380c linter/unicorn: Implement `import-style` rule (#22173) (Hao
Chen)
- dde40fe linter/vue: Implement no-watch-after-await rule (#22006) (bab)
- a735eb0 linter/vue: Implement valid-next-tick rule (#22531) (bab)
- 6dc615d linter/vue: Implement no-shared-component-data rule (#21842)
(bab)
- a656418 linter/vue: Implement valid-define-options rule (#22107) (bab)
- bb6f1b2 linter/vue: Implement require-slots-as-functions rule (#22244)
(bab)
- 5fa4774 linter/n: Implement `callback-return` rule (#22470) (Mikhail
Baev)

### 🐛 Bug Fixes

- 52bd016 linter: Respect allow unused disable directives in LSP
(#22715) (camc314)
- fa7c463 semantic: Correct TS enum member symbol spans (#22689)
(camc314)
- ed445ba linter: Respect flags overrides for `RegExp(/regex/i, "u")`
(#22678) (Sysix)
- 26b9396 semantic: Resolve parameter decorators outside parameter scope
(#22623) (camc314)
- 203952d linter: `no-misleading-character-class` fix
`is_unicode_code_point_escape` check (#22655) (Sysix)
- 2f64d3d linter: `no-misleading-character-class` own diagnostic for
surrogate pairs without u flag (#22654) (Sysix)
- 0c6ebc2 linter/eslint/no-lone-blocks: Do not flag empty loops (#22649)
(Mikhail Baev)
- 2a7562e linter/no-focused-tests: Mark fixer as a suggestion (#22645)
(camc314)
- dbe644f linter: Respect args none for unused rest parameters (#22627)
(camc314)
- d0211b0 linter: Allow undefined in DummyRuleMap index (#22626)
(camc314)
- 36fc0ec oxlint/lsp: "ignore this" actions merge with existing
directive (#22604) (Sysix)
- f7f370e linter/vitest/prefer-expect-type-of: Recommend `toBeTypeOf`
instead of `expectTypeOf` (#22612) (Mikhail Baev)
- 77063e5 linter/consistent-indexed-object-style: Preserve interface
modifiers in fixes (#22579) (camc314)
- 4f33aa7 linter: Treat `TSGlobalDeclaration` as ambient in
`has_ambient_typescript_ancestor` (#22577) (camc314)

### ⚡ Performance

- c22938d linter/no-async-endpoint-handlers: Populate node types
(#22601) (camc314)
- 607486e linter/no-negated-condition: Populate node types (#22602)
(camc314)
- 4dcaa59 linter/consistent-type-imports: Populate node types (#22600)
(camc314)
- 5bd3b25 linter/no-unused-vars: Avoid cloned ancestor iterator (#22598)
(camc314)
- 97fe9ba linter/no-extra-non-null-assertion: Reduce node matches
(#22588) (camc314)
- ae98296 linter/consistent-indexed-object-style: Populate node types
(#22578) (camc314)
# Oxfmt
### 🚀 Features

- 16b8058 oxfmt: Support `vite-plus/resolveConfig` for vite.config.ts
(#22454) (leaysgur)

### 🐛 Bug Fixes

- 5a26479 formatter: Preserve import phases (#22692) (Cameron)

### ⚡ Performance

- 78cf83f formatter: Pre-size output buffer using source text length
(#22594) (Dunqing)

Co-authored-by: Dunqing <29533304+Dunqing@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-editor Area - Editor and Language Server A-linter Area - Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extension: quick fix disable rule doesn't edit existing comment and creates duplicate disable comments instead

3 participants