[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added inline comments.


================
Comment at: unittests/Format/FormatTest.cpp:11736
+  // line and there are no further args.
+  verifyFormat("function(1, [this, that] {\n"
+               "  //\n"
----------------
krasimir wrote:
> oleg.smolsky wrote:
> > djasper wrote:
> > > oleg.smolsky wrote:
> > > > krasimir wrote:
> > > > > Could we please have a test case where there are several args packed on the first line, then a line break, then an arg, then a multiline lambda as a last arg (illustrating that we don't pull the first arg down if there's only a multiline lambda as the last arg):
> > > > > ```
> > > > > function(a, b, ccccccc,
> > > > >          d, [] () {
> > > > >   body
> > > > > });
> > > > > ```
> > > > Sure, that seems to work, but not in the way you expected :) I'll update the patch...
> > > > 
> > > > ```
> > > >   verifyFormat("function(a, b, c, //\n"
> > > >                "         d, [this, that] {\n"
> > > >                "           //\n"
> > > >                "         });\n");
> > > > ```
> > > We should try to prevent that (unless it's also the current behavior of course). People have filed various bugs about this before and it is not generally an accepted formatting.
> > This behavior is consistent with 5.0 and 6.0, so we are OK.
> Maybe add a FIXME for that test that this is not ideal formatting and we should also be pulling the first arg on a newline in that case too in the future then.
Sure, added a comment:

```
  // FIXME: this format is not ideal and we should consider forcing the first arg
  // onto its own line.
  verifyFormat("function(a, b, c, //\n"
               "         d, [this, that] {\n"
               "           //\n"
               "         });\n");
```


Repository:
  rC Clang

https://reviews.llvm.org/D52676