[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 30 06:56:59 PST 2020


njames93 added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:2962
+                                    llvm::sys::path::filename(FileName));
+            if (auto OuterStyle = getStyle(DefaultFormatStyle, FileForLanguage,
+                                           FallbackStyleName, Code, FS,
----------------
HazardyKnusperkeks wrote:
> njames93 wrote:
> > This seems a very inefficient way of doing this, getStyle does a lot of things we simple don't need to worry about. Maybe the file loading part should be extracted out into its own function, that can then recursively call itself. Or use the clang-tidy approach of storing configurations in a stack (will have to be unparsed cause how clang-format works) and then parse them on top of each other.
> Yeah I know that this is double work, but I thought it only affects the case `BasedOnStyle: File` and keeps the "normal" way untouched.
> 
> If I do build a stack I would actually parse all `.clang-format` files up to the root directory and in the not `BasedOnStyle: File` case throw away the results, wouldn't I? The question is how often someone has a `.clang-fomrat` in one of his parent directories which is not used because it got overwritten.
> 
> But I can do that.
> 
> Or is there an easy way to just check the `BasedOnStyle` value? As far as I understood the yaml code it parses the whole file into an internal structure and we only query this structure, right?
> Yeah I know that this is double work, but I thought it only affects the case `BasedOnStyle: File` and keeps the "normal" way untouched.
> 
> If I do build a stack I would actually parse all `.clang-format` files up to the root directory and in the not `BasedOnStyle: File` case throw away the results, wouldn't I? The question is how often someone has a `.clang-fomrat` in one of his parent directories which is not used because it got overwritten.

I'm not saying start at the route directory and work down to the child. Still start at the child, but when you parse a configuration that has needs the parent, put that unparsed config into a stack, then carry on. At the end then you can join all those configurations
> 
> But I can do that.
> 
> Or is there an easy way to just check the `BasedOnStyle` value? As far as I understood the yaml code it parses the whole file into an internal structure and we only query this structure, right?
It should be possible, not an expert but I'm not sure it like being traversed multiple times normally, though a workaround should be feasible. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93844



More information about the cfe-commits mailing list