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

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky marked 2 inline comments as done.
oleg.smolsky added inline comments.


================
Comment at: unittests/Format/FormatTest.cpp:11604
+               "      x.end(),   //\n"
+               "      [&](int, int) { return 1; });\n"
                "}\n");
----------------
oleg.smolsky wrote:
> djasper wrote:
> > 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.
> > If you prefer less indentation, great. Set AlignAfterOpenBracket to "AlwaysBreak" and we don't need any of this ;-) (as Krasimir has suggested before).
> 
> Well, `AlignAfterOpenBracket` is too big a gun as we talking about lambda function args here.
> 
> > 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.
> 
> OK, that's fair. Let me try to figure out how to get the former behavior restored for this case.
OK, I've just implemented heuristics for counting the introducer's length, but it turns out that it's not needed. I just had a bug in one of the conditions... So, I've added a couple more tests and will try posting a patch against the new monorepo momentarily...


Repository:
  rC Clang

https://reviews.llvm.org/D52676