Max_S added a comment. In D98237#2616193 <https://reviews.llvm.org/D98237#2616193>, @MyDeveloperDay wrote:
> Just out of interest and we are supposed to ask for this but can you point to > public style guide that uses this style. (actually I don't mind if other > formatting tools have this capability and you highlight it, like astyle or > editorConfig etc) > > From my perspective this seems like a reasonable suggestion. (even if I'm > unlikely to use it myself ;-)) I can not specify an other formater or a style guide that uses this style. The problem is that clang-format is rather destructive at this point. Other formatters like astyle or the one from CLion keep empty lines after an access modifier. The behavior of clang-format before this patch was to remove all empty lines after an access modifier. The option `MaxEmptyLinesToKeep` is currently ignored at this point. After thinking a little bit about this: If the removal of all lines after an access modifier is rather a bug than a feature, then the change: if (PreviousLine && PreviousLine->First->isAccessSpecifier() && (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline)) - Newlines = std::min(1u, Newlines); + Newlines = std::max(1u, Newlines); if (Newlines) would solve this problem. ================ Comment at: clang/unittests/Format/FormatTest.cpp:9205 + "};"; + EXPECT_EQ(test1NL, format(test0NL)); + verifyFormat(test1NL); ---------------- MyDeveloperDay wrote: > why can't you just verifyFormat them all? Yes. I will change this in the next update. ================ Comment at: clang/unittests/Format/FormatTest.cpp:9212 + StyleWithLine.EmptyLinesAfterAccessModifier = 1u; + EXPECT_EQ(test2NL, format(test0NL, StyleWithLine)); + EXPECT_EQ(test2NL, format(test1NL, StyleWithLine)); ---------------- MyDeveloperDay wrote: > yeah I'm not a fan of this like this... sorry... just write the test out in > long form, when it goes wrong I don't have to be a compiler to understand > what is going wrong I can just see it. I can change this, but the current output of the tests is (I forced the error): ``` /<path>/llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure Expected: Expected.str() Which is: "class Foo {\nprivate:\n\n int i;\n};" To be equal to: format(Expected, Style) Which is: "class Foo {\nprivate:\n int i;\n};" With diff: @@ -1,5 @@ class Foo { private: - int i; }; ``` Which is actually human readable in this case. Shall I still change it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98237/new/ https://reviews.llvm.org/D98237 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits