sammccall added a comment.
Thanks! Just some organization nits.
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:26
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
#include "CompileCommands.h"
----------------
This is a pretty weird place to depend on clang-tidy.
Can we move the "is this a clang-tidy check name" function to somewhere more
clang-tidy related, like `TidyProvider.h` or even
`clang-tidy/ClangTidyModuleRegistry.h`? ("isRegisteredCheck")
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:355
+ // tidy check, most likely due to a typo.
+ static bool isInvalidCheckGlob(StringRef CheckGlob) {
+ // Any wildcards just assume they aren't invalid
----------------
naming nits:
- prefer positive over negative names
- "glob" is confusing here as we don't actually handle globs
- outside of the clang-tidy namespace, "check" needs a "tidy" qualifier
What about `isTidyCheckName`, and move the `*` test into the caller? Then the
semantics of the function are really clear without having to read the body or
comment.
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:386
+ llvm::formatv(
+ "Check glob '{0}' doesn't specify any known clang-tidy check",
+ Str)
----------------
message could be clearer - "clang-tidy check '{0}' was not found"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92874/new/
https://reviews.llvm.org/D92874
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
