shellcheck: add code actions to disable rule#1069
shellcheck: add code actions to disable rule#1069ThomasFaivre wants to merge 1 commit intobash-lsp:mainfrom
Conversation
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 <thfaivre@gmail.com>
ThomasFaivre
force-pushed
the
fix_933_v2
branch
from
9f94f89 to
dd1f805
Compare
skovhus
left a comment
skovhus
left a comment
There was a problem hiding this comment.
Thanks! Seems like a great starting point. I suggest adding some unit test and the. After that see if we should improve the implementation.
| ) | ||
|
|
||
| expect(result).toHaveLength(1) | ||
| expect(result).toHaveLength(3) |
There was a problem hiding this comment.
Let us check what all the code actions are about. Consider just doing a snapshot test of the entire result object.
| result, | ||
| }: { | ||
| document: TextDocument | ||
| uri: string |
There was a problem hiding this comment.
No reason to pass in the uri if we have the document.
| lineNb, | ||
| }: { | ||
| document: TextDocument | ||
| lineNb: number |
There was a problem hiding this comment.
Nit: the nb abbreviation and postfix seems unnecessary.
| }) | ||
| } | ||
|
|
||
| // TODO: use directives module |
|
|
||
| private static getIndentation(line: string): string { | ||
| const match = line.match(/^([\s]*)/) | ||
| if (match && match[1]) { |
There was a problem hiding this comment.
Probably worth exploring using the AST for this.
|
@ThomasFaivre are you going to be working on this PR? |
|
Friendly ping, this would be really good feature to add (I keep getting confused what comment syntax is used to disable a rule). |
3 participants
Following my comment in #933, I ended up finishing my draft.
It is now fully functionnal:
Tested with Neovim (NVIM v0.10.0-dev-1846+gf31f260f0).
Therefore, I would like your opinion on the implementation.
I have already spotted some room for improvements:
What do you think?
Thanks!