[clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484)

[catskul]

Summary

I'm trying to get D14484: [clang-format] Formatting constructor initializer lists by putting them always on different lines past the finish line so I've adopted it.

This revision:

• fixes the unit tests
• resolves cases where the colon is attached (where the original did not)
• removes non-sensical 'backward compatibility'

[MyDeveloperDay]

I'm a little confused between BestFit and Compact

I'm not a massive fan of changing unit tests, just saying.

clang/docs/ClangFormatStyleOptions.rst

1473

We can't easily remove an option once its been released, I know you have code to handle it, but I think it needs to remain in the documentation and hence the Format.h and somehow be marked deprecated otherwise someone is going to look at the documentation and say, I could of sworn I used to be able to use this setting...

clang/lib/Format/Format.cpp

401

Nit:What pre-merge said!

clang/unittests/Format/FormatTest.cpp

4714

if you are setting this here to OnePerLine, then how come in the config if its true we set it to best fit?

5034

I personally think these tests should remain unaltered and it should just work?

[curdeius]

I agree with @MyDeveloperDay that the existing tests should not be altered.

clang/docs/ClangFormatStyleOptions.rst

1473

I agree that ConstructorInitializerAllOnOneLineOrOnePerLine should be kept for backwards compatibility and translated to the new option ConstructorInitializer: true -> BinPack (BestFit) and false -> Compact, as it is done for example with AlwaysBreakAfterDefinitionReturnType.

1481–1483

Shouldn't that be called something like BinPack to match existing options (BinPackArguments, BinPackParameters)?

1489–1500

Please modify examples.

[nhlubek]

I tried to highlight the story with this bug that this commit tries to fix in the following bug report:
https://bugs.llvm.org/show_bug.cgi?id=50549

It goes back to 2015 and several people have tried to come up with solutions. The solutions are not complicated but due to the complex nature of C++ formatting and the clang-format options everyone so far has failed a code review and doesn't really know how to proceed.
I think this can only be resolved by a clang-format core dev who needs to adopt the changes in a form suitable for clang-format. A small task that will make a lot of people happy.

[MyDeveloperDay]

A small task that will make a lot of people happy

Its not the happy people that concern me, its those who end up not happy that cause us the most pain ;-)

1. I question if the rst was generated via docs/tools/dump_format_style.py or by hand, it has to be generated so it stay in lock step.
2. We need to keep the old style and just make it initialise your new style if present, the easiest way of doing that is take the old style to be an enum, have the parser handle true/false, then provide a mapping of the name "if you cannot stomach the old option name"
   • people use the latest version doc to look things up, they don't always find the doc for their version
   • we cannot have the 100,000 of uses of the old option cause a runtime failure or cause people to be confused because they cannot find it in the docs (we have to give them a reference even it its a "use this now")
   • we can add a new style and put a big note on the old name to say its been deprecated

3. Fundamentally this patch looks ok, I would like to see more cases where the : wasn't always on a new line
4. I'd like to know how this is impacted by the new "AfterComma" style

[owenpan]

See D108752.