[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[Max Sagebaum via Phabricator via cfe-commits]

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