[PATCH] D87587: [clang-format][PR47290] Add ShortNamespaceLines format option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Although I would put it in one test, this is fine by me.



================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1201-1206
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 2u;
----------------
curdeius wrote:
> What I meant by "merging the tests together" is just writing:
> ```
> TEST_F(ShortNamespaceLinesTest,
>        MultipleUnwrappedLine) {
>   auto Style = getLLVMStyle();
>   Style.ShortNamespaceLines = 2u;
> 
>   EXPECT_EQ(...);
> 
>   EXPECT_EQ(...);
> }
> ```
> This will get rid of duplicated setup (even if small), and, what's more important, show the behaviour with the same style on different cases.
> 
> Or even, but I'm not forcing you to do so, having a single test case for all `ShortNamespaceLinesTest.*` tests. It groups the test for the same feature together.
I would even go further and put the 0, 1, and 2 in one function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87587/new/

https://reviews.llvm.org/D87587