[PATCH] D48063: [clang-format] Discourage breaks in submessage entries, hard rule
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334517: [clang-format] Discourage breaks in submessage
entries, hard rule (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D48063
Files:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -3101,10 +3101,39 @@
!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: valu }
+ //
+ // when the snippet does not fit into one line.
+ // Prefer:
+ //
+ // submessage: {
+ // key: valu
+ // }
+ //
+ // 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 &&
Index: cfe/trunk/unittests/Format/FormatTestTextProto.cpp
===
--- cfe/trunk/unittests/Format/FormatTestTextProto.cpp
+++ cfe/trunk/unittests/Format/FormatTestTextProto.cpp
@@ -485,8 +485,15 @@
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, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("submessage: {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage: <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("repeatedd: [\n"
+ " 'ey'\n"
+ "]");
+ // "{" is going over the column limit.
+ verifyFormat(
+ "submessagee: {\n"
+ " key: 'a'\n"
+ "}");
+}
+
} // end namespace tooling
} // end namespace clang
Index: cfe/trunk/unittests/Format/FormatTestProto.cpp
===
--- cfe/trunk/unittests/Format/FormatTestProto.cpp
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp
@@ -624,5 +624,34 @@
"}");
}
+TEST_F(FormatTestProto, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: {\n"
+ "key: 'a'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage {\n"
+ "key: 'a'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: <\n"
+ "key: 'a'\n"
[PATCH] D48063: [clang-format] Discourage breaks in submessage entries, hard rule
krasimir updated this revision to Diff 150980.
krasimir added a comment.
- Split-up the if-condition
Repository:
rC Clang
https://reviews.llvm.org/D48063
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestProto.cpp
unittests/Format/FormatTestTextProto.cpp
Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -485,8 +485,15 @@
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, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("submessage: {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage: <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("repeatedd: [\n"
+ " 'ey'\n"
+ "]");
+ // "{" is going over the column limit.
+ verifyFormat(
+ "submessagee: {\n"
+ " key: 'a'\n"
+ "}");
+}
+
} // end namespace tooling
} // end namespace clang
Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -624,5 +624,34 @@
"}");
}
+TEST_F(FormatTestProto, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: {\n"
+ "key: 'a'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage {\n"
+ "key: 'a'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: <\n"
+ "key: 'a'\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage <\n"
+ "key: 'a'\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " repeatedd: [\n"
+ "'ey'\n"
+ " ]\n"
+ "}");
+}
+
+
} // end namespace tooling
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -3101,10 +3101,39 @@
!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: valu }
+ //
+ // when the snippet does not fit into one line.
+ // Prefer:
+ //
+ // submessage: {
+ // key: valu
+ // }
+ //
+ // 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: [ ... ]
+
[PATCH] D48063: [clang-format] Discourage breaks in submessage entries, hard rule
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Format/TokenAnnotator.cpp:3104
if (Left.is(tok::colon) && Left.isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
if ((Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) &&
It's really hard to follow the boolean logic here.
Can you break this up into multiple if statements with comments, or extract
some named subexpressions or something? e.g.
```
if (proto or textproto) {
if (bool isSubmessage = ...)
return false;
if (right is string && !break before multiline)
return false;
return true;
}
Repository:
rC Clang
https://reviews.llvm.org/D48063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48063: [clang-format] Discourage breaks in submessage entries, hard rule
krasimir updated this revision to Diff 150926.
krasimir added a comment.
- Add tests
Repository:
rC Clang
https://reviews.llvm.org/D48063
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestProto.cpp
unittests/Format/FormatTestTextProto.cpp
Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -485,8 +485,15 @@
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, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("submessage: {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage: <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("repeatedd: [\n"
+ " 'ey'\n"
+ "]");
+ // "{" is going over the column limit.
+ verifyFormat(
+ "submessagee: {\n"
+ " key: 'a'\n"
+ "}");
+}
+
} // end namespace tooling
} // end namespace clang
Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -624,5 +624,34 @@
"}");
}
+TEST_F(FormatTestProto, PreventBreaksBetweenKeyAndSubmessages) {
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: {\n"
+ "key: 'a'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage {\n"
+ "key: 'a'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage: <\n"
+ "key: 'a'\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " submessage <\n"
+ "key: 'a'\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " repeatedd: [\n"
+ "'ey'\n"
+ " ]\n"
+ "}");
+}
+
+
} // end namespace tooling
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -3103,7 +3103,35 @@
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())
+((!Style.AlwaysBreakBeforeMultilineStrings &&
+ Right.isStringLiteral()) ||
+ // Prevent cases like:
+ //
+ // submessage:
+ // { key: valu }
+ //
+ // when the snippet does not fit into one line.
+ // Prefer:
+ //
+ // submessage: {
+ // key: valu
+ // }
+ //
+ // 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: [ ... ]
+ ((Right.is(tok::l_brace) || Right.is(tok::less)) &&
+ Right.is(TT_DictLiteral)) ||
+ Right.is(TT_ArrayInitializerLSquare)))
return false;
[PATCH] D48063: [clang-format] Discourage breaks in submessage entries, hard rule
krasimir created this revision.
Herald added a subscriber: cfe-commits.
This is an alternative to https://reviews.llvm.org/D48034.
Repository:
rC Clang
https://reviews.llvm.org/D48063
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestTextProto.cpp
Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -485,8 +485,15 @@
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,23 @@
"}");
}
+TEST_F(FormatTestTextProto, TODO) {
+ verifyFormat("submessage: {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage: <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage: [\n"
+ " 'ey'\n"
+ "]");
+}
+
} // end namespace tooling
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -3101,10 +3101,16 @@
!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;
+ if ((Right.is(tok::l_brace) || Right.is(tok::less)) &&
+ Right.is(TT_DictLiteral))
+return false;
+ if (Right.is(TT_ArrayInitializerLSquare))
+return false;
+}
return true;
}
if (Right.is(tok::r_square) && Right.MatchingParen &&
Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -485,8 +485,15 @@
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,23 @@
"}");
}
+TEST_F(FormatTestTextProto, TODO) {
+ verifyFormat("submessage: {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage {\n"
+ " key: 'a'\n"
+ "}");
+ verifyFormat("submessage: <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage <\n"
+ " key: 'a'\n"
+ ">");
+ verifyFormat("submessage: [\n"
+ " 'ey'\n"
+ "]");
+}
+
} // end namespace tooling
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -3101,10 +3101,16 @@
!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) &&
-
