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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
