This allows the define BasedOnStyle: InheritParentConfig and then clang-format looks into the parent directories for their .clang-format and takes that as a basis.
My use case is to remove the column limit for my tests.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
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?
Comment Actions
Yes of course I can, but I don't want to copy everything except the one thing I want to change.
I have one .clang-format in my main directory and since I only want to change the column limit for my tests I want to write a .clang-format in the test directory with the content:
BasedOnStyle: File ColumnLimit: 0
So when I change (or better add) a value in my main .clang-format it will also apply to my tests, without touching their file.
Comment Actions
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.
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?
| clang/lib/Format/Format.cpp | ||
|---|---|---|
| 2980 | 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. | |
Comment Actions
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.
| clang/lib/Format/Format.cpp | ||
|---|---|---|
| 2980 | 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? | |
Comment Actions
I like what you are suggesting, do you think BasedOnStyle: File is the best terminology as the term File is used elsewhere to mean read from the .clang_format file, how about
BasedOnStyle: Inherit
BasedOnStyle: InheritFromParent
BasedOnStyle: InheritFromFile
or why not just
BasedOnStyle: InheritParentConfig
to match clang-tidy?
| clang/lib/Format/Format.cpp | ||
|---|---|---|
| 2980 |
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
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. | |
Comment Actions
- Rebased
- Renamed to InhteritParentConfig
- Reworked to remove the recursion of getStyle
- Now supports the command line style options
- Test cases extended for the latter
Comment Actions
What I can't easily tell from the tests is if you are overriding any styles defined in the parent with a local style.
| clang/include/clang/Format/Format.h | ||
|---|---|---|
| 57 | I don't quite understand this last sentence | |
Comment Actions
Yes I do, but I will try to make it clearer in the tests.
| clang/include/clang/Format/Format.h | ||
|---|---|---|
| 57 | If I would use /// it would end up in the ClangFormatStyleOptions.rst. I only want to make clear it is no mistake that it only got // while all other members are documented with ///. | |
Comment Actions
Okay after rechecking the code (it is a bit ago that I wrote it), what more do you need? I compare the parsed style against a base style (Google, to show that it’s not the default LLVM) modified with the values from the overwrite. Only for the last checks where I basically check the same over and over again, just trough different means I use a stored style.