Added Suppress rule for this line / Suppress rule for file quick actions / Show documentation#530
Conversation
Fix possible issue with incorrect command run when 2 rule single fixes are available
|
|
|
@dbaeumer any chance this could be merged? Thanks |
server/src/eslintServer.ts
Outdated
| } | ||
|
|
||
| // cache documentation urls for all rules | ||
| ruleDocUrls.clear(); |
There was a problem hiding this comment.
Is it necessary to cache this on every validate which is actually run very frequently or could we cache this once on only clear the cache if the .eslintrc file changes?
| CommandIds.applyAutoFix, | ||
| CommandIds.applyDisableLine, | ||
| CommandIds.applyDisableFile, | ||
| CommandIds.openRuleDoc, |
There was a problem hiding this comment.
This is not necessary since the is sent from the server to the client and not the other way.
There was a problem hiding this comment.
When I removed this, the quick action for Show documentation stopped working. I think if we don't specify it here, the command doesn't get sent from quick actions to the server.
There was a problem hiding this comment.
Makes sense. I misunderstood what you wanted to achieve
server/src/eslintServer.ts
Outdated
| let workspaceChange = new WorkspaceChange(); | ||
| workspaceChange.getTextEditChange({uri, version: documentVersion}).add(createTextEdit(editInfo)); | ||
| commands.set(CommandIds.applySingleFix, workspaceChange); | ||
| let lineText = textDocument.getText(Range.create(Position.create(editInfo.line - 1, 0), Position.create(editInfo.line - 1, 255))); |
There was a problem hiding this comment.
You can use Integer.MAX_VALUE here instead of 255 to be on the save side. Will have the same effect.
| @@ -181,7 +198,8 @@ interface AutoFix { | |||
| label: string; | |||
There was a problem hiding this comment.
Since with the change we now store problem in general per file shouldn't we rename this. Something that contains problem in the name
There was a problem hiding this comment.
FixableProblem might be an idea.
Use MAX_VALUE to get line instead of 255
loune
changed the title
|
Very nice work. |
|
Thanks @dbaeumer ! |
|
I did some minor tweaks. One thing I forgot about is that different files can have different .eslintrc files (in different workspace folders) so we need to remember the rules per file. Changed this and pushed. Will publish a new version of ESLint beginning of next week. |
|
Great improvement! Thanks a lot for this! |
|
Is there an option to hide those additional menu items? |
|
Actually that is a fair request. @loune would you be willing to add such a setting ? |
|
Sure, I can have a look at adding a setting. |
|
Hello, great addition, but what happened to Fix All of a specific problem? thank you. |
|
@alexiosd this patch shouldn't change how Fix All of a specific problem works. I'll see what's wrong with that as well. |
6 participants
Added show documentation quick action
Fix possible issue with incorrect command run when 2 rule single fixes are available. This is due to
CommandIds.applySingleFixgetting overriden insidecommandsby the last added single fix quick action. The fix is to include theruleIdin the key when adding tocommands.