[PATCH] D105099: [clang-format] Add an option to put one constructor initializer per line

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 30 08:41:06 PDT 2021


curdeius added a comment.

In D105099#2848583 <https://reviews.llvm.org/D105099#2848583>, @owenpan wrote:

> If we were to start this all over without the need of backward compatibility and the existence of the related unit tests, an enum might be a better option. Then I still think the user might have some trouble relating the following to the enum.
>
>   If AlwaysOnePerLine:
>     Put each on its own line.
>   Else If AllOnOneLineOrOnePerline:
>     If they all fit on one line:
>       Put all of them on a single line.
>     Else If AllOnNextLine:
>       Put (the rest of) them on the next line.
>     Else:
>       Put each on its own line.
>   Else:
>     ...

Is there anything else in the "Else:" part above? Is there an option that we forgot?

I'm not sure if I understand you correctly. Is your point that an enum option would be hard to find by the users?
But if the doc of each currently existing option pointed to this enum option it would be pretty easy, no?
Or does backward compatibility seem difficult to achieve? It should be a straightforward mapping.

Also, I find that having option that are mutually exclusive (or one that doesn't change anything when another is enabled) is a smell that tells us to use an enum.
Personally I find it harder to finder multiple options that concern the same aspect of the code style rather than a single option.

What I find problematic is clear and descriptive names for all options and possible values, but that's unrelated to whether it's a single enum option or several boolean options.
Having said that, I need to admit that `ConstructorInitializerAllOnOneLineOrOnePerLine` name makes my blood run cold :).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105099



More information about the cfe-commits mailing list