sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/clangd/Compiler.h:64 +/// Clears \p CI from options that are not supported by clangd, like codegen or +/// plugins. ---------------- Comment is good, but call this disableUnsupportedOptions? (We're in namespace clangd already) ================ Comment at: clang-tools-extra/clangd/Compiler.h:64 +/// Clears \p CI from options that are not supported by clangd, like codegen or +/// plugins. ---------------- sammccall wrote: > Comment is good, but call this disableUnsupportedOptions? > (We're in namespace clangd already) This should also probably have a comment about being paired with CommandMangler, as that provides similar functionality that just happens to be best expressed on the command instead (e.g. stripping --save-temps) ================ Comment at: clang-tools-extra/clangd/test/indexer.test:3 +# RUN: touch %t.cpp +# RUN: clangd-indexer %t.cpp -- -Xclang -verify --save-temps -- 2>&1 | FileCheck %s +# CHECK-NOT: error: no expected directives found: consider use of 'expected-no-diagnostics' ---------------- Add a comment (or rename test file) saying what functionality this is trying to test? it's not obvious Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106669/new/ https://reviews.llvm.org/D106669 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
