[PATCH] D119599: Add option to align compound assignments like `+=`
MyDeveloperDay via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 12 01:55:26 PST 2022
MyDeveloperDay added inline comments.
================
Comment at: clang/include/clang/Format/Format.h:163
+ /// \endcode
+ bool AlignCompoundAssignments;
+
----------------
This option is not independent of `AlignConsecutiveAssignments` is it? this will cause confusion when people turn it on without turning on `AlignConsecutiveAssignments`
Options have a lifecycle we have noticed
1) They start as bool
2) They become enums
3) They then end up as a struct of bool
4) Sometimes the struct becomes of enums
Whilst I like what you are doing here I fear we will get bugs where people say I set AlignCompoundAssignments: true but its doesn't work.
`AlignConsecutiveAssignments` is already gone too far on the enum, it should be a struct
so rather than
```
enum AlignConsecutiveStyle {
ACS_None,
ACS_Consecutive,
ACS_AcrossEmptyLines,
ACS_AcrossComments,
ACS_AcrossEmptyLinesAndComments
};
AlignConsecutiveStyle AlignConsecutiveAssignments ;
```
it should be perhaps
```
struct {
bool AcrossEmptyLines,
bool AcrossComments,
bool AlignCompound
} AlignConsecutiveStyle;
AlignConsecutiveStyle AlignConsecutiveAssignments;
```
in the .clang-format file it would become
```
AlignConsecutiveAssignments: Custom
AlignConsecutiveAssignmentsOptions:
AcrossEmptyLines: true
AcrossComments: true
AlignCompound: false
```
I realise this would be a much bigger piece of work (in the config) but the existing options would map to the new options, and then we have a structure for which have space for future expansion.
The introduction of a dependent option in my view triggers the need for that config change? @HazardyKnusperkeks
you thoughts, I know we've done this before, what do you think in this case?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119599/new/
https://reviews.llvm.org/D119599
More information about the cfe-commits
mailing list