krasimir updated this revision to Diff 150273.
krasimir added a comment.
- Polish
Repository:
rC Clang
https://reviews.llvm.org/D46757
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTestProto.cpp
unittests/Format/FormatTestRawStrings.cpp
unittests/Format/FormatTestTextProto.cpp
Index: unittests/Format/FormatTestTextProto.cpp
===================================================================
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -171,17 +171,48 @@
verifyFormat("msg_field < field_a < field_b <> > >");
verifyFormat("msg_field: < field_a < field_b: <> > >");
verifyFormat("msg_field < field_a: OK, field_b: \"OK\" >");
- verifyFormat("msg_field < field_a: OK field_b: <>, field_c: OK >");
- verifyFormat("msg_field < field_a { field_b: 1 }, field_c: < f_d: 2 > >");
verifyFormat("msg_field: < field_a: OK, field_b: \"OK\" >");
- verifyFormat("msg_field: < field_a: OK field_b: <>, field_c: OK >");
- verifyFormat("msg_field: < field_a { field_b: 1 }, field_c: < fd_d: 2 > >");
- verifyFormat("field_a: \"OK\", msg_field: < field_b: 123 >, field_c: {}");
- verifyFormat("field_a < field_b: 1 >, msg_fid: < fiel_b: 123 >, field_c <>");
- verifyFormat("field_a < field_b: 1 > msg_fied: < field_b: 123 > field_c <>");
- verifyFormat("field < field < field: <> >, field <> > field: < field: 1 >");
-
// Multiple lines tests
+ verifyFormat("msg_field <\n"
+ " field_a: OK\n"
+ " field_b: <>,\n"
+ " field_c: OK\n"
+ ">");
+
+ verifyFormat("msg_field <\n"
+ " field_a { field_b: 1 },\n"
+ " field_c: < f_d: 2 >\n"
+ ">");
+
+ verifyFormat("msg_field: <\n"
+ " field_a: OK\n"
+ " field_b: <>,\n"
+ " field_c: OK\n"
+ ">");
+
+ verifyFormat("msg_field: <\n"
+ " field_a { field_b: 1 },\n"
+ " field_c: < fd_d: 2 >\n"
+ ">");
+
+ verifyFormat("field_a: \"OK\",\n"
+ "msg_field: < field_b: 123 >,\n"
+ "field_c: {}");
+
+ verifyFormat("field_a < field_b: 1 >,\n"
+ "msg_fid: < fiel_b: 123 >,\n"
+ "field_c <>");
+
+ verifyFormat("field_a < field_b: 1 >\n"
+ "msg_fied: < field_b: 123 >\n"
+ "field_c <>");
+
+ verifyFormat("field <\n"
+ " field < field: <> >,\n"
+ " field <>\n"
+ ">\n"
+ "field: < field: 1 >");
+
verifyFormat("msg_field <\n"
" field_a: OK\n"
" field_b: \"OK\"\n"
@@ -242,7 +273,10 @@
" field_d: ok\n"
"}");
- verifyFormat("field_a: < f1: 1, f2: <> >\n"
+ verifyFormat("field_a: <\n"
+ " f1: 1,\n"
+ " f2: <>\n"
+ ">\n"
"field_b <\n"
" field_b1: <>\n"
" field_b2: ok,\n"
@@ -507,5 +541,107 @@
">");
}
+TEST_F(FormatTestTextProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+ Style.ColumnLimit = 60;
+ // The column limit allows for the keys submessage to be put on 1 line, but we
+ // break it since it contains a submessage an another entry.
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " sub <>\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " sub {}\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub {}\n"
+ " sub: <>\n"
+ " sub: []\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub { msg: 1 }\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub: { msg: 1 }\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub < msg: 1 >\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub: [ msg: 1 ]\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: <\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub: [ 1, 2 ]\n"
+ ">");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub {}\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub: []\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub <>\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub { key: value }\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub: [ 1, 2 ]\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " sub < sub_2: {} >\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: data\n"
+ " sub: [ 1, 2 ]\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("key: valueeeeeeee\n"
+ "keys: {\n"
+ " item: data\n"
+ " sub < sub_2: {} >\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ "}");
+ verifyFormat("sub: {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub: [ 1, 2 ]\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("sub: {\n"
+ " sub: {}\n"
+ "}\n"
+ "\n"
+ "# comment\n");
+}
+
} // end namespace tooling
} // end namespace clang
Index: unittests/Format/FormatTestRawStrings.cpp
===================================================================
--- unittests/Format/FormatTestRawStrings.cpp
+++ unittests/Format/FormatTestRawStrings.cpp
@@ -199,12 +199,6 @@
format(
R"test(P p = TP(R"pb(item_1:1 item_2:2)pb");)test",
getRawStringPbStyleWithColumns(40)));
- expect_eq(
- R"test(P p = TP(R"pb(item_1 < 1 > item_2: { 2 })pb");)test",
- format(
- R"test(P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
- getRawStringPbStyleWithColumns(40)));
-
// Merge two short lines into one.
expect_eq(R"test(
std::string s = R"pb(
@@ -220,6 +214,18 @@
getRawStringPbStyleWithColumns(40)));
}
+TEST_F(FormatTestRawStrings, BreaksShortRawStringsWhenNeeded) {
+ // The raw string contains multiple submessage entries, so break for
+ // readability.
+ expect_eq(R"test(
+P p = TP(R"pb(item_1 < 1 >
+ item_2: { 2 })pb");)test",
+ format(
+ R"test(
+P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
+ getRawStringPbStyleWithColumns(40)));
+}
+
TEST_F(FormatTestRawStrings, BreaksRawStringsExceedingColumnLimit) {
expect_eq(R"test(
P p = TPPPPPPPPPPPPPPP(
Index: unittests/Format/FormatTestProto.cpp
===================================================================
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -493,5 +493,136 @@
"};");
}
+TEST_F(FormatTestProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+ Style.ColumnLimit = 60;
+ // The column limit allows for the keys submessage to be put on 1 line, but we
+ // break it since it contains a submessage an another entry.
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " sub <>\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " sub {}\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub {}\n"
+ " sub: <>\n"
+ " sub: []\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub { msg: 1 }\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub: { msg: 1 }\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub < msg: 1 >\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub: [ msg: 1 ]\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: <\n"
+ " item: 'aaaaaaaaaaa'\n"
+ " sub: [ 1, 2 ]\n"
+ " >\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub {}\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub: []\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub <>\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub { key: value }\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub: [ 1, 2 ]\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub < sub_2: {} >\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: data\n"
+ " sub: [ 1, 2 ]\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " item: data\n"
+ " sub < sub_2: {} >\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ "}");
+ verifyFormat("option (MyProto.options) = {\n"
+ " sub: {\n"
+ " key: valueeeeeeee\n"
+ " keys: {\n"
+ " sub: [ 1, 2 ]\n"
+ " item: 'aaaaaaaaaaaaaaaa'\n"
+ " }\n"
+ " }\n"
+ "}");
+}
+
} // end namespace tooling
} // end namespace clang
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -14,6 +14,10 @@
#include <queue>
#define DEBUG_TYPE "format-formatter"
+#define DBG(A) \
+ DEBUG({ \
+ llvm::dbgs() << __func__ << ":" << __LINE__ << #A << " = " << A << "\n"; \
+ });
namespace clang {
namespace format {
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -19,6 +19,10 @@
#include "llvm/Support/Debug.h"
#define DEBUG_TYPE "format-token-annotator"
+#define DBG(A) \
+ DEBUG({ \
+ llvm::dbgs() << __func__ << ":" << __LINE__ << #A << " = " << A << "\n"; \
+ });
namespace clang {
namespace format {
@@ -2924,6 +2928,68 @@
if (Right.is(TT_ProtoExtensionLSquare))
return true;
+ // In text proto instances if a submessage contains at least 2 entries and at
+ // least one of them is a submessage, like A { ... B { ... } ... },
+ // put all of the entries of A on separate lines by forcing the selector of
+ // the submessage B to be put on a newline.
+ //
+ // Example: these can stay on one line:
+ // a { scalar_1: 1 scalar_2: 2 }
+ // a { b { key: value } }
+ //
+ // and these entries need to be on a new line even if putting them all in one
+ // line is under the column limit:
+ // a {
+ // scalar: 1
+ // b { key: value }
+ // }
+ //
+ // Be careful to exclude the case [proto.ext] { ... } since the `]` is
+ // a TT_SelectorName there, but we do not want to put a line break before `]`.
+ if ((Style.Language == FormatStyle::LK_Proto ||
+ Style.Language == FormatStyle::LK_TextProto) &&
+ Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
+ // Look for the scope opener after selector in cases like:
+ // selector { ...
+ // selector: { ...
+ FormatToken *LParen =
+ Right.Next->is(tok::colon) ? Right.Next->Next : Right.Next;
+ if (LParen &&
+ // The scope opener is one of {, [, <:
+ // selector { ... }
+ // selector [ ... ]
+ // selector < ... >
+ //
+ // In case of selector { ... }, the l_brace is TT_DictLiteral.
+ // In case of an empty selector {}, the l_brace is not TT_DictLiteral,
+ // so we check for immediately following r_brace.
+ ((LParen->is(tok::l_brace) &&
+ (LParen->is(TT_DictLiteral) ||
+ (LParen->Next && LParen->Next->is(tok::r_brace)))) ||
+ LParen->is(TT_ArrayInitializerLSquare) || LParen->is(tok::less))) {
+ // If Left.ParameterCount is 0, then this submessage entry is not the
+ // first in its parent submessage, and we want to break before this entry.
+ // If Left.ParameterCount is greater than 0, then its parent submessage
+ // might contain 1 or more entries and we want to break before this entry
+ // if it contains at least 2 entries. We deal with this case later by
+ // detecting and breaking before the next entry in the parent submessage.
+ if (Left.ParameterCount == 0)
+ return true;
+ // However, if this submessage is the first entry in its parent
+ // submessage, Left.ParameterCount might be 1 in some cases.
+ // We deal with this case later by detecting an entry
+ // following a closing paren of this submessage.
+ }
+
+ // If this is an entry immediately following a submessage, it will be
+ // preceeded by a closing paren of that submessage, like in:
+ // left---. .---right
+ // v v
+ // sub: { ... } key: value
+ if (Left.isOneOf(tok::r_brace, tok::greater, tok::r_square))
+ return true;
+ }
+
return false;
}
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -354,6 +354,7 @@
State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
!Current.isOneOf(tok::r_paren, tok::r_brace))
return true;
+
if (((Previous.is(TT_DictLiteral) && Previous.is(tok::l_brace)) ||
(Previous.is(TT_ArrayInitializerLSquare) &&
Previous.ParameterCount > 1) ||
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
