Hiralo marked an inline comment as done.
Hiralo added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:68
+  /// Clang-tidy-config
+  llvm::Optional<std::string> ClangTidyConfig;
+
----------------
DmitryPolukhin wrote:
> I'm not sure that we need it here. I would reuse code path for `--config` 
> options as much as possible and implement new option as a simple wrapper that 
> reads content of the file and interpret it as `--config` option. Moreover I 
> think it should not be possible to specify both options in command line.
> it should not be possible to specify both options in command line.

Made local changes with...

$ ./bin/clang-tidy -config="{Checks: '*'}" --config-file=/some/path/config --
Error: --config-file and --config are mutually exclusive. Specify only one.

$ ./bin/clang-tidy --checks='*' --config-file=/some/path/config --
Error: --config-file and --checks are mutually exclusive. Specify only one.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:233
   llvm::Optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
+  llvm::Optional<OptionsSource> tryReadConfigFile(llvm::StringRef Path,
+                                                  bool IsFile);
----------------
DmitryPolukhin wrote:
> It looks like the second argument was added only for overload resolution. But 
> I think it is better to rename this function. After all it is not "try" 
> anymore because it reports fatal error in case of errors.
Renamed function to 'readConfigFile'.


================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:76
 
+static cl::opt<std::string> ClangTidyConfig("clang-tidy-config", cl::desc(R"(
+Specify full path of .clang-tidy config file.
----------------
DmitryPolukhin wrote:
> I would call it something like `--config-file`
renamed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89936/new/

https://reviews.llvm.org/D89936

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to