[PATCH] D45719: [clang-Format] Fix indentation of member call after block
This revision was automatically updated to reflect the committed changes. Closed by commit rC342363: [clang-Format] Fix indentation of member call after block (authored by ibiryukov, committed by ). Repository: rC Clang https://reviews.llvm.org/D45719 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h unittests/Format/FormatTest.cpp Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -403,7 +403,9 @@ // }.bind(...)); // FIXME: We should find a more generic solution to this problem. !(State.Column <= NewLineColumn && -Style.Language == FormatStyle::LK_JavaScript)) +Style.Language == FormatStyle::LK_JavaScript) && + !(Previous.closesScopeAfterBlock() && +State.Column <= NewLineColumn)) return true; // If the template declaration spans multiple lines, force wrap before the Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -325,6 +325,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if (BlockKind == BK_Block) + return true; +if (closesScope()) + return Previous->closesScopeAfterBlock(); +return false; + } + /// \c true if this token starts a sequence with the given tokens in order, /// following the ``Next`` pointers, ignoring comments. template Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4557,6 +4557,40 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " (\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .foo();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo(\"a\"\n" + " \"\");\n" + "}", + getLLVMStyleWithColumns(30)); } TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -403,7 +403,9 @@ // }.bind(...)); // FIXME: We should find a more generic solution to this problem. !(State.Column <= NewLineColumn && -Style.Language == FormatStyle::LK_JavaScript)) +Style.Language == FormatStyle::LK_JavaScript) && + !(Previous.closesScopeAfterBlock() && +State.Column <= NewLineColumn)) return true; // If the template declaration spans multiple lines, force wrap before the Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -325,6 +325,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if (BlockKind == BK_Block) + return true; +if (closesScope()) + return Previous->closesScopeAfterBlock(); +return false; + } + /// \c true if this token starts a sequence with the given tokens in order, /// following the ``Next`` pointers, ignoring comments. template Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4557,6 +4557,40 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" +
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ilya-biryukov added a comment. Landed the patch as r342363. Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. ping @klimek Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. ping Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. Ping! it would be awesome to get some help getting this merged since I do not have merge rights Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank updated this revision to Diff 158737. ank added a comment. fix space in if ( Repository: rC Clang https://reviews.llvm.org/D45719 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4416,6 +4416,40 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " (\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .foo();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo(\"a\"\n" + " \"\");\n" + "}", + getLLVMStyleWithColumns(30)); } TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -325,6 +325,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if (BlockKind == BK_Block) + return true; +if (closesScope()) + return Previous->closesScopeAfterBlock(); +return false; + } + /// \c true if this token starts a sequence with the given tokens in order, /// following the ``Next`` pointers, ignoring comments. template Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -403,7 +403,9 @@ // }.bind(...)); // FIXME: We should find a more generic solution to this problem. !(State.Column <= NewLineColumn && -Style.Language == FormatStyle::LK_JavaScript)) +Style.Language == FormatStyle::LK_JavaScript) && + !(Previous.closesScopeAfterBlock() && +State.Column <= NewLineColumn)) return true; // If the template declaration spans multiple lines, force wrap before the Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4416,6 +4416,40 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " (\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .foo();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo(\"a\"\n" + " \"\");\n" + "}", + getLLVMStyleWithColumns(30)); } TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -325,6 +325,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if (BlockKind == BK_Block) + return true; +if (closesScope()) +
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
danilaml added inline comments. Comment at: lib/Format/FormatToken.h:323 + bool closesScopeAfterBlock() const { +if(BlockKind == BK_Block) + return true; Looks like all other `if`s in this file have space before `(`. Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
klimek added a comment. Sorry that I lost track of that, but feel free to ping once / week - unfortunately the patch doesn't apply cleanly, can you rebase? Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. Herald added a subscriber: acoomans. Ping! it would be awesome to get some help getting this merged since I do not have merge rights Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. awesome, I do not have merge rights so help with merging this would be greatly appreciated Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank updated this revision to Diff 152870. Repository: rC Clang https://reviews.llvm.org/D45719 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4416,6 +4416,40 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " (\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .foo();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo(\"a\"\n" + " \"\");\n" + "}", + getLLVMStyleWithColumns(30)); } TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -319,6 +319,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if(BlockKind == BK_Block) + return true; +if(closesScope()) + return Previous->closesScopeAfterBlock(); +return false; + } + /// \c true if this token starts a sequence with the given tokens in order, /// following the ``Next`` pointers, ignoring comments. template Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -399,7 +399,9 @@ // }.bind(...)); // FIXME: We should find a more generic solution to this problem. !(State.Column <= NewLineColumn && -Style.Language == FormatStyle::LK_JavaScript)) +Style.Language == FormatStyle::LK_JavaScript) && + !(Previous.closesScopeAfterBlock() && +State.Column <= NewLineColumn)) return true; // If the template declaration spans multiple lines, force wrap before the Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4416,6 +4416,40 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " (\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .foo();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo(\"a\"\n" + " \"\");\n" + "}", + getLLVMStyleWithColumns(30)); } TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -319,6 +319,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if(BlockKind == BK_Block) + return true; +if(closesScope()) + return Previous->closesScopeAfterBlock();
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added inline comments. Comment at: unittests/Format/FormatTest.cpp:4449-4450 + " })\n" + " .foo(\"aaa\"\n" + " \"bb\");\n" + "}"); klimek wrote: > One more nit: I'd use a Style with a smaller column limit for these tests, > makes them a lot more readable :) makes sense, I'll do that. Comment at: unittests/Format/FormatTest.cpp:4452 + "}"); + + verifyFormat("test() {\n" klimek wrote: > Are the ones below here testing anything that's not tested above? nope I'll remove them! Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4449-4450 + " })\n" + " .foo(\"aaa\"\n" + " \"bb\");\n" + "}"); One more nit: I'd use a Style with a smaller column limit for these tests, makes them a lot more readable :) Comment at: unittests/Format/FormatTest.cpp:4452 + "}"); + + verifyFormat("test() {\n" Are the ones below here testing anything that's not tested above? Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); klimek wrote: > ank wrote: > > klimek wrote: > > > ank wrote: > > > > klimek wrote: > > > > > What would be interesting is tests that: > > > > > a) have another value after the closing }; doesn't really make sense > > > > > in this test, but usually these are in calls > > > > > f([]() { ... }, foo, bar).call(...) > > > > > > > > > > b) make .as("") have paramters that go beyond the limit > > > > > > > > > > c) add another chained call behind .as(""). > > > > Hello, sorry for the late follow up on this! > > > > > > > > Indeed an interesting thing to test, and so I did, the patterns you > > > > describe seems to give strange indentation as far back as release_36 > > > > and probably has always done so. The case I'm testing here worked in > > > > release_40 but stopped working somewhere before release_50 > > > > > > > > maybe fixing those other cases can be done in another patch > > > > > > > > cheers, > > > > Anders > > > I meant: please add these tests :) > > I need to clarify, those tests will not be correctly indented by this > > commit, I could add those tests but then they would verify a faulty > > behaviour, this is how they will be indented even with this patch applied: > > > > > > ``` > > // Dont break if only closing statements before member call > > verifyFormat("test() {\n" > > " ([]() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " }).foo();\n" > > "}"); > > verifyFormat("test() {\n" > > " (\n" > > " []() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " },\n" > > " foo, bar)\n" > > " .foo();\n" > > "}"); > > verifyFormat("test() {\n" > > " ([]() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " })\n" > > " .foo()\n" > > " .bar();\n" > > "}"); > > verifyFormat("test() {\n" > > " ([]() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " })\n" > > " > > .foo(\"aaa\"\n" > > " \"bb\");\n" > > "}"); > > > > verifyFormat("test() {\n" > > " foo([]() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " }).foo();\n" > > "}"); > > verifyFormat("test() {\n" > > " foo(\n" > > " []() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " },\n" > > " foo, bar)\n" > > " .as();\n" > > "}"); > > verifyFormat("test() {\n" > > " foo([]() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " })\n" > > " .foo()\n" > > " .bar();\n" > > "}"); > > verifyFormat("test() {\n" > > " foo([]() -> {\n" > > "int b = 32;\n" > > "return 3;\n" > > " })\n" > > " > > .as(\"\"\n" > > " \"bb\");\n" > > "}"); > > ``` > I'm not sure we agree that the behavior is "faulty" - I do believe it was an > intentional change :) > This is an indent of 4 from the start of the expression that call belongs to. > For example, in example (2), having the .foo() at the end of line after a > potentially complex parameter strongly de-emphasizes the parameter. > In example (3), how would you want to layout a chain of calls? I see! I think your argument makes perfect sense, I think example 3 should be as is, considering that it is consistent with the other behaviour :) thanks for taking the time to explain! Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank updated this revision to Diff 152842. Repository: rC Clang https://reviews.llvm.org/D45719 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -4416,6 +4416,71 @@ verifyFormat(".(\n" "aa)\n" ".aa();"); + + // Dont break if only closing statements before member call + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " (\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .foo();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " ([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo(\"aaa\"\n" + " \"bb\");\n" + "}"); + + verifyFormat("test() {\n" + " foo([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " }).foo();\n" + "}"); + verifyFormat("test() {\n" + " foo(\n" + " []() -> {\n" + "int b = 32;\n" + "return 3;\n" + " },\n" + " foo, bar)\n" + " .as();\n" + "}"); + verifyFormat("test() {\n" + " foo([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .foo()\n" + " .bar();\n" + "}"); + verifyFormat("test() {\n" + " foo([]() -> {\n" + "int b = 32;\n" + "return 3;\n" + " })\n" + " .as(\"\"\n" + " \"bb\");\n" + "}"); } TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -319,6 +319,14 @@ } template bool isNot(T Kind) const { return !is(Kind); } + bool closesScopeAfterBlock() const { +if(BlockKind == BK_Block) + return true; +if(closesScope()) + return Previous->closesScopeAfterBlock(); +return false; + } + /// \c true if this token starts a sequence with the given tokens in order, /// following the ``Next`` pointers, ignoring comments. template Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -399,7 +399,9 @@ // }.bind(...)); // FIXME: We should find a more generic solution to this problem. !(State.Column <= NewLineColumn && -Style.Language == FormatStyle::LK_JavaScript)) +Style.Language == FormatStyle::LK_JavaScript) && + !(Previous.closesScopeAfterBlock() && +State.Column <= NewLineColumn)) return true; // If the template declaration spans multiple lines, force wrap before the ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); ank wrote: > klimek wrote: > > ank wrote: > > > klimek wrote: > > > > What would be interesting is tests that: > > > > a) have another value after the closing }; doesn't really make sense in > > > > this test, but usually these are in calls > > > > f([]() { ... }, foo, bar).call(...) > > > > > > > > b) make .as("") have paramters that go beyond the limit > > > > > > > > c) add another chained call behind .as(""). > > > Hello, sorry for the late follow up on this! > > > > > > Indeed an interesting thing to test, and so I did, the patterns you > > > describe seems to give strange indentation as far back as release_36 and > > > probably has always done so. The case I'm testing here worked in > > > release_40 but stopped working somewhere before release_50 > > > > > > maybe fixing those other cases can be done in another patch > > > > > > cheers, > > > Anders > > I meant: please add these tests :) > I need to clarify, those tests will not be correctly indented by this commit, > I could add those tests but then they would verify a faulty behaviour, this > is how they will be indented even with this patch applied: > > > ``` > // Dont break if only closing statements before member call > verifyFormat("test() {\n" > " ([]() -> {\n" > "int b = 32;\n" > "return 3;\n" > " }).foo();\n" > "}"); > verifyFormat("test() {\n" > " (\n" > " []() -> {\n" > "int b = 32;\n" > "return 3;\n" > " },\n" > " foo, bar)\n" > " .foo();\n" > "}"); > verifyFormat("test() {\n" > " ([]() -> {\n" > "int b = 32;\n" > "return 3;\n" > " })\n" > " .foo()\n" > " .bar();\n" > "}"); > verifyFormat("test() {\n" > " ([]() -> {\n" > "int b = 32;\n" > "return 3;\n" > " })\n" > " > .foo(\"aaa\"\n" > " \"bb\");\n" > "}"); > > verifyFormat("test() {\n" > " foo([]() -> {\n" > "int b = 32;\n" > "return 3;\n" > " }).foo();\n" > "}"); > verifyFormat("test() {\n" > " foo(\n" > " []() -> {\n" > "int b = 32;\n" > "return 3;\n" > " },\n" > " foo, bar)\n" > " .as();\n" > "}"); > verifyFormat("test() {\n" > " foo([]() -> {\n" > "int b = 32;\n" > "return 3;\n" > " })\n" > " .foo()\n" > " .bar();\n" > "}"); > verifyFormat("test() {\n" > " foo([]() -> {\n" > "int b = 32;\n" > "return 3;\n" > " })\n" > " > .as(\"\"\n" > " \"bb\");\n" > "}"); > ``` I'm not sure we agree that the behavior is "faulty" - I do believe it was an intentional change :) This is an indent of 4 from the start of the expression that call belongs to. For example, in example (2), having the .foo() at the end of line after a potentially complex parameter strongly de-emphasizes the parameter. In example (3), how would you want to layout a chain of calls? Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); klimek wrote: > ank wrote: > > klimek wrote: > > > What would be interesting is tests that: > > > a) have another value after the closing }; doesn't really make sense in > > > this test, but usually these are in calls > > > f([]() { ... }, foo, bar).call(...) > > > > > > b) make .as("") have paramters that go beyond the limit > > > > > > c) add another chained call behind .as(""). > > Hello, sorry for the late follow up on this! > > > > Indeed an interesting thing to test, and so I did, the patterns you > > describe seems to give strange indentation as far back as release_36 and > > probably has always done so. The case I'm testing here worked in release_40 > > but stopped working somewhere before release_50 > > > > maybe fixing those other cases can be done in another patch > > > > cheers, > > Anders > I meant: please add these tests :) I need to clarify, those tests will not be correctly indented by this commit, I could add those tests but then they would verify a faulty behaviour, this is how they will be indented even with this patch applied: ``` // Dont break if only closing statements before member call verifyFormat("test() {\n" " ([]() -> {\n" "int b = 32;\n" "return 3;\n" " }).foo();\n" "}"); verifyFormat("test() {\n" " (\n" " []() -> {\n" "int b = 32;\n" "return 3;\n" " },\n" " foo, bar)\n" " .foo();\n" "}"); verifyFormat("test() {\n" " ([]() -> {\n" "int b = 32;\n" "return 3;\n" " })\n" " .foo()\n" " .bar();\n" "}"); verifyFormat("test() {\n" " ([]() -> {\n" "int b = 32;\n" "return 3;\n" " })\n" " .foo(\"aaa\"\n" " \"bb\");\n" "}"); verifyFormat("test() {\n" " foo([]() -> {\n" "int b = 32;\n" "return 3;\n" " }).foo();\n" "}"); verifyFormat("test() {\n" " foo(\n" " []() -> {\n" "int b = 32;\n" "return 3;\n" " },\n" " foo, bar)\n" " .as();\n" "}"); verifyFormat("test() {\n" " foo([]() -> {\n" "int b = 32;\n" "return 3;\n" " })\n" " .foo()\n" " .bar();\n" "}"); verifyFormat("test() {\n" " foo([]() -> {\n" "int b = 32;\n" "return 3;\n" " })\n" " .as(\"\"\n" " \"bb\");\n" "}"); ``` Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); ank wrote: > klimek wrote: > > What would be interesting is tests that: > > a) have another value after the closing }; doesn't really make sense in > > this test, but usually these are in calls > > f([]() { ... }, foo, bar).call(...) > > > > b) make .as("") have paramters that go beyond the limit > > > > c) add another chained call behind .as(""). > Hello, sorry for the late follow up on this! > > Indeed an interesting thing to test, and so I did, the patterns you describe > seems to give strange indentation as far back as release_36 and probably has > always done so. The case I'm testing here worked in release_40 but stopped > working somewhere before release_50 > > maybe fixing those other cases can be done in another patch > > cheers, > Anders I meant: please add these tests :) Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. Is there any chance to get this change or a similar one in so we get same behaviour as in release_40, even though it does not correct all of the problems? Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); klimek wrote: > What would be interesting is tests that: > a) have another value after the closing }; doesn't really make sense in this > test, but usually these are in calls > f([]() { ... }, foo, bar).call(...) > > b) make .as("") have paramters that go beyond the limit > > c) add another chained call behind .as(""). Hello, sorry for the late follow up on this! Indeed an interesting thing to test, and so I did, the patterns you describe seems to give strange indentation as far back as release_36 and probably has always done so. The case I'm testing here worked in release_40 but stopped working somewhere before release_50 maybe fixing those other cases can be done in another patch cheers, Anders Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); What would be interesting is tests that: a) have another value after the closing }; doesn't really make sense in this test, but usually these are in calls f([]() { ... }, foo, bar).call(...) b) make .as("") have paramters that go beyond the limit c) add another chained call behind .as(""). Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. ping Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits