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