Max_S added a comment.
As already described in the update I changed the option name and how it
behaves. It is now more in line with EmptyLineBeforeAccessModifier.
I hope this was the correct way to push the rewrite, since all line remarks are
now off.
================
Comment at: clang/include/clang/Format/Format.h:1957
+ /// Defines how many lines are put after access modifiers.
+ unsigned EmptyLinesAfterAccessModifier;
+
----------------
curdeius wrote:
> This option seems to be very different from `EmptyLineBeforeAccessModifier`.
> I don't mean in what it does, because this is analogical, but in the possible
> options.
> Wouldn't it be less surprising to have (at least some) similar options here
> and there?
> Is there any value in having more than one line after access modifiers?
> Couldn't that be achieved with Leave option?
> How do the two options work together?
>
> Also, the difference in singular vs. plural form of "Line(s)" in these two
> options is disconcerting.
> From the user perspective, it's error-prone to have two options that are at
> the same time so similar and so different.
I agree with you and changed it accordingly. I left out the option
`LogicalBlock`.
The interaction between the two options is quite minimal. I can add extra
tests, that would demonstrate this but I do not think that this is necessary.
The leave option would now applies MaxEmptyLinesToKeep in a correct way. See
also my remarks in >>! In D98237#2616621.
================
Comment at: clang/unittests/Format/FormatTest.cpp:9212
+ StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+ EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+ EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));
----------------
HazardyKnusperkeks wrote:
> Max_S wrote:
> > 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?
> I'm a fan of it. :)
> Especially with the demonstration that the string is still expanded.
I changed all to EXPECT_EQ since the failer output there is better. It shows
the variables names and not the arbitrary code in verfyFormat. Hope this is ok.
```
/home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:9202:
Failure
Expected: test2NL
Which is: "class Foo {\nprivate:\n\n int i;\n};"
To be equal to: format(test1NL)
Which is: "class Foo {\nprivate:\n int i;\n};"
With diff:
@@ -1,5 @@
class Foo {
private:
-
int i;
};
````
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
