Hiralo added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:320-324
+ if (!Checks.empty()) {
+ llvm::errs() << "Error: --config-file and --checks are mutually "
+ "exclusive. Specify only one.\n";
+ return nullptr;
+ }
----------------
DmitryPolukhin wrote:
> DmitryPolukhin wrote:
> > njames93 wrote:
> > > I disagree with this check here, `Config` is not mutually exclusive with
> > > `Checks`, `Checks` gets applied atop of `Config`. So the same should
> > > happen when using `ConfigFile` with `Checks`
> > +1
> Clarify: +1 to @njames93 that we don't need this check.
> I disagree with this check here, `Config` is not mutually exclusive with
> `Checks`, `Checks` gets applied atop of `Config`. So the same should happen
> when using `ConfigFile` with `Checks`
ok, will remove these two checks :)
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:337
+
+ Config.assign((*Text)->getBuffer());
+ }
----------------
Hiralo wrote:
> DmitryPolukhin wrote:
> > DmitryPolukhin wrote:
> > > I suggest creating new local variable with text of the config from
> > > `Config` or `ConfigFile` file content i.e. avoid modifying `Config`
> > > itself.
> > It doesn't compile and using local variable instead of changing command
> > line option will make this code easier to read and understand:
> > ```
> > clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:334:12: error: no
> > matching member function for call to 'assign'
> > Config.assign((*Text)->getBuffer());
> > ~~~~~~~^~~~~~
> > ```
> > Could you please fix build and please upload diff with `arc diff` as
> > @njames93 suggested so buildbot can test your changes?
> > I suggest creating new local variable with text of the config from `Config`
> > or `ConfigFile` file content i.e. avoid modifying `Config` itself.
>
> Sorry! probably I missed this comment!
>
> std::string Temp = (*Text)->getBuffer();
> Config.assign(Temp); // ???
>
> if so how code will enter into below block... because we want to reuse
> code-path of 'Config' correct?
>
> if (!Config.empty()) {
> if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
> parseConfiguration(Config)) {
> ...
> }
>
> ?
> It doesn't compile and using local variable instead of changing command line
> option will make this code easier to read and understand:
> ```
> clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:334:12: error: no
> matching member function for call to 'assign'
> Config.assign((*Text)->getBuffer());
> ~~~~~~~^~~~~~
> ```
> Could you please fix build and please upload diff with `arc diff` as
> @njames93 suggested so buildbot can test your changes?
Let me try.
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
