hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land.
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 <https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts#L146-L156> handles it. ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:177 + // Called when a text document is closed. Removes any highlighting entries for + // the text document that was closed. ---------------- I think `remove all highlightings for the file with the given fileUri.` is enough. ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:211 - // Applies all the highlightings currently stored for a file with fileUri. - protected applyHighlights(fileUri: string) { + // Applies all the highlightings currently stored for a file with fileUri. If + public applyHighlights(fileUri: string) { ---------------- nit: the trailing `If`? ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:212 + // Applies all the highlightings currently stored for a file with fileUri. If + public applyHighlights(fileUri: string) { + if (!this.files.has(fileUri)) ---------------- could you move this method after `highlight`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66735/new/ https://reviews.llvm.org/D66735 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
