Author: krasimir
Date: Tue Jun 12 10:26:31 2018
New Revision: 334517
URL: http://llvm.org/viewvc/llvm-project?rev=334517&view=rev
Log:
[clang-format] Discourage breaks in submessage entries, hard rule
Summary:
Currently clang-format allows this for text protos:
```
submessage:
{ key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' }
```
when it is under the column limit and when putting it all on one line exceeds
the column limit.
This is not a very intuitive formatting, so I'd prefer having
```
submessage: {
key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
}
```
instead, even if it takes one line more.
This patch prevents clang-format from inserting a break between `: {` and
similar cases.
Reviewers: djasper, sammccall
Reviewed By: sammccall
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D48063
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=334517&r1=334516&r2=334517&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Jun 12 10:26:31 2018
@@ -3101,10 +3101,39 @@ bool TokenAnnotator::canBreakBefore(cons
!Right.isOneOf(TT_CtorInitializerColon, TT_InlineASMColon))
return false;
if (Left.is(tok::colon) && Left.isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
- if ((Style.Language == FormatStyle::LK_Proto ||
- Style.Language == FormatStyle::LK_TextProto) &&
- !Style.AlwaysBreakBeforeMultilineStrings && Right.isStringLiteral())
- return false;
+ if (Style.Language == FormatStyle::LK_Proto ||
+ Style.Language == FormatStyle::LK_TextProto) {
+ if (!Style.AlwaysBreakBeforeMultilineStrings && Right.isStringLiteral())
+ return false;
+ // Prevent cases like:
+ //
+ // submessage:
+ // { key: valueeeeeeeeeeee }
+ //
+ // when the snippet does not fit into one line.
+ // Prefer:
+ //
+ // submessage: {
+ // key: valueeeeeeeeeeee
+ // }
+ //
+ // instead, even if it is longer by one line.
+ //
+ // Note that this allows allows the "{" to go over the column limit
+ // when the column limit is just between ":" and "{", but that does
+ // not happen too often and alternative formattings in this case are
+ // not much better.
+ //
+ // The code covers the cases:
+ //
+ // submessage: { ... }
+ // submessage: < ... >
+ // repeated: [ ... ]
+ if (((Right.is(tok::l_brace) || Right.is(tok::less)) &&
+ Right.is(TT_DictLiteral)) ||
+ Right.is(TT_ArrayInitializerLSquare))
+ return false;
+ }
return true;
}
if (Right.is(tok::r_square) && Right.MatchingParen &&
Modified: cfe/trunk/unittests/Format/FormatTestProto.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=334517&r1=334516&r2=334517&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp Tue Jun 12 10:26:31 2018
@@ -624,5 +624,34 @@ TEST_F(FormatTestProto, BreaksEntriesOfS
"}");
}
+TEST_F(FormatTestProto, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: {\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage {\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: <\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage <\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " repeatedd: [\n"
+ " 'eyaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ " ]\n"
+ "}");
+}
+
+
} // end namespace tooling
} // end namespace clang
Modified: cfe/trunk/unittests/Format/FormatTestTextProto.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestTextProto.cpp?rev=334517&r1=334516&r2=334517&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestTextProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestTextProto.cpp Tue Jun 12 10:26:31 2018
@@ -485,8 +485,15 @@ TEST_F(FormatTestTextProto, FormatsRepea
verifyFormat("keys: []");
verifyFormat("keys: [ 1 ]");
verifyFormat("keys: [ 'ala', 'bala' ]");
- verifyFormat("keys:\n"
- " [ 'ala', 'bala', 'porto', 'kala', 'too', 'long', 'ng' ]");
+ verifyFormat("keys: [\n"
+ " 'ala',\n"
+ " 'bala',\n"
+ " 'porto',\n"
+ " 'kala',\n"
+ " 'too',\n"
+ " 'long',\n"
+ " 'ng'\n"
+ "]");
verifyFormat("key: item\n"
"keys: [\n"
" 'ala',\n"
@@ -670,5 +677,28 @@ TEST_F(FormatTestTextProto, BreaksEntrie
"}");
}
+TEST_F(FormatTestTextProto, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("submessage: {\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("submessage {\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("submessage: <\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ ">");
+ verifyFormat("submessage <\n"
+ " key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ ">");
+ verifyFormat("repeatedd: [\n"
+ " 'eyaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+ "]");
+ // "{" is going over the column limit.
+ verifyFormat(
+ "submessageeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee: {\n"
+ " key: 'aaaaa'\n"
+ "}");
+}
+
} // end namespace tooling
} // end namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
