klimek added a comment.
I'm not generally opposed to this, given that
a) clang-format already changes code; I think by now we're not fixing double
semicolon mainly for workflow reasons, would be fine to add
b) the implementation is very self contained
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:1378
+**ConstStyle** (``ConstAlignmentStyle``)
+ Different ways to arrange const.
----------------
Personally, I'm somewhat against having 3 different aliases for the options.
I'd chose one, even though it doesn't make everybody happy, and move on. I'm
fine with East/West as long as the documentation makes it clear what it is.
================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:143
+ SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+ FormatTokenLexer &Tokens) {
+ const AdditionalKeywords &Keywords = Tokens.getKeywords();
----------------
This function is super large - can we split it up?
================
Comment at: clang/unittests/Format/FormatTest.cpp:30-32
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style)
\
+ verifyFormat(expect, actual, style, __LINE__)
----------------
I'm somewhat opposed to introducing these macros in this patch. If we want
them, we should create an extra patch, and figure out ho to use them in all
format tests.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69764/new/
https://reviews.llvm.org/D69764
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
