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

curdeius added a comment. I agree with both of you. I shouldn't have used the word "regression" indeed. I just meant a change in behaviour. Sorry for that. I'll try to play around and propose a patch to enhance this feature :). If you have any pointers about how to check if everything fits on a

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

MyDeveloperDay added a comment. @oleg.smolsky I agree, what you have here covers a myriad of other cases and was committed in 2018, we can't call this commit a regression it was a feature ;-), if we want to improve the feature that is something else. Repository: rL LLVM CHANGES SINCE LAST

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

oleg.smolsky added a comment. @MyDeveloperDay that's the exact point. I authored this change to close a gap in some lambda formatting cases. The tests (existing, modified and added) express all relevant cases that I knew at the time. @jaafar, @curdeius - this is just C++ code. Nothing is set

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

MyDeveloperDay added a comment. I think if you have a proposal for changing the behavior lets see the patch and how it impacts the existing unit tests something tells me these tests are going to change? // A lambda passed as arg0 is always pushed to the next line.

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

curdeius added subscribers: MyDeveloperDay, curdeius. curdeius added a comment. Hi, I know it's an old revision, but I confirm that it provoked the bug https://bugs.llvm.org/show_bug.cgi?id=45141. The problem is in the `TokenAnnotator::mustBreakBefore` as @jaafar pointed out: if

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

jaafar added inline comments. Comment at: cfe/trunk/lib/Format/TokenAnnotator.cpp:3072 + return false; +if (!Next->isOneOf(TT_LambdaLSquare, tok::l_brace, tok::caret)) + return true; This is the clause that triggers the problem. Repository: rL

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

jaafar added a comment. It's been pointed out to me that 28546 preceded this commit... so it may only //appear// related. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52676/new/ https://reviews.llvm.org/D52676 ___

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

jaafar added a comment. Hi everyone, I've been investigating a bug and bisected it to this commit. The problem, in brief, is that lambdas passed as arguments can cause an extra line break before the first argument, but only if the lambda is neither the first nor the last argument. This

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL345753: [clang-format] tweaked another case of lambda formatting (authored by krasimir, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

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

krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Thank you! Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1282335, @krasimir wrote: > Looks good! Will stamp when the scopes are removed. Cool, thanks, Krasimir. I've just posted the updated patch. > Oleg, do you need someone to commit this for you? I do, could you commit this please?

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

oleg.smolsky updated this revision to Diff 171932. oleg.smolsky marked 2 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D52676 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp

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

krasimir added a comment. Looks good! Will stamp when the scopes are removed. Oleg, do you need someone to commit this for you? Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

oleg.smolsky marked 2 inline comments as done. oleg.smolsky added inline comments. Comment at: unittests/Format/FormatTest.cpp:11854 + // case above. + { +auto Style = getGoogleStyle(); djasper wrote: > No need to use a scope here. Feel free to redefine

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

djasper added a comment. I think this roughly looks fine. Krasimir, any thoughts? Comment at: unittests/Format/FormatTest.cpp:11854 + // case above. + { +auto Style = getGoogleStyle(); No need to use a scope here. Feel free to redefine Style. If in fact

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

oleg.smolsky added a comment. Folks, are there any other comments/suggestions? Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

oleg.smolsky updated this revision to Diff 170915. oleg.smolsky added a comment. Added a FIXME comment at Krasimir's request. Repository: rC Clang https://reviews.llvm.org/D52676 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp

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

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: > > >

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

krasimir added inline comments. Comment at: unittests/Format/FormatTest.cpp:11736 + // line and there are no further args. + verifyFormat("function(1, [this, that] {\n" + " //\n" oleg.smolsky wrote: > djasper wrote: > > oleg.smolsky wrote: > > >

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

oleg.smolsky updated this revision to Diff 170776. oleg.smolsky added a comment. Corrected test regressions, removed temporary hacks. Repository: rC Clang https://reviews.llvm.org/D52676 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp

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

oleg.smolsky added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:78 EXPECT_EQ(Expected.str(), format(Code, Style)); +#if 0 if (Style.Language == FormatStyle::LK_Cpp) { Oops., let me fix this... Repository: rC Clang

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

oleg.smolsky updated this revision to Diff 170773. oleg.smolsky added a comment. Corrected test regressions. Repository: rC Clang https://reviews.llvm.org/D52676 Files: clang/lib/Format/ContinuationIndenter.cpp clang/lib/Format/FormatToken.h clang/lib/Format/TokenAnnotator.cpp

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

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");

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

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"); djasper

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

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

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

oleg.smolsky updated this revision to Diff 170515. oleg.smolsky edited the summary of this revision. oleg.smolsky added a comment. Added another test case. Repository: rC Clang https://reviews.llvm.org/D52676 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h

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

oleg.smolsky added inline comments. Comment at: unittests/Format/FormatTest.cpp:11604 + " x.end(), //\n" + " [&](int, int) { return 1; });\n" "}\n"); krasimir wrote: > This looks a bit suspicious: I'd

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

