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