kuzkry updated this revision to Diff 313218.
kuzkry retitled this revision from "[clang-format][PR47290] Add 
MaxUnwrappedLinesForShortNamespace format option" to "[clang-format][PR47290] 
Add ShortNamespaceLines format option".
kuzkry added a comment.

Renamed MaxUnwrappedLinesForShortNamespace to ShortNamespaceLines (thank you 
@MyDeveloperDay for the idea and @curdeius for mentioning clang-tidy). I 
haven't changed the description of this option so you might wanna complain on 
that. Tbh. English isn't my native language so I'll take your tips if you wanna 
help simplify it.

Luckily formatting is magically fixed and hopefully CI check's gonna be 
satisfied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===================================================================
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -816,7 +816,7 @@
                                     "}\n"));
 }
 
-TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+TEST_F(NamespaceEndCommentsFixerTest, AddsEndCommentForNamespacesAroundMacros) {
   // Conditional blocks around are fine
   EXPECT_EQ("namespace A {\n"
             "#if 1\n"
@@ -1116,6 +1116,92 @@
                                     "}\n"
                                     "}\n"));
 }
+
+using ShortNamespaceLinesTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(ShortNamespaceLinesTest, DefaultsToOneUnwrappedLine) {
+  constexpr auto UnwrappedLines = 1u;
+  auto const Style = getLLVMStyle();
+
+  EXPECT_EQ(UnwrappedLines, Style.ShortNamespaceLines);
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 0;
+
+  EXPECT_EQ("namespace OneLinerNamespace {}\n",
+            fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       ZeroUnwrappedLines_DoesNotAddEndCommentForNonOneLinerNamespace) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 0;
+
+  EXPECT_EQ("namespace NonOneLinerNamespace {\n"
+            "}\n",
+            fixNamespaceEndComments("namespace NonOneLinerNamespace {\n"
+                                    "}\n",
+                                    Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       OneUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+  EXPECT_EQ("namespace ShortNamespace {\n"
+            "int i;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace ShortNamespace {\n"
+                                    "int i;\n"
+                                    "}\n"));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+  EXPECT_EQ("namespace LongNamespace {\n"
+            "int i;\n"
+            "int j;\n"
+            "}// namespace LongNamespace\n",
+            fixNamespaceEndComments("namespace LongNamespace {\n"
+                                    "int i;\n"
+                                    "int j;\n"
+                                    "}\n"));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       MultipleUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 2;
+
+  EXPECT_EQ("namespace ShortNamespace {\n"
+            "int i;\n"
+            "int j;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace ShortNamespace {\n"
+                                    "int i;\n"
+                                    "int j;\n"
+                                    "}\n",
+                                    Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+       MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 2;
+
+  EXPECT_EQ("namespace LongNamespace {\n"
+            "int i;\n"
+            "int j;\n"
+            "int k;\n"
+            "}// namespace LongNamespace\n",
+            fixNamespaceEndComments("namespace LongNamespace {\n"
+                                    "int i;\n"
+                                    "int j;\n"
+                                    "int k;\n"
+                                    "}\n",
+                                    Style));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -22,10 +22,6 @@
 namespace format {
 
 namespace {
-// The maximal number of unwrapped lines that a short namespace spans.
-// Short namespaces don't need an end comment.
-static const int kShortNamespaceMaxLines = 1;
-
 // Computes the name of a namespace given the namespace token.
 // Returns "" for anonymous namespace.
 std::string computeName(const FormatToken *NamespaceTok) {
@@ -280,7 +276,7 @@
     const std::string EndCommentText =
         computeEndCommentText(NamespaceName, AddNewline, NamespaceTok);
     if (!hasEndComment(EndCommentPrevTok)) {
-      bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
+      bool isShort = I - StartLineIndex <= Style.ShortNamespaceLines + 1;
       if (!isShort)
         addEndComment(EndCommentPrevTok, EndCommentText, SourceMgr, &Fixes);
     } else if (!validEndComment(EndCommentPrevTok, NamespaceName,
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -573,6 +573,7 @@
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
     IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
+    IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
     IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
     IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
     IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
@@ -935,6 +936,7 @@
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
+  LLVMStyle.ShortNamespaceLines = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1653,12 +1653,14 @@
   /// not use this in config files, etc. Use at your own risk.
   bool ExperimentalAutoDetectBinPacking;
 
-  /// If ``true``, clang-format adds missing namespace end comments and
-  /// fixes invalid existing ones.
+  /// If ``true``, clang-format adds missing namespace end comments for
+  /// short namespaces and fixes invalid existing ones. Short ones are
+  /// controlled by "ShortNamespaceLines".
   /// \code
   ///    true:                                  false:
   ///    namespace a {                  vs.     namespace a {
   ///    foo();                                 foo();
+  ///    bar();                                 bar();
   ///    } // namespace a                       }
   /// \endcode
   bool FixNamespaceComments;
@@ -2330,6 +2332,27 @@
   bool ReflowComments;
   // clang-format on
 
+  /// The maximal number of unwrapped lines that a short namespace spans.
+  /// Defaults to 1.
+  ///
+  /// This determines the maximum length of short namespaces by counting
+  /// unwrapped lines (i.e. containing neither opening nor closing
+  /// namespace brace) and makes "FixNamespaceComments" omit adding
+  /// end comments for those.
+  /// \code
+  ///    ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+  ///    namespace a {                      namespace a {
+  ///      int foo;                           int foo;
+  ///    }                                  } // namespace a
+  ///
+  ///    ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+  ///    namespace b {                      namespace b {
+  ///      int foo;                           int foo;
+  ///      int bar;                           int bar;
+  ///    } // namespace b                   } // namespace b
+  /// \endcode
+  unsigned ShortNamespaceLines;
+
   /// If ``true``, clang-format will sort ``#includes``.
   /// \code
   ///    false:                                 true:
@@ -2795,6 +2818,7 @@
            MacroBlockBegin == R.MacroBlockBegin &&
            MacroBlockEnd == R.MacroBlockEnd &&
            MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
+           ShortNamespaceLines == R.ShortNamespaceLines &&
            NamespaceIndentation == R.NamespaceIndentation &&
            NamespaceMacros == R.NamespaceMacros &&
            ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1886,14 +1886,16 @@
   not use this in config files, etc. Use at your own risk.
 
 **FixNamespaceComments** (``bool``)
-  If ``true``, clang-format adds missing namespace end comments and
-  fixes invalid existing ones.
+  If ``true``, clang-format adds missing namespace end comments for
+  short namespaces and fixes invalid existing ones. Short ones are
+  controlled by "ShortNamespaceLines".
 
   .. code-block:: c++
 
      true:                                  false:
      namespace a {                  vs.     namespace a {
      foo();                                 foo();
+     bar();                                 bar();
      } // namespace a                       }
 
 **ForEachMacros** (``std::vector<std::string>``)
@@ -2713,6 +2715,28 @@
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
       * information */
 
+**ShortNamespaceLines** (``unsigned``)
+  The maximal number of unwrapped lines that a short namespace spans.
+  Defaults to 1.
+
+  This determines the maximum length of short namespaces by counting
+  unwrapped lines (i.e. containing neither opening nor closing
+  namespace brace) and makes "FixNamespaceComments" omit adding
+  end comments for those.
+
+  .. code-block:: c++
+
+     ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+     namespace a {                      namespace a {
+       int foo;                           int foo;
+     }                                  } // namespace a
+
+     ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+     namespace b {                      namespace b {
+       int foo;                           int foo;
+       int bar;                           int bar;
+     } // namespace b                   } // namespace b
+
 **SortIncludes** (``bool``)
   If ``true``, clang-format will sort ``#includes``.
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to