[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