krasimir updated this revision to Diff 167749.
krasimir added a comment.
- Add more tests and tidy-up
Repository:
rC Clang
https://reviews.llvm.org/D52448
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestRawStrings.cpp
Index: unittests/Format/FormatTestRawStrings.cpp
===================================================================
--- unittests/Format/FormatTestRawStrings.cpp
+++ unittests/Format/FormatTestRawStrings.cpp
@@ -576,10 +576,13 @@
auto S = R"pb(item_1:1 item_2:2)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
+ // `rest` fits on the line after )pb", but forced on newline since the raw
+ // string literal is multiline.
expect_eq(R"test(
auto S = R"pb(item_1: 1,
item_2: 2,
- item_3: 3)pb" + rest;)test",
+ item_3: 3)pb" +
+ rest;)test",
format(R"test(
auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
@@ -615,7 +618,8 @@
expect_eq(R"test(
auto S = R"pb(item_1: 1,
item_2: 2,
- item_3: 3)pb" + rest;)test",
+ item_3: 3)pb" +
+ rest;)test",
format(R"test(
auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
@@ -889,6 +893,95 @@
Style));
}
+TEST_F(FormatTestRawStrings,
+ BreaksBeforeNextParamAfterMultilineRawStringParam) {
+ FormatStyle Style = getRawStringPbStyleWithColumns(60);
+ expect_eq(R"test(
+int f() {
+ int a = g(x, R"pb(
+ key: 1 #
+ key: 2
+ )pb",
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, R"pb(
+ key: 1 #
+ key: 2
+ )pb", 3, 4);
+}
+)test",
+ Style));
+
+ // Breaks after a parent of a multiline param.
+ expect_eq(R"test(
+int f() {
+ int a = g(x, h(R"pb(
+ key: 1 #
+ key: 2
+ )pb"),
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, h(R"pb(
+ key: 1 #
+ key: 2
+ )pb"), 3, 4);
+}
+)test",
+ Style));
+
+ expect_eq(R"test(
+int f() {
+ int a = g(x,
+ h(R"pb(
+ key: 1 #
+ key: 2
+ )pb",
+ 2),
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, h(R"pb(
+ key: 1 #
+ key: 2
+ )pb", 2), 3, 4);
+}
+)test",
+ Style));
+ // Breaks if formatting introduces a multiline raw string.
+ expect_eq(R"test(
+int f() {
+ int a = g(x, R"pb(key1: value111111111
+ key2: value2222222222)pb",
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, R"pb(key1: value111111111 key2: value2222222222)pb", 3, 4);
+}
+)test",
+ Style));
+ // Does not force a break after an original multiline param that is
+ // reformatterd as on single line.
+ expect_eq(R"test(
+int f() {
+ int a = g(R"pb(key: 1)pb", 2);
+})test",
+ format(R"test(
+int f() {
+ int a = g(R"pb(key:
+ 1)pb", 2);
+})test", Style));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1502,10 +1502,25 @@
// violate the rectangle rule and visually flows within the surrounding
// source.
bool ContentStartsOnNewline = Current.TokenText[OldPrefixSize] == '\n';
- unsigned NextStartColumn =
- ContentStartsOnNewline
- ? State.Stack.back().NestedBlockIndent + Style.IndentWidth
- : FirstStartColumn;
+ // If this token is the last parameter (checked by looking if it's followed by
+ // `)`, the base the indent off the line's nested block indent. Otherwise,
+ // base the indent off the arguments indent, so we can achieve:
+ // fffffffffff(1, 2, 3, R"pb(
+ // key1: 1 #
+ // key2: 2)pb");
+ //
+ // fffffffffff(1, 2, 3,
+ // R"pb(
+ // key1: 1 #
+ // key2: 2
+ // )pb",
+ // 5);
+ unsigned CurrentIndent = (Current.Next && Current.Next->is(tok::r_paren))
+ ? State.Stack.back().NestedBlockIndent
+ : State.Stack.back().Indent;
+ unsigned NextStartColumn = ContentStartsOnNewline
+ ? CurrentIndent + Style.IndentWidth
+ : FirstStartColumn;
// The last start column is the column the raw string suffix starts if it is
// put on a newline.
@@ -1517,7 +1532,7 @@
// indent.
unsigned LastStartColumn = Current.NewlinesBefore
? FirstStartColumn - NewPrefixSize
- : State.Stack.back().NestedBlockIndent;
+ : CurrentIndent;
std::pair<tooling::Replacements, unsigned> Fixes = internal::reformat(
RawStringStyle, RawText, {tooling::Range(0, RawText.size())},
@@ -1527,8 +1542,7 @@
auto NewCode = applyAllReplacements(RawText, Fixes.first);
tooling::Replacements NoFixes;
if (!NewCode) {
- State.Column += Current.ColumnWidth;
- return 0;
+ return addMultilineToken(Current, State);
}
if (!DryRun) {
if (NewDelimiter != OldDelimiter) {
@@ -1577,7 +1591,16 @@
unsigned PrefixExcessCharacters =
StartColumn + NewPrefixSize > Style.ColumnLimit ?
StartColumn + NewPrefixSize - Style.ColumnLimit : 0;
- return Fixes.second + PrefixExcessCharacters * Style.PenaltyExcessCharacter;
+ unsigned Penalty =
+ Fixes.second + PrefixExcessCharacters * Style.PenaltyExcessCharacter;
+ bool IsMultiline =
+ ContentStartsOnNewline || (NewCode->find('\n') != std::string::npos);
+ if (IsMultiline) {
+ // Break before further function parameters on all levels.
+ for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
+ State.Stack[i].BreakBeforeParameter = true;
+ }
+ return Penalty;
}
unsigned ContinuationIndenter::addMultilineToken(const FormatToken &Current,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
