[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333171: [clang-format] Fix putting ObjC message arguments in
one line for multiline… (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47195?vs=148169=148370#toc
Repository:
rC Clang
https://reviews.llvm.org/D47195
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1387,6 +1387,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,41 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+ // Arguments just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1387,6 +1387,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak added a comment. Does it look fine now @krasimir ? Repository: rC Clang https://reviews.llvm.org/D47195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
krasimir wrote:
> jolesiak wrote:
> > krasimir wrote:
> > > jolesiak wrote:
> > > > krasimir wrote:
> > > > > What's the receiver's scope in this comment referring to?
> > > > > Also, how would the old test cases be formatted?
> > > > For a receiver: `[obj a:42]` I meant `]` as a token closing the scope.
> > > > I'll rephrase the comment to be more precise.
> > > >
> > > > Old tests were introduced in D46879. After this change the formatting
> > > > will be the same as it was before D46879, i.e. the same as for last
> > > > test touched in this change:
> > > > ```
> > > > [[obj aa:42
> > > >b:42]
> > > > cc:42
> > > > d:42];
> > > > ```
> > > > even if
> > > > ```
> > > > [[obj aa:42
> > > >b:42]
> > > > cc:42 d:42];
> > > > ```
> > > > satisfies the column limit.
> > > Ah, I think get it now: the new code should only apply to after object
> > > blocks and not after object receivers? Is this the intention?
> > The intention is to put arguments into one line if they fit but only in the
> > same line as last character of a receiver expression, e.g.:
> > ```
> > [[object block:^{
> > return 42;
> > }] aa:42 bb:42];
> > ```
> > instead of
> > ```
> > [[object block:^{
> > return 42;
> > }] aa:42
> >bb:42];
> > ```
> > but not
> > ```
> > [[obj a:42]
> > a:42 b:42];
> > ```
> >
> > I think it gets a little bit too complicated for no reason. Let me revert
> > D46879 and rebase this change.
> In that case, would this be allowed?
> ```
> // limit: V
> [[obj a:42
> b:42
> c:42
> d:42] e:42 f:42]
> ```
Yes, I added this test.
Repository:
rC Clang
https://reviews.llvm.org/D47195
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak updated this revision to Diff 148169.
jolesiak added a comment.
- Add test
Repository:
rC Clang
https://reviews.llvm.org/D47195
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,41 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+ // Arguments just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1386,6 +1386,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,41 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+ // Arguments just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
krasimir added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
jolesiak wrote:
> krasimir wrote:
> > jolesiak wrote:
> > > krasimir wrote:
> > > > What's the receiver's scope in this comment referring to?
> > > > Also, how would the old test cases be formatted?
> > > For a receiver: `[obj a:42]` I meant `]` as a token closing the scope.
> > > I'll rephrase the comment to be more precise.
> > >
> > > Old tests were introduced in D46879. After this change the formatting
> > > will be the same as it was before D46879, i.e. the same as for last test
> > > touched in this change:
> > > ```
> > > [[obj aa:42
> > >b:42]
> > > cc:42
> > > d:42];
> > > ```
> > > even if
> > > ```
> > > [[obj aa:42
> > >b:42]
> > > cc:42 d:42];
> > > ```
> > > satisfies the column limit.
> > Ah, I think get it now: the new code should only apply to after object
> > blocks and not after object receivers? Is this the intention?
> The intention is to put arguments into one line if they fit but only in the
> same line as last character of a receiver expression, e.g.:
> ```
> [[object block:^{
> return 42;
> }] aa:42 bb:42];
> ```
> instead of
> ```
> [[object block:^{
> return 42;
> }] aa:42
>bb:42];
> ```
> but not
> ```
> [[obj a:42]
> a:42 b:42];
> ```
>
> I think it gets a little bit too complicated for no reason. Let me revert
> D46879 and rebase this change.
In that case, would this be allowed?
```
// limit: V
[[obj a:42
b:42
c:42
d:42] e:42 f:42]
```
Repository:
rC Clang
https://reviews.llvm.org/D47195
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
