ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113 const semanticHighlightingFeature = - new semanticHighlighting.SemanticHighlightingFeature(); + new semanticHighlighting.SemanticHighlightingFeature( + getConfig<boolean>('semanticHighlighting')); ---------------- hokein wrote: > ilya-biryukov wrote: > > Why not avoid calling `clangdClient.registerFeature` instead? > > Would allow to: > > - not change the `SemanticHighlightingFeature` class, keeping it simpler, > > - ensure we do not do any extra work if the feature is disabled. > good point, done. Should we update other uses of `semanticHighlightingFeature` too? `context.subscriptions.push(vscode.Disposable.from(semanticHighlightingFeature))` probably ensures we call `dispose()` when the `clangdClient` is getting removed, I guess we definitely don't need that. Other uses as well: - no need to pass notification is highlighting is disabled. - no need to cleanup if highlighting is disabled. Maybe assign null or undefined to `semanticHighlightingFeature` when the flag is false? At each usage we can check whether the `semanticHighlightingFeature` is not null and only call relevant methods if that's the case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67096/new/ https://reviews.llvm.org/D67096 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits