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