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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Please regroup tests and add release notes.



================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1126
+
+  EXPECT_EQ(UnwrappedLines, Style.ShortNamespaceLines);
+}
----------------
This could be tested together with another test.


================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1129
+
+TEST_F(ShortNamespaceLinesTest,
+       ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {
----------------
In general, I think you can merge some test cases to match the current testing "style".


================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1141
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 0;
+
----------------
>From the tests one could imply that 0 means "never add comments", or "add comments for non empty namespaces". Please add a test with at least one line in the namespace.


================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1161
+TEST_F(ShortNamespaceLinesTest,
+       OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+  EXPECT_EQ("namespace LongNamespace {\n"
----------------
You should check that Style.ShortNamespaceLines is 1 here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587