MyDeveloperDay added inline comments.
================
Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:93
+ EXPECT_EQ(Tokens.size(), 10u) << Tokens;
+ EXPECT_TOKEN(Tokens[6], tok::star, TT_PointerOrReference);
}
----------------
HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > Can you add a verifyFormat test that shows what you want? as well
> I think the annotator test is sufficient. Because it's just about annotating
> the token, formatting is secondary (and dependent on style - these tests are
> already in place).
I know where you are coming from, but actually if it wasn't for the
`enable_if<>{} && ...` example in the unit tests then we'd have missed the &&
case and caused a regression. (that was ONLY covered by the small example code
snippet)
Whilst I believe the TokenAnnotator tests are correct to have as well, I think
we should be adding formatting examples just to ensure someone doesn't breaking
the formatting rule that this depends on. I always follow the Beyoncé rule...
"If you like it you should have put a test on it!"
So If you don't want anyone to break your `*` placement after the `}` then I
see no harm in adding a single verifyFormat(), but while you are at it please
also test to ensure the Left/Middle/Right works with your example as you might
expect. (just incase there are some rules about that, that could interfere with
your setting)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127873/new/
https://reviews.llvm.org/D127873
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
