Page MenuHomePhabricator

[clangd] Handling text editor/document lifetimes in vscode extension.

Authored by jvikstrom on Aug 26 2019, 5:25 AM.



Just reapplies highlightings for all files when visible text editors change. Could find the correct text editor that should be reapplied but going for a simple implementation instead.
Removes the associated highlighting entry from the Colorizer when a text document is closed.

Diff Detail

Event Timeline

jvikstrom created this revision.Aug 26 2019, 5:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2019, 5:25 AM
hokein added inline comments.Aug 26 2019, 5:49 AM

I think we can do it like vscode.window.onDidChangeVisibleTextEditors((TextEditor[] editors) => { // call applyHighlight for each text editor }).


the name is a bit weird onDidCloseTextDocument in the Highlighter, I would name it removeFileHighlights

jvikstrom marked an inline comment as done.Aug 26 2019, 6:03 AM
jvikstrom added inline comments.

There might be TextEditors that are not a c++ file/haven't gotten highlightings yet.

So we'd still need to do the this.files.has(fileUri) check which we also do in the initialize function.

Maybe I should just expose the reapplyAllHighlightings function and call that one instead?

hokein added inline comments.Aug 26 2019, 6:12 AM

if the file is not a c++, the current implementation of applyHighlights is tolerant with that I think, it doesn't do anything (we get an empty decoration ranges).

I think we can do the this.files.has(fileUri) check in applyHighlights.

jvikstrom updated this revision to Diff 217131.Aug 26 2019, 6:31 AM

Address comments.

jvikstrom updated this revision to Diff 217134.Aug 26 2019, 6:36 AM

Fixed changes that weren't supposed to be made.

hokein accepted this revision.Aug 26 2019, 6:42 AM

I think there is one more case -- we need to cleanup the highlighting cache if clangd crashes, the extension will automatically restart clangd up to 5 times if it sees clangd crashes, you can see how filestatus handles it.


I think remove all highlightings for the file with the given fileUri. is enough.


nit: the trailing If?


could you move this method after highlight?

This revision is now accepted and ready to land.Aug 26 2019, 6:42 AM
This revision was automatically updated to reflect the committed changes.
jvikstrom marked 4 inline comments as done.