[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