krasimir added inline comments. Comment at: unittests/Format/FormatTest.cpp:11604 + " x.end(), //\n" + " [&](int, int) { return 1; });\n" "}\n"); This looks a bit suspicious: I'd expect a break before the

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

oleg.smolsky updated this revision to Diff 170270. oleg.smolsky added a comment. Generalized the patch so that it deals with lambda args irrespective of bin packing. Added additional tests and patched existing ones. Repository: rC Clang https://reviews.llvm.org/D52676 Files:

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

oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1268806, @djasper wrote: > In https://reviews.llvm.org/D52676#1268748, @oleg.smolsky wrote: > > > In https://reviews.llvm.org/D52676#1268706, @djasper wrote: > > > > > Ok, I think I agree with both of you to a certain extent, but I

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

djasper added a comment. In https://reviews.llvm.org/D52676#1268748, @oleg.smolsky wrote: > In https://reviews.llvm.org/D52676#1268706, @djasper wrote: > > > Ok, I think I agree with both of you to a certain extent, but I also think > > this change as is, is not yet right. > > > > First, it

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

oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1268706, @djasper wrote: > Ok, I think I agree with both of you to a certain extent, but I also think > this change as is, is not yet right. > > First, it does too much. The original very first example in this CL is > actually

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

djasper added a comment. Ok, I think I agree with both of you to a certain extent, but I also think this change as is, is not yet right. First, it does too much. The original very first example in this CL is actually not produced by clang-format (or if it is, I don't know with which flag

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

oleg.smolsky added a comment. @djasper @klimek could you chime in please? This patch strives to cleanup a quirk in lambda formatting which pushes code too far to the right. Here is the problematic case: void f() { something.something.something.Method(some_arg,

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

krasimir added a comment. In https://reviews.llvm.org/D52676#1251391, @oleg.smolsky wrote: > In https://reviews.llvm.org/D52676#1251342, @krasimir wrote: > > > Digging a bit further, seems like the behavior you're looking for could be > > achieved by setting the `AlignAfterOpenBracket` option

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

oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1251342, @krasimir wrote: > Digging a bit further, seems like the behavior you're looking for could be > achieved by setting the `AlignAfterOpenBracket` option to `DontAlign` or > `AlwaysBreak`: > > % clang-format

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

krasimir added a comment. Digging a bit further, seems like the behavior you're looking for could be achieved by setting the `AlignAfterOpenBracket` option to `DontAlign` or `AlwaysBreak`: % clang-format -style='{BasedOnStyle: google, AlignAfterOpenBracket: AlwaysBreak}' test.cc void f()

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

oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1250124, @oleg.smolsky wrote: > In https://reviews.llvm.org/D52676#1250071, @krasimir wrote: > > > In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote: > > > > > In https://reviews.llvm.org/D52676#1249784, @krasimir

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

krasimir added a comment. In https://reviews.llvm.org/D52676#1250124, @oleg.smolsky wrote: > In https://reviews.llvm.org/D52676#1250071, @krasimir wrote: > > > In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote: > > > > > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote:

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

oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1250071, @krasimir wrote: > In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote: > > > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote: > > > > > IMO `BinPackArguments==false` does not imply that there

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

oleg.smolsky updated this revision to Diff 167611. oleg.smolsky marked an inline comment as done. oleg.smolsky added a comment. Tweaked if/else/return structure, added comments. No functional changes. Repository: rC Clang https://reviews.llvm.org/D52676 Files:

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

krasimir added a comment. In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote: > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote: > > > IMO `BinPackArguments==false` does not imply that there should be a line > > break before the first arguments, only that there should

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

JonasToth added a comment. Please upload the patch with full context (i belive `diff -c 999`) Comment at: lib/Format/TokenAnnotator.cpp:3054 + return true; +else if (!Style.BinPackArguments) + return true; please no `else` after return. You