djasper added inline comments.
================
Comment at: unittests/Format/FormatTest.cpp:11604
+ " x.end(), //\n"
+ " [&](int, int) { return 1; });\n"
"}\n");
----------------
oleg.smolsky wrote:
> krasimir wrote:
> > This looks a bit suspicious: I'd expect a break before the first arg to be
> > forced only when there exists a multiline (after formatting) lambda
> > expression arg. Is this (multiline vs. lambdas fitting 1 line) something
> > that we (can) differentiate with respect to? djasper@ might have an insight
> > on this.
> Well, yes, I can see where you are coming from - the lambda is short and
> would fit. Unfortunately, I am not sure how to implement this nuance... I
> think I'd need to get the length of the unwrapped line and then check whether
> it fits in TokenUnnotator.cc....
>
> Also, I personally favor less indentation (i.e. full width for the lambda) as
> that prevents drastic reformat when the lambda body changes. (that's why this
> patch exists)
I agree with Krasimir here.
If you prefer less indentation, great. Set AlignAfterOpenBracket to
"AlwaysBreak" and we don't need any of this ;-) (as Krasimir has suggested
before).
In more seriousness, I think getting all these cases right, I appreciate that
it is difficult. However, I also like to make sure that we do get them right.
Changing clang-format's behavior for any of these cases is not a small thing,
it will cause churn for *a lot* of people. We should try really hard to not
have changes in there that people will find detrimental. This of course is
subjective, so we won't get to 100%, but if in doubt for specific cases, let's
err on the side of not changing the current behavior.
================
Comment at: unittests/Format/FormatTest.cpp:11736
+ // line and there are no further args.
+ verifyFormat("function(1, [this, that] {\n"
+ " //\n"
----------------
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.
Repository:
rC Clang
https://reviews.llvm.org/D52676
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
