[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 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
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. 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 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
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 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 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
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
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 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
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 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
