================
Comment at: clang-tidy/ClangTidyOptions.cpp:102
@@ +101,3 @@
+
+const ClangTidyOptions &FileOptionsProvider::getOptions(StringRef FileName) {
+ DEBUG(llvm::dbgs() << "Getting options for file " << FileName << "...\n");
----------------
alexfh wrote:
> djasper wrote:
> > Can we pull out code shared with getStyle() in
> > include/clang/Format/Format.h? Seems bad to have this kind of logic in the
> > codebase twice.
> getStyle differs significantly from this one: it handles languages, default
> styles, unsuitable configurations etc. This could be abstracted away, but I
> have no confidence that this will simplify anything. I could try to do this
> separately from this patch, if you don't mind, so we don't break two tools
> simultaneously ;)
Absolutely, add a FIXME.
FWIW, it might makes sense to just have a function called
findParentFileWithName or something that only encapsulates the functionality of
walking up directories. Here, this will be sufficient. For clang-format that
can then be called in a loop if a certain file is found but unusable.
================
Comment at: clang-tidy/ClangTidyOptions.cpp:83
@@ +82,3 @@
+
+ if (Other.HeaderFilterRegex)
\
+ Result.HeaderFilterRegex = Other.HeaderFilterRegex;
----------------
I think you want to remove the escaped newline.
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:50
@@ +49,3 @@
+ "set of enabled checks.\n"
+ "This option's value is appended to the value read\n"
+ "from a .clang-tidy file, if any."),
----------------
alexfh wrote:
> djasper wrote:
> > I am not entirely certain that appending is the right thing to do here. But
> > I understand that otherwise, it would be next to impossible to say
> > "everything in the config file +/- this one check". Then again, with
> > -dump-config, it might be easy enough to copy and past the existing active
> > checks for a given file and modify them.
> This is done to keep the current behavior. If we want to change it, I'd
> rather do this as a separate step. And we also need strong reasons to do so,
> as it would degrade user experience, imo.
This can't be the current behavior as currently, there are no .clang-tidy
files. And that's also why I want us to think about it now, before we have to
break existing behavior. In your opinion, what are the pros/cons for/against
appending?
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:91
@@ +90,3 @@
+
+static cl::opt<bool> AnalyzeTemporaryDtors(
+ "analyze-temporary-dtors",
----------------
alexfh wrote:
> djasper wrote:
> > Hm. I am wondering whether it really makes sense to keep these individual
> > config flags. The approach in clang-format, where we can just pass in a
> > JSON config through a single flag seems like a good alternative.
> I'd keep at least the most frequently used options, e.g. -checks:
>
> clang-tidy -config='{Checks: "..."}'
>
> instead of
>
> clang-tidy -checks="..."
>
> is not extremely ugly, but a bit less convenient. Anyways, I'd like to make
> any unnecessary behavior changes in a separate patch.
Ok, but what are the more frequently used flags? Also, dumping the current
config into a .clang-tidy file and changing that seems like the more convenient
way to change these things anyway.
FWIW, I somewhat agree on keeping "-checks" (although I have no data on how
frequently it's used). However, I don't thing -analyze-temporary-dtors is or
should be a "frequent flag", which is why I left the comment here.
http://reviews.llvm.org/D5186
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
