<div dir="ltr">Can you make sure that all changes are covered in the test? For example, I see no 'break' statement in the test.<div><br></div><div>Thanks!</div><div>/Manuel</div></div><div class="gmail_extra"><br><br>
<div class="gmail_quote">On Thu, Aug 1, 2013 at 3:17 PM, Frank Miller <span dir="ltr"><<a href="mailto:fmiller@sjm.com" target="_blank">fmiller@sjm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Allman style is the opposite of attach. Braces are always on their own line.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1257" target="_blank">http://llvm-reviews.chandlerc.com/D1257</a><br>
<br>
Files:<br>
include/clang/Format/Format.h<br>
lib/Format/Format.cpp<br>
lib/Format/UnwrappedLineParser.cpp<br>
unittests/Format/FormatTest.cpp<br>
<br>
Index: include/clang/Format/Format.h<br>
===================================================================<br>
--- include/clang/Format/Format.h<br>
+++ include/clang/Format/Format.h<br>
@@ -163,7 +163,9 @@<br>
/// class definitions.<br>
BS_Linux,<br>
/// Like \c Attach, but break before function definitions.<br>
- BS_Stroustrup<br>
+ BS_Stroustrup,<br>
+ /// Always break before braces<br>
+ BS_Allman<br>
};<br>
<br>
/// \brief The brace breaking style to use.<br>
Index: lib/Format/Format.cpp<br>
===================================================================<br>
--- lib/Format/Format.cpp<br>
+++ lib/Format/Format.cpp<br>
@@ -50,6 +50,7 @@<br>
IO.enumCase(Value, "Attach", clang::format::FormatStyle::BS_Attach);<br>
IO.enumCase(Value, "Linux", clang::format::FormatStyle::BS_Linux);<br>
IO.enumCase(Value, "Stroustrup", clang::format::FormatStyle::BS_Stroustrup);<br>
+ IO.enumCase(Value, "Allman", clang::format::FormatStyle::BS_Allman);<br>
}<br>
};<br>
<br>
Index: lib/Format/UnwrappedLineParser.cpp<br>
===================================================================<br>
--- lib/Format/UnwrappedLineParser.cpp<br>
+++ lib/Format/UnwrappedLineParser.cpp<br>
@@ -594,7 +594,8 @@<br>
// FIXME: Figure out cases where this is not true, and add projections<br>
// for them (the one we know is missing are lambdas).<br>
if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||<br>
- Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup)<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
addUnwrappedLine();<br>
@@ -759,8 +760,13 @@<br>
parseParens();<br>
bool NeedsUnwrappedLine = false;<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
- NeedsUnwrappedLine = true;<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
+ else<br>
+ NeedsUnwrappedLine = true;<br>
} else {<br>
addUnwrappedLine();<br>
++Line->Level;<br>
@@ -770,6 +776,8 @@<br>
if (FormatTok->Tok.is(tok::kw_else)) {<br>
nextToken();<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
addUnwrappedLine();<br>
} else if (FormatTok->Tok.is(tok::kw_if)) {<br>
@@ -791,7 +799,8 @@<br>
if (FormatTok->Tok.is(tok::identifier))<br>
nextToken();<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
- if (Style.BreakBeforeBraces == FormatStyle::BS_Linux)<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
addUnwrappedLine();<br>
<br>
bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||<br>
@@ -814,6 +823,8 @@<br>
if (FormatTok->Tok.is(tok::l_paren))<br>
parseParens();<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
addUnwrappedLine();<br>
} else {<br>
@@ -828,6 +839,8 @@<br>
assert(FormatTok->Tok.is(tok::kw_do) && "'do' expected");<br>
nextToken();<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
} else {<br>
addUnwrappedLine();<br>
@@ -854,9 +867,15 @@<br>
if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))<br>
--Line->Level;<br>
if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
- if (FormatTok->Tok.is(tok::kw_break))<br>
- parseStructuralElement(); // "break;" after "}" goes on the same line.<br>
+ if (FormatTok->Tok.is(tok::kw_break)) {<br>
+ // "break;" after "}" on its own line only for BS_Allman<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
+ parseStructuralElement();<br>
+ }<br>
}<br>
addUnwrappedLine();<br>
Line->Level = OldLineLevel;<br>
@@ -877,6 +896,8 @@<br>
if (FormatTok->Tok.is(tok::l_paren))<br>
parseParens();<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
+ addUnwrappedLine();<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
addUnwrappedLine();<br>
} else {<br>
@@ -973,7 +994,8 @@<br>
}<br>
}<br>
if (FormatTok->Tok.is(tok::l_brace)) {<br>
- if (Style.BreakBeforeBraces == FormatStyle::BS_Linux)<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
addUnwrappedLine();<br>
<br>
parseBlock(/*MustBeDeclaration=*/true);<br>
Index: unittests/Format/FormatTest.cpp<br>
===================================================================<br>
--- unittests/Format/FormatTest.cpp<br>
+++ unittests/Format/FormatTest.cpp<br>
@@ -5419,6 +5419,30 @@<br>
BreakBeforeBrace);<br>
}<br>
<br>
+TEST_F(FormatTest, AllmanBraceBreaking) {<br>
+ FormatStyle BreakBeforeBrace = getLLVMStyle();<br>
+ BreakBeforeBrace.BreakBeforeBraces = FormatStyle::BS_Allman;<br>
+ verifyFormat("namespace a\n"<br>
+ "{\n"<br>
+ "class A\n"<br>
+ "{\n"<br>
+ " void f()\n"<br>
+ " {\n"<br>
+ " if (true)\n"<br>
+ " {\n"<br>
+ " a();\n"<br>
+ " b();\n"<br>
+ " }\n"<br>
+ " }\n"<br>
+ " void g()\n"<br>
+ " {\n"<br>
+ " return;\n"<br>
+ " }\n"<br>
+ "}\n"<br>
+ "}",<br>
+ BreakBeforeBrace);<br>
+}<br>
+<br>
bool allStylesEqual(ArrayRef<FormatStyle> Styles) {<br>
for (size_t i = 1; i < Styles.size(); ++i)<br>
if (!(Styles[0] == Styles[i]))<br>
</blockquote></div><br></div>