[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

I like the idea, but we need more tests.



================
Comment at: clang/include/clang/Format/Format.h:93
 
+  /// if ``true``, when using static initialization for an array of structs
+  /// aligns the fields into columns
----------------
Why static?


================
Comment at: clang/include/clang/Format/Format.h:98-100
+  ///       {56, 23,        "hello" },
+  ///       {-1, 93463,     "world" },
+  ///       {7,  5,         "!!"    }
----------------
Why not like this?
Why all these spaces?


================
Comment at: clang/include/clang/Format/Format.h:103
+  /// \endcode
+  bool AlignArrayOfStructuresInit;
+
----------------
Maybe simplify the name. Should it really apply only to initialization, why not assignment?
Also, don't use "Init" but "Initialization" if need be.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16371
+               Style);
+}
+
----------------
I think we need more tests:
* with a comma after the last element of array
* with assignment and not only initialization?
* without `struct` keyword
* with short lines/long values to see how wrapping behaves
  * also, with multiple string literals in one struct, e.g. `"hello" "world"` (that will get splitted into multiple lines)
* with non-plain-array types (i.e. missing `[]`/`[3]`)
* interaction with other Align* options (e.g. `AlignConsecutiveAssignments`, two arrays one after another, `AlignConsecutiveDeclarations`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868