microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.18k stars 28.84k forks source link

Skip re-requesting implicit code actions if diagnostics around the cursor have not changed #215340

Open mjbvz opened 3 months ago

mjbvz commented 3 months ago

https://github.com/microsoft/vscode/blob/a0185ee7cbcceda421361a2af44989dd40916a51/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts#L57

I believe the implicit code actions are re-requested whenever diagnostics in the file change. Instead I proposed that we only re-request code actions if the diagnostics on the cursor's currently line change. For example, if I'm one line 100 and the diagnostics at the very top of the file change, we should not compute code actions

This is impacting some prototype work that TS is doing around sending incremental diagnostic updates

mjbvz commented 3 months ago

Also I'm not 100% sure this is correct but _onMarkerChanges should perhaps check that the cursor is not whitespace enclosed before requesting the diagnostics. I just noticed that we do that in some cases but not others

justschen commented 3 months ago

gotcha! some points/questions:

https://github.com/microsoft/vscode/assets/54879025/ffa38b54-8d09-4613-b120-f1b72dc0eccf

mjbvz commented 3 months ago

Good points. Adding a few thoughts:

  1. Yes that's used for the lightbulb widget but worth exploring if we're firing too often here too

  2. Possible today but seems kind of suspect if an extension/language is actually doing it. I'd say we shouldn't go out of our way to support it

    If needed, we could always add a new event to the API called something like CodeActionProvider.didChangeCodeActions that an extension could fire to signal that code actions should be explicitly re-requested. But I wouldn't do this unless there's a really good use case

  3. You may be seeing this when the markers shift position as you type. VS Code does that automatically. If possible, it seems like we should only update the code actions when the markers really do change