curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.
LGTM. That's a great piece work @feg208. Thank you!
I've added many nit comments, but I didn't do it for all code comments.
Please check that all comments are full phrases (with full stops :) ) before
landing.
Some comments are in .rst but you know that you need to update Format.h and
then regenerate .rst :).
Also, don't hesitate to mark comments as done.
Do you need somebody to land it on your behalf?
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:213-218
+ struct test demo[] =
+ {
+ {56, 23, "hello"},
+ {-1, 93463, "world"},
+ { 7, 5, "!!"}
+ }
----------------
This repetition of `demo` struct seems not very useful.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:247
+ * ``AIAS_None`` (in configuration: ``None``)
+ Don't align array initializer columns
+
----------------
Nit. This can be done when landing. No need to update the patch just for this.
================
Comment at: clang/include/clang/Format/Format.h:93
+ /// Different style for aligning array initializers
+ enum ArrayInitializerAlignmentStyle {
----------------
================
Comment at: clang/lib/Format/FormatToken.h:434-442
+ /// The first token in set of column elements
+ bool StartsColumn = false;
+
+ /// This notes the start of the line of an array initializer
+ bool ArrayInitializerLineStart = false;
+
+ /// This starts an array initializer
----------------
Nits.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:994
+
+ // Now go through and fixup the spaces
+ auto *CellIter = Cells.begin();
----------------
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:1005
+ // on a line that was split. If so on that line we make sure that
+ // the spaces in front of the brace are enough
+ Changes[CellIter->Index].NewlinesBefore = 0;
----------------
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:1013-1014
+ }
+ // Except if the array is empty we need the position of all the
+ // cells immediantly adjacent to this
+ if (CellIter != Cells.begin()) {
----------------
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:1062
+
+ // Now go through and fixup the spaces
+ auto *CellIter = Cells.begin();
----------------
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:1064
+ auto *CellIter = Cells.begin();
+ // The first cell needs to be against the left brace
+ if (Changes[CellIter->Index].NewlinesBefore == 0)
----------------
================
Comment at: clang/lib/Format/WhitespaceManager.h:185-190
+ constexpr bool operator==(const CellDescription &oth) const {
+ return Index == oth.Index && Cell == oth.Cell && EndIndex ==
oth.EndIndex;
+ }
+ constexpr bool operator!=(const CellDescription &oth) const {
+ return !(*this == oth);
+ }
----------------
Nit.
================
Comment at: clang/lib/Format/WhitespaceManager.h:265
+ // If we broke the line the initial spaces are already
+ // accounted for
+ if (Changes[PrevIter->Index].NewlinesBefore > 0)
----------------
Nit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101868/new/
https://reviews.llvm.org/D101868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
