[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:198
 
-**AlignConsecutiveAssignments** (``bool``)
-  If ``true``, aligns consecutive assignments.
+**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``)
+  Style of aligning assignments on consecutive lines.
----------------
tinloaf wrote:
> MyDeveloperDay wrote:
> > As you plan to do the other cases won't this introduce
> > 
> > `AlignBitFieldStyle`
> > and
> > `AlignMacroStyle`
> > 
> > why not have 1  `AlignConsecutiveStyle` that can be used for all 3, then you don't need the other enum `TokenAlignmentStyle ` which means you won't need to map from one to the other?
> Yes, it would. I was under the assumption that every multiple-choice option in the options should have its own enum, for type safety. But yes, that would be way more elegant (and make the diff much smaller). I'll change it!
we don't make that distinction for those options that use true/false so I'm not sure why enums need to be any different

it may require some changes to the doc/tools/dump_style.py to ensure it gives meaningful documentation, but some consistency between the values might help a little (especially as we'd like to use the same AlignTokens algortihm)

You'd end up with 3 functions to map the various styles to you TokenAlignmentStyle anyway, and hence you'd lose the type safety

To be honest I get a bit annoyed at the use of "None" "Never" "false", "No" anyway, so I don't feel we are in a great place in terms of consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986