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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D93844#2472352 <https://reviews.llvm.org/D93844#2472352>, @njames93 wrote:

> In D93844#2472346 <https://reviews.llvm.org/D93844#2472346>, @MyDeveloperDay wrote:
>
>> I'm a little confused why this is needed as clang-format always read up the directory tree until it see a .clang-format file, perhaps I don't quite understand the use case. Can't you just have a different .clang-format in the subdirectory?
>
> It's my understanding this is akin to the `InheritParentConfig` option found in `.clang-tidy`.
> The idea is you can put a `.clang-format` file in your project root, then for specific directories you can add another `.clang-format` file that modifies that root `.clang-format`
> The example here is a test folder that needs extended lines but want the rest of the formatting to match the root folder without having to copy the contents of the root and make the small edit.
>
> We have it quite lucky here at LLVM where for tests we can just use:
>
>   BasedOnStyle: LLVM
>   ColumnLimit: 0
>
> However for other projects with their own style they could then use:
>
>   # Copy the root directory clang.format config
>   BasedOnStyle: file
>   ColumnLimit: 0
>
> This would also make modifying the formatting style of a project much easier as you would only need to touch the root directory `.clang-format` file in most cases.

Exactly.

> With this implementation I'm not too sure on the specifics though. `.clang-format` files are more complex to parse than `.clang-tidy` files so there's more edge cases to consider.
> Also it appears this wouldn't handle the case of invoking clang-format with the style set inline on the command line.
> `clang-format <file> --style='{BasedOnStyle: File, ColumnLimit: 80}'`
> Should this then check the directory for `.clang-format` files?

Initially I thought it shouldn't. But now I think maybe it should.



================
Comment at: clang/lib/Format/Format.cpp:2962
+                                    llvm::sys::path::filename(FileName));
+            if (auto OuterStyle = getStyle(DefaultFormatStyle, FileForLanguage,
+                                           FallbackStyleName, Code, FS,
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93844