pastey updated this revision to Diff 235466.
pastey added a comment.
Bouska, thank you! Don't know how I missed that failing test. Never commit on
friday :)
However, seeing this test result in a browser window gave me that "a-ha". I
understood that I had wrong perception of what MultiLine feature is all about.
As I wrote before, I would expect it to format code like this:
if (foo ||
bar)
{
doSomething();
}
else {
doSomethingElse();
}
In my head it didn't have anything to do with ColumnLimit – it was all about
more than one line in 'if' condition expression.
After I understood that MultiLine is bound to ColumnLimit, I could understand
what was going on @ line 308 on UnwrappedLineFormatter.cpp.
Also found that UnwrappedLine actually wraps every line, which is ... confusing.
Uploaded the new diff. Removed my previous "fix" from UnwrappedLineParser.cpp.
Added special case for else/catch in UnwrappedLineFormatter.cpp. Tests pass.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71939/new/
https://reviews.llvm.org/D71939
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
" baz();\n"
"}",
format("try{foo();}catch(Exception&bar){baz();}", Style));
+ Style.ColumnLimit =
+ 40; // to concentrate at brace wrapping, not line wrap due to column
limit
+ EXPECT_EQ("try {\n"
+ " foo();\n"
+ "} catch (Exception &bar) {\n"
+ " baz();\n"
+ "}",
+ format("try{foo();}catch(Exception&bar){baz();}", Style));
+ Style.ColumnLimit =
+ 20; // to concentrate at brace wrapping, not line wrap due to column
limit
+
+ Style.BraceWrapping.BeforeElse = true;
+ EXPECT_EQ(
+ "if (foo) {\n"
+ " bar();\n"
+ "}\n"
+ "else if (baz ||\n"
+ " quux)\n"
+ "{\n"
+ " foobar();\n"
+ "}\n"
+ "else {\n"
+ " barbaz();\n"
+ "}",
+ format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
+ Style));
+
+ Style.BraceWrapping.BeforeCatch = true;
+ EXPECT_EQ("try {\n"
+ " foo();\n"
+ "}\n"
+ "catch (...) {\n"
+ " baz();\n"
+ "}",
+ format("try{foo();}catch(...){baz();}", Style));
}
//===----------------------------------------------------------------------===//
@@ -14881,7 +14916,7 @@
verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
}
- TEST_F(FormatTest, SpacesInConditionalStatement) {
+TEST_F(FormatTest, SpacesInConditionalStatement) {
FormatStyle Spaces = getLLVMStyle();
Spaces.SpacesInConditionalStatement = true;
verifyFormat("for ( int i = 0; i; i++ )\n continue;", Spaces);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -326,6 +326,14 @@
FormatStyle::BWACS_Always)
? tryMergeSimpleBlock(I, E, Limit)
: 0;
+ } else if (I[1]->First->is(tok::l_brace) &&
+ TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
+ Style.BraceWrapping.AfterControlStatement ==
+ FormatStyle::BWACS_MultiLine) {
+ return (Style.ColumnLimit == 0 ||
+ TheLine->Last->TotalLength <= Style.ColumnLimit)
+ ? 1
+ : 0;
}
// Try to merge either empty or one-line block if is precedeed by control
// statement token
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
" baz();\n"
"}",
format("try{foo();}catch(Exception&bar){baz();}", Style));
+ Style.ColumnLimit =
+ 40; // to concentrate at brace wrapping, not line wrap due to column limit
+ EXPECT_EQ("try {\n"
+ " foo();\n"
+ "} catch (Exception &bar) {\n"
+ " baz();\n"
+ "}",
+ format("try{foo();}catch(Exception&bar){baz();}", Style));
+ Style.ColumnLimit =
+ 20; // to concentrate at brace wrapping, not line wrap due to column limit
+
+ Style.BraceWrapping.BeforeElse = true;
+ EXPECT_EQ(
+ "if (foo) {\n"
+ " bar();\n"
+ "}\n"
+ "else if (baz ||\n"
+ " quux)\n"
+ "{\n"
+ " foobar();\n"
+ "}\n"
+ "else {\n"
+ " barbaz();\n"
+ "}",
+ format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
+ Style));
+
+ Style.BraceWrapping.BeforeCatch = true;
+ EXPECT_EQ("try {\n"
+ " foo();\n"
+ "}\n"
+ "catch (...) {\n"
+ " baz();\n"
+ "}",
+ format("try{foo();}catch(...){baz();}", Style));
}
//===----------------------------------------------------------------------===//
@@ -14881,7 +14916,7 @@
verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
}
- TEST_F(FormatTest, SpacesInConditionalStatement) {
+TEST_F(FormatTest, SpacesInConditionalStatement) {
FormatStyle Spaces = getLLVMStyle();
Spaces.SpacesInConditionalStatement = true;
verifyFormat("for ( int i = 0; i; i++ )\n continue;", Spaces);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -326,6 +326,14 @@
FormatStyle::BWACS_Always)
? tryMergeSimpleBlock(I, E, Limit)
: 0;
+ } else if (I[1]->First->is(tok::l_brace) &&
+ TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
+ Style.BraceWrapping.AfterControlStatement ==
+ FormatStyle::BWACS_MultiLine) {
+ return (Style.ColumnLimit == 0 ||
+ TheLine->Last->TotalLength <= Style.ColumnLimit)
+ ? 1
+ : 0;
}
// Try to merge either empty or one-line block if is precedeed by control
// statement token
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
