Add WebKit brace style configuration option.

[lifted]

Summary

Add brace style BS_WebKit as described on https://www.webkit.org/coding/coding-style.html:

• Function definitions: place each brace on its own line.
• Other braces: place the open brace on the line preceding the code block; place the close brace on its own line.

Set brace style used in getWebKitStyle() to the newly added BS_WebKit.

[djasper]

Looks good. Do you have commit access?

include/clang/Format/Format.h

180 ↗

(On Diff #31518)

Remove the second comma (and regenerate the docs)

[klimek]

Hm, the referenced style guide doesn't say anything about namespaces. I also couldn't find anything in the qt source repo (just took a quick look though). Do you have an example of why Qt projects shouldn't just use BS_Linux?

[lifted]

Hm, the referenced style guide doesn't say anything about namespaces.

You're right. Futhermore, it's hard to find any namespaces in qt repos at all - Qt uses a macro for conditional compilation with namespaces.

I also couldn't find anything in the qt source repo (just took a quick look though).

Hm, I've just browsed through the qtcore and seen no evidence that qt uses attached braces everywhere but after classes and function. Qt use of style is inconsistent (sometimes even within a single file), and few examples I've looked at actually use something close to Mozilla style - they break on structs, enums, classes, and functions (http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qchar.h?id=13c16bbd06e0034210ba2164f1be208c49a6e3a7).

My bad, I read the Qt Brace style description and decided that it could fit my current project style (we break after functions, and some people also break after classes). But it turned out that Qt style is not as well defined as I thought, so I'm out of luck.

I believe it's better to abandon this patch.

I hope I will find some time to introduce extra configuration options to override pre-defined brace styles, as it was discussed long time ago.

[lifted]

Hooray, I've found the style I need - it's WebKit
https://www.webkit.org/coding/coding-style.html

So I believe there are other people waiting for the same style as me - https://code.google.com/p/chromium/issues/detail?id=454958

How do you feel about renaming BS_Qt -> BS_WebKit and reverting the changes in UnwrappedLineParser.cpp?

[klimek]

Webkit doesn't break for classes:

class MyClass {
    ...
};

[lifted]

In D11837#220394, @klimek wrote:

Webkit doesn't break for classes:

That's why we need to revert changes in UnwrappedLineParser.cpp.
The funny thing with WebKit style is that we don't need to add any logic to deal with it, any style different from existing ones is WebKit :)

It's actually ok for me to not break after classes - only couple of people in our project do this. I was just trying to find a style that is both ubiquitous and close enough to our code base.

[klimek]

Ah, ok. Interestingly, our current Webkit style uses BS_Stroustrup, which is incorrect for } else {. So yep, implementing BS_Webkit sounds like a win.

[lifted]

Removed Qt brace style, added WebKit brace style.

Changed default brace style in the WebKit configuration to BS_WebKit.

[klimek]

Can you also fix the current code in getWebKitStyle?

[lifted]

In D11837#220408, @klimek wrote:

Can you also fix the current code in getWebKitStyle?

Yep, I've already done it. I'll include this information into the diff description.

[klimek]

Ah, awesome, looks good then. Do you have commit access or need one of us to land it?

[lifted]

I had commit access earlier, but I haven't tried it for long time. I'll ask for help if there are any problems.

cfe/trunk/docs/ClangFormatStyleOptions.rst

* ``BS_Attach`` (in configuration: ``Attach``)
     Always attach braces to surrounding context.
   * ``BS_Linux`` (in configuration: ``Linux``)
     Like ``Attach``, but break before braces on function, namespace and
     class definitions.
   * ``BS_Mozilla`` (in configuration: ``Mozilla``)
     Like ``Attach``, but break before braces on enum, function, and record
     definitions.
   * ``BS_Stroustrup`` (in configuration: ``Stroustrup``)
-    Like ``Attach``, but break before function definitions, and 'else'.
+    Like ``Attach``, but break before function definitions, 'catch', and 'else'.
   * ``BS_Allman`` (in configuration: ``Allman``)
     Always break before braces.
   * ``BS_GNU`` (in configuration: ``GNU``)
     Always break before braces and add an extra level of indentation to
     braces of control statements, not to those of class, function
     or other definitions.
+  * ``BS_WebKit`` (in configuration: ``WebKit``)
+    Like ``Attach``, but break before functions.
 
 
 **BreakBeforeTernaryOperators** (``bool``)
   If ``true``, ternary operators will be placed after line breaks.
 
 **BreakConstructorInitializersBeforeComma** (``bool``)
   Always break constructor initializers before commas and align
   the commas with the colon.

cfe/trunk/include/clang/Format/Format.h

enum BraceBreakingStyle {
     /// Always attach braces to surrounding context.
     BS_Attach,
     /// Like \c Attach, but break before braces on function, namespace and
     /// class definitions.
     BS_Linux,
     /// Like ``Attach``, but break before braces on enum, function, and record
     /// definitions.
     BS_Mozilla,
-    /// Like \c Attach, but break before function definitions, and 'else'.
+    /// Like \c Attach, but break before function definitions, 'catch', and 'else'.
     BS_Stroustrup,
     /// Always break before braces.
     BS_Allman,
     /// Always break before braces and add an extra level of indentation to
     /// braces of control statements, not to those of class, function
     /// or other definitions.
-    BS_GNU
+    BS_GNU,
+    /// Like ``Attach``, but break before functions.
+    BS_WebKit
   };
 
   /// \brief The brace breaking style to use.
   BraceBreakingStyle BreakBeforeBraces;
 
   /// \brief If \c true, ternary operators will be placed after line breaks.
   bool BreakBeforeTernaryOperators;
 

cfe/trunk/lib/Format/Format.cpp

 template <> struct ScalarEnumerationTraits<FormatStyle::BraceBreakingStyle> {
   static void enumeration(IO &IO, FormatStyle::BraceBreakingStyle &Value) {
     IO.enumCase(Value, "Attach", FormatStyle::BS_Attach);
     IO.enumCase(Value, "Linux", FormatStyle::BS_Linux);
     IO.enumCase(Value, "Mozilla", FormatStyle::BS_Mozilla);
     IO.enumCase(Value, "Stroustrup", FormatStyle::BS_Stroustrup);
     IO.enumCase(Value, "Allman", FormatStyle::BS_Allman);
     IO.enumCase(Value, "GNU", FormatStyle::BS_GNU);
+    IO.enumCase(Value, "WebKit", FormatStyle::BS_WebKit);
   }
 };
 
 template <> struct ScalarEnumerationTraits<FormatStyle::DefinitionReturnTypeBreakingStyle> {
   static void enumeration(IO &IO, FormatStyle::DefinitionReturnTypeBreakingStyle &Value) {
     IO.enumCase(Value, "None", FormatStyle::DRTBS_None);
     IO.enumCase(Value, "All", FormatStyle::DRTBS_All);
     IO.enumCase(Value, "TopLevel", FormatStyle::DRTBS_TopLevel);

 
 FormatStyle getWebKitStyle() {
   FormatStyle Style = getLLVMStyle();
   Style.AccessModifierOffset = -4;
   Style.AlignAfterOpenBracket = false;
   Style.AlignOperands = false;
   Style.AlignTrailingComments = false;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
-  Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+  Style.BreakBeforeBraces = FormatStyle::BS_WebKit;
   Style.BreakConstructorInitializersBeforeComma = true;
   Style.Cpp11BracedListStyle = false;
   Style.ColumnLimit = 0;
   Style.IndentWidth = 4;
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   Style.ObjCBlockIndentWidth = 4;
   Style.ObjCSpaceAfterProperty = true;
   Style.PointerAlignment = FormatStyle::PAS_Left;

cfe/trunk/unittests/Format/FormatTest.cpp

verifyFormat("try {\n"
                "  g();\n"
                "} catch (B b) MACRO(x) {\n"
                "  g();\n"
                "}");
 }
 
 TEST_F(FormatTest, FormatTryCatchBraceStyles) {
   FormatStyle Style = getLLVMStyle();
-  Style.BreakBeforeBraces = FormatStyle::BS_Attach;
+  for (auto BraceStyle :
+       {FormatStyle::BS_Attach, FormatStyle::BS_Mozilla, FormatStyle::BS_WebKit}) {
+    Style.BreakBeforeBraces = BraceStyle;
     verifyFormat("try {\n"
                  "  // something\n"
                  "} catch (...) {\n"
                  "  // something\n"
                  "}",
                  Style);
+  }
   Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
   verifyFormat("try {\n"
                "  // something\n"
                "}\n"
                "catch (...) {\n"
                "  // something\n"
                "}",
                Style);
TEST_F(FormatTest, GNUBraceBreaking) {
   verifyFormat("void foobar() { int i = 5; }\n"
                "#ifdef _DEBUG\n"
                "void bar() {}\n"
                "#else\n"
                "void bar() { foobar(); }\n"
                "#endif",
                GNUBraceStyle);
 }
+
+TEST_F(FormatTest, WebKitBraceBreaking) {
+  FormatStyle WebKitBraceStyle = getLLVMStyle();
+  WebKitBraceStyle.BreakBeforeBraces = FormatStyle::BS_WebKit;
+  verifyFormat("namespace a {\n"
+               "class A {\n"
+               "  void f()\n"
+               "  {\n"
+               "    if (true) {\n"
+               "      a();\n"
+               "      b();\n"
+               "    }\n"
+               "  }\n"
+               "  void g() { return; }\n"
+               "};\n"
+               "enum E {\n"
+               "  A,\n"
+               "  // foo\n"
+               "  B,\n"
+               "  C\n"
+               "};\n"
+               "struct B {\n"
+               "  int x;\n"
+               "};\n"
+               "}\n",
+               WebKitBraceStyle);
+  verifyFormat("struct S {\n"
+               "  int Type;\n"
+               "  union {\n"
+               "    int x;\n"
+               "    double y;\n"
+               "  } Value;\n"
+               "  class C {\n"
+               "    MyFavoriteType Value;\n"
+               "  } Class;\n"
+               "};\n",
+               WebKitBraceStyle);
+}
+
 TEST_F(FormatTest, CatchExceptionReferenceBinding) {
   verifyFormat("void f() {\n"
                "  try {\n"
                "  } catch (const Exception &e) {\n"
                "  }\n"
                "}\n",
                getLLVMStyle());
 }
CHECK_PARSE("BreakBeforeBraces: Linux", BreakBeforeBraces,
               FormatStyle::BS_Linux);
   CHECK_PARSE("BreakBeforeBraces: Mozilla", BreakBeforeBraces,
               FormatStyle::BS_Mozilla);
   CHECK_PARSE("BreakBeforeBraces: Stroustrup", BreakBeforeBraces,
               FormatStyle::BS_Stroustrup);
   CHECK_PARSE("BreakBeforeBraces: Allman", BreakBeforeBraces,
               FormatStyle::BS_Allman);
   CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
+  CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces, FormatStyle::BS_WebKit);
 
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
               AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: All",
               AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_All);
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: TopLevel",
               AlwaysBreakAfterDefinitionReturnType,