Skip to content

Code actions to disable shellcheck rules for lines or entire files #933

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

Open
polyzen opened this issue Aug 6, 2023 · 3 comments
Open

Code actions to disable shellcheck rules for lines or entire files #933

polyzen opened this issue Aug 6, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@polyzen
Copy link
Contributor

polyzen commented Aug 6, 2023

What is the problem this feature will solve?

Currently it seems there are only code actions to apply fixes when available. Code actions to disable the rules (using comments) would also be appreciated.

#490 (comment)

What is the feature you are proposing to solve the problem?

Additional code actions, eg. null-ls offers:

Disable ShellCheck rule 2059 for the entire file
Disable ShellCheck rule 2059 for this line

What alternatives have you considered?

No response

@polyzen polyzen added the enhancement New feature or request label Aug 6, 2023
@xero
Copy link

xero commented Aug 10, 2023

+1 for this idea 💡

@polyzen i’m not sure what editor you use, but as a stopgap, i’ve written a little neovim lua command to insert a shellcheck disable statement based on the current line’s lsp diagnostics into your buffer:

xero/dotfiles@f3821be#diff-cf8a96b3f61615a85a1fb4313e82a75b58d6e0bd71869e9c3b1ead9d1bbd91ab

hope this helps for now.

@Zeioth
Copy link

Zeioth commented Oct 15, 2023

@xero I think I found a better solution for neovim: #999 (comment)

screenshot_2023-10-16_01-48-35_133417388

@ThomasFaivre
Copy link

ThomasFaivre commented Dec 16, 2023

Hello there!

I have made a first draft to try to address this issue:
ThomasFaivre@d0b80ff
It does stuff (tested in neovim+LazyVim), but it is not finished for several reasons:

  • handle indentation
  • handle existing disable lines
  • make tests
  • Stuff I'm missing

I wanted to get some inputs before continuing.
And I have some questions. I am very new to Typescript (like, this is the second time I am touching javascript related code :D ), so I apologize if my questions are strange.

  • What would be the best way to get indentation? I'm assuming it would have something to do with the TextDocument, but how to get a given line? Kind of the same question to detect an existing disabling line.
  • Should I keep using getTextEdit* methods? Since data does not come from a ShellCheckReplacement object. I could use a TextEdit object directly. For instance, insertionPoint and precedence are not used.
  • codeActions variable could directly be an array, instead of a mapping of array with id as keys that we just flatten after creating it. But I'm afraid of breaking Fix ShellCheck code actions not working on some clients #700.
  • I have no idea how the node engine works, so there might be some optimization/performance concerns to deal with.

As for enhancement, this patch would open the possibility of adding a disabling of the rule at the scope level (function/if/while...) which does not exist in nvimtools/none-ls.nvim.

Also, tell me if you want a Pull Request in order to start a proper review.

Hope this will help!

ThomasFaivre added a commit to ThomasFaivre/bash-language-server that referenced this issue Dec 22, 2023
Implements bash-lsp#933.

Always add a code action to disable a given diagnostic for a given line
or for the whole file.
This patch handles:
 * indentation
 * existing directive. It also sorts the disable directives if any
 * multi-line command

Add corresponding tests.

Signed-off-by: Thomas Faivre <[email protected]>
ThomasFaivre added a commit to ThomasFaivre/bash-language-server that referenced this issue Dec 28, 2023
Implements bash-lsp#933.

Always add a code action to disable a given diagnostic for a given line
or for the whole file.
This patch handles:
 * indentation
 * existing directive. It also sorts the disable directives if any
 * multi-line command

Add corresponding tests.

Signed-off-by: Thomas Faivre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants