[PATCH] clang-format: Added option for Allman style brace breaking

[Manuel Klimek]

Can you make sure that all changes are covered in the test? For example, I
see no 'break' statement in the test.

Thanks!
/Manuel


On Thu, Aug 1, 2013 at 3:17 PM, Frank Miller <fmiller at sjm.com> wrote:

> Allman style is the opposite of attach. Braces are always on their own
> line.
>
> http://llvm-reviews.chandlerc.com/D1257
>
> Files:
>   include/clang/Format/Format.h
>   lib/Format/Format.cpp
>   lib/Format/UnwrappedLineParser.cpp
>   unittests/Format/FormatTest.cpp
>
> Index: include/clang/Format/Format.h
> ===================================================================
> --- include/clang/Format/Format.h
> +++ include/clang/Format/Format.h
> @@ -163,7 +163,9 @@
>      /// class definitions.
>      BS_Linux,
>      /// Like \c Attach, but break before function definitions.
> -    BS_Stroustrup
> +    BS_Stroustrup,
> +    /// Always break before braces
> +    BS_Allman
>    };
>
>    /// \brief The brace breaking style to use.
> Index: lib/Format/Format.cpp
> ===================================================================
> --- lib/Format/Format.cpp
> +++ lib/Format/Format.cpp
> @@ -50,6 +50,7 @@
>      IO.enumCase(Value, "Attach", clang::format::FormatStyle::BS_Attach);
>      IO.enumCase(Value, "Linux", clang::format::FormatStyle::BS_Linux);
>      IO.enumCase(Value, "Stroustrup",
> clang::format::FormatStyle::BS_Stroustrup);
> +    IO.enumCase(Value, "Allman", clang::format::FormatStyle::BS_Allman);
>    }
>  };
>
> Index: lib/Format/UnwrappedLineParser.cpp
> ===================================================================
> --- lib/Format/UnwrappedLineParser.cpp
> +++ lib/Format/UnwrappedLineParser.cpp
> @@ -594,7 +594,8 @@
>          // FIXME: Figure out cases where this is not true, and add
> projections
>          // for them (the one we know is missing are lambdas).
>          if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||
> -            Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup)
> +            Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup ||
> +            Style.BreakBeforeBraces == FormatStyle::BS_Allman)
>            addUnwrappedLine();
>          parseBlock(/*MustBeDeclaration=*/false);
>          addUnwrappedLine();
> @@ -759,8 +760,13 @@
>      parseParens();
>    bool NeedsUnwrappedLine = false;
>    if (FormatTok->Tok.is(tok::l_brace)) {
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +      addUnwrappedLine();
>      parseBlock(/*MustBeDeclaration=*/false);
> -    NeedsUnwrappedLine = true;
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +      addUnwrappedLine();
> +    else
> +      NeedsUnwrappedLine = true;
>    } else {
>      addUnwrappedLine();
>      ++Line->Level;
> @@ -770,6 +776,8 @@
>    if (FormatTok->Tok.is(tok::kw_else)) {
>      nextToken();
>      if (FormatTok->Tok.is(tok::l_brace)) {
> +      if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +        addUnwrappedLine();
>        parseBlock(/*MustBeDeclaration=*/false);
>        addUnwrappedLine();
>      } else if (FormatTok->Tok.is(tok::kw_if)) {
> @@ -791,7 +799,8 @@
>    if (FormatTok->Tok.is(tok::identifier))
>      nextToken();
>    if (FormatTok->Tok.is(tok::l_brace)) {
> -    if (Style.BreakBeforeBraces == FormatStyle::BS_Linux)
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||
> +        Style.BreakBeforeBraces == FormatStyle::BS_Allman)
>        addUnwrappedLine();
>
>      bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||
> @@ -814,6 +823,8 @@
>    if (FormatTok->Tok.is(tok::l_paren))
>      parseParens();
>    if (FormatTok->Tok.is(tok::l_brace)) {
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +      addUnwrappedLine();
>      parseBlock(/*MustBeDeclaration=*/false);
>      addUnwrappedLine();
>    } else {
> @@ -828,6 +839,8 @@
>    assert(FormatTok->Tok.is(tok::kw_do) && "'do' expected");
>    nextToken();
>    if (FormatTok->Tok.is(tok::l_brace)) {
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +      addUnwrappedLine();
>      parseBlock(/*MustBeDeclaration=*/false);
>    } else {
>      addUnwrappedLine();
> @@ -854,9 +867,15 @@
>    if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
>      --Line->Level;
>    if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace))
> {
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +      addUnwrappedLine();
>      parseBlock(/*MustBeDeclaration=*/false);
> -    if (FormatTok->Tok.is(tok::kw_break))
> -      parseStructuralElement(); // "break;" after "}" goes on the same
> line.
> +    if (FormatTok->Tok.is(tok::kw_break)) {
> +      // "break;" after "}" on its own line only for BS_Allman
> +      if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +        addUnwrappedLine();
> +      parseStructuralElement();
> +    }
>    }
>    addUnwrappedLine();
>    Line->Level = OldLineLevel;
> @@ -877,6 +896,8 @@
>    if (FormatTok->Tok.is(tok::l_paren))
>      parseParens();
>    if (FormatTok->Tok.is(tok::l_brace)) {
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Allman)
> +      addUnwrappedLine();
>      parseBlock(/*MustBeDeclaration=*/false);
>      addUnwrappedLine();
>    } else {
> @@ -973,7 +994,8 @@
>      }
>    }
>    if (FormatTok->Tok.is(tok::l_brace)) {
> -    if (Style.BreakBeforeBraces == FormatStyle::BS_Linux)
> +    if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||
> +        Style.BreakBeforeBraces == FormatStyle::BS_Allman)
>        addUnwrappedLine();
>
>      parseBlock(/*MustBeDeclaration=*/true);
> Index: unittests/Format/FormatTest.cpp
> ===================================================================
> --- unittests/Format/FormatTest.cpp
> +++ unittests/Format/FormatTest.cpp
> @@ -5419,6 +5419,30 @@
>                 BreakBeforeBrace);
>  }
>
> +TEST_F(FormatTest, AllmanBraceBreaking) {
> +  FormatStyle BreakBeforeBrace = getLLVMStyle();
> +  BreakBeforeBrace.BreakBeforeBraces = FormatStyle::BS_Allman;
> +  verifyFormat("namespace a\n"
> +               "{\n"
> +               "class A\n"
> +               "{\n"
> +               "  void f()\n"
> +               "  {\n"
> +               "    if (true)\n"
> +               "    {\n"
> +               "      a();\n"
> +               "      b();\n"
> +               "    }\n"
> +               "  }\n"
> +               "  void g()\n"
> +               "  {\n"
> +               "    return;\n"
> +               "  }\n"
> +               "}\n"
> +               "}",
> +               BreakBeforeBrace);
> +}
> +
>  bool allStylesEqual(ArrayRef<FormatStyle> Styles) {
>    for (size_t i = 1; i < Styles.size(); ++i)
>      if (!(Styles[0] == Styles[i]))
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130801/4f8528de/attachment.html>