• Address review comments

A newly added test seems to be failing:

/usr/local/google/home/krasimir/work/llvm/tools/clang/test/Driver/hexagon-hvx.c:71:25: error: expected string not found in input
// CHECK-HVXLENGTH-64B: "-target-feature" "+hvx{{.*}}" "-target-feature" "+hvx-length64b"

How is this even reviewed?

Great! I like the unit testing approach a lot!

ping

ping

Looks good!

• Change bool from Paren to Line and generalize it

ping

ping

+1! I totally agree with this!

Makes sense.

• Stable sort using declarations

This is how you could add a test in NamespaceEndCommentsFixerTest.cpp:

TEST_F(NamespaceEndCommentsFixerTest, FixesNamespaceCommentsInAllmanStyle) {
  FormatStyle AllmanStyle = getLLVMStyle();
  AllmanStyle.BreakBeforeBraces = FormatStyle::BS_Allman;
  EXPECT_EQ("namespace a\n"
            "{\n"
            "void f();\n"
            "void g();\n"
            "}// namespace a\n",
            fixNamespaceEndComments("namespace a\n"
                                    "{\n"
                                    "void f();\n"
                                    "void g();\n"
                                    "}\n",
                                    AllmanStyle));
}

Could you please move the test that adds a namespace end comment to unittests/Format/NamespaceEndCommentsFixerTest.cpp?

Looks good! Should I commit this for you?

Looks great! Just one more nit: please add a line checking the parsing of the new option similar to the line CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct); in unittests/Format/FormatTest.cpp.

Patch looks good, but I also would like to see it splited. I would suggest to first get the statement macro part in, which requires less code. Then we can put the namespace macros on top of that. I really like the generality of this approach and would want to also add support for class macros eventually.

Yes, I meant that it should at least be controlled by a flag. However, adding flags to clang-format requires careful consideration, we usually would accept options that are necessary for commonly used styles with style guides and with a community willing to support these options.

• Add the original test cases

I think this is ready for another round of reviews.

• Document FirstStateColumn @ getInitialState

• Remove raw string cache

• Fix a regression after preprocessor indentation

Great! Would you like me to commit this for you?

ping

Great!

• Rebase with master

I'll land this for you, as discussed offline. The best thing is to apply for commit rights after you have a few patches landed.

I think this is ready for another round of reviews.

• Actually add a test

• Address review comments

Thank you! I prefer this patch to be split in two. I am still not convinced about the extern part: some clients might prefer the other style.

@euhlmann: are you planning to commit this?

• Updated indent to IndentWidth
• Revised the rules for formatting the prefix and suffix

These look like they could be two separate patches: I like the typedef enum part, not convinced about the extern part. Also, please add unit tests.

Thank you! I understand this patch better now. Looks good from my side!

The test CursorAtStartOfFunction is segfaulting.

@MTC, do you have commit access or should we commit this for you?

Thank you! I think your suggestion is nice. I'll create a follow-up patch for that!

@djasper: No. The test case is a special constellation.