[PATCH] D73354: clang-format: insert trailing commas into containers.

[Martin Probst via Phabricator via cfe-commits]

mprobst marked 6 inline comments as done.
mprobst added a comment.

PTAL.



================
Comment at: clang/include/clang/Format/Format.h:552
+    /// Insert trailing commas in container literals that were wrapped over
+    /// multiple lines.
+    TCS_Wrapped,
----------------
mprobst wrote:
> sammccall wrote:
> > Hadn't occurred to me that this will interact with bin-packing. That seems to increase the chances of going over the column limit :(
> Uh. Turns out we don't have a separate option for bin packing containers, only for bin packing arguments and parameters.
> 
> Should we add that option to make this setting work?
> Should we have this setting imply bin packing containers (as opposed to parameters) is disabled?
OK, so I added a bit of validation logic to reject bin packing arguments together with TSC_Wrapped. We can figure out in a next step whether we want to expose an option for bin packing containers, or whether we can just disable bin packing for those people who want trailing commas.


================
Comment at: clang/unittests/Format/FormatTestJS.cpp:1229
                "      (param): param is {\n"
-               "        a: SomeType\n"
+               "        a: SomeType;\n"
                "      }&ABC => 1)\n"
----------------
MyDeveloperDay wrote:
> mprobst wrote:
> > MyDeveloperDay wrote:
> > > is this correct?
> > Yes, type definitions should use `;`
> if you hadn't added this to the test would it have added a "," ?
More precisely:
you can  use either `;` or `,` to separate entries in object literal types. So using `;` is correct here, but c-f with this option would insert a `,`, which is correct as well. I've added a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73354