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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to