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

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

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

No, it's just what you called "one other enum value for the default case without special handling."

> 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?

An enum makes it easier for the user to find all related options, but my point is that in this case it would be harder for the user to relate the enum names to the behaviors because of the nested `If`.

> Or does backward compatibility seem difficult to achieve? It should be a straightforward mapping.

Part of backward compatibility is not breaking/changing existing unit tests, which may be difficult to do.

> 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.

You are not suggesting using two enums, one for the outer `If` and the other for the nested `If`, right?

If we didn't have `AllOnOneLineOrOnePerLine`, we could simply use an enum for `ConstructorInitializer` with three values:

- `OnePerLine` (i.e., `AlwaysOnePerLine = true`)
- `BinPack` (i.e., `AllOnOneLineOrOnePerLine = OnNextLine = true`)
- `Neither` (i.e., `AlwaysOnePerLine = AllOnOneLineOrOnePerLine = false`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105099