gergap added inline comments.
================
Comment at: clang/unittests/Format/FormatTest.cpp:15045
+ // PAS_RIGHT
EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
" int const i = 1;\n"
----------------
gergap wrote:
> gergap wrote:
> > HazardyKnusperkeks wrote:
> > > I don't know why this is `EXPECT_EQ` instead of `verifyFormat`, but I
> > > know someone who will request that. :)
> > >
> > > You should change that here and use that for your following tests.
> > I don't know, because I'm not the author of that code.
> > But I can change it to verifyFormat if this is what you prefer.
> >
> The verifyFormat() call with one code parameter does not work with this test
> pattern, because the internal messUpCode function removes the newlines.
> The consecutive alignments are interrupted by newlines, which lead to
> different indent for each section. This breaks with messUpCode.
>
> However, there is a verifyFormat function with two code arguments, wich
> behaves similar to the existing EXPECT_EQ. This way it works.
> I go for this option now.
Unfortunately, verifyFormat does not work with this test pattern.
As you can see in line 80 below there is another mussUp(Code) call for the
Objective-C++ checks.
This again screws up the newlines and it does not help to use the verifyFormat
overload with two code arguments.
I think this is a general limitation. Clang-format distinguishes between
ACS_Consecutive and ACS_AcrossEmptyLines.
Using verifyFormat you cannot use empty lines, so you cannot test if
ACS_Consecutive does NOT align across empty lines.
IMO, it is better to keep the EXPECT_EQ tests. The other solution would be to
remove the empty lines from the test patterns.
```
68 void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
69 llvm::StringRef Code,
70 const FormatStyle &Style = getLLVMStyle()) {
71 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
72 EXPECT_EQ(Expected.str(), format(Expected, Style))
73 << "Expected code is not stable";
74 EXPECT_EQ(Expected.str(), format(Code, Style));
75 if (Style.Language == FormatStyle::LK_Cpp) {
76 // Objective-C++ is a superset of C++, so everything checked for C++
77 // needs to be checked for Objective-C++ as well.
78 FormatStyle ObjCStyle = Style;
79 ObjCStyle.Language = FormatStyle::LK_ObjC;
80 EXPECT_EQ(Expected.str(), format(test::messUp(Code), ObjCStyle));
81 }
82 }
```
================
Comment at: clang/unittests/Format/FormatTest.cpp:15068
Alignment));
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+ " int const i = 1;\n"
----------------
curdeius wrote:
> Is there a reason why we don't use `verifyFormat` in these tests?
same answer as above.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103245/new/
https://reviews.llvm.org/D103245
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
