This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ca52815fb3c: [clang-format][PR47290] Add
ShortNamespaceLines format option (authored by kuzkry, committed by
HazardyKnusperkeks).
Changed prior to commit:
https://reviews.llvm.org/D87587?vs=323289&id=327246#toc
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/docs/ReleaseNotes.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,75 @@
"}\n"
"}\n"));
}
+
+using ShortNamespaceLinesTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(ShortNamespaceLinesTest, ZeroUnwrappedLines) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 0u;
+
+ EXPECT_EQ("namespace OneLinerNamespace {}\n",
+ fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "}\n",
+ Style));
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "}\n",
+ Style));
+}
+
+TEST_F(ShortNamespaceLinesTest, OneUnwrappedLine) {
+ constexpr auto DefaultUnwrappedLines = 1u;
+ auto const Style = getLLVMStyle();
+
+ EXPECT_EQ(DefaultUnwrappedLines, Style.ShortNamespaceLines);
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n"));
+ 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) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 2u;
+
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n",
+ Style));
+ 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) {
@@ -283,7 +279,7 @@
computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
Style.SpacesInLineCommentPrefix.Minimum);
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
@@ -642,6 +642,7 @@
IO.mapOptional("PointerAlignment", Style.PointerAlignment);
IO.mapOptional("RawStringFormats", Style.RawStringFormats);
IO.mapOptional("ReflowComments", Style.ReflowComments);
+ IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
IO.mapOptional("SortIncludes", Style.SortIncludes);
IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
@@ -1006,6 +1007,7 @@
LLVMStyle.ObjCSpaceAfterProperty = false;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
+ LLVMStyle.ShortNamespaceLines = 1;
LLVMStyle.SpacesBeforeTrailingComments = 1;
LLVMStyle.Standard = FormatStyle::LS_Latest;
LLVMStyle.UseCRLF = false;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1964,12 +1964,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;
@@ -2644,6 +2646,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;
+
/// Include sorting options.
enum SortIncludesOptions : unsigned char {
/// Includes are never sorted.
@@ -3224,6 +3247,7 @@
R.PenaltyBreakTemplateDeclaration &&
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
+ ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -203,6 +203,9 @@
- Option ``IndentAccessModifiers`` has been added to be able to give access
modifiers their own indentation level inside records.
+- Option ``ShortNamespaceLines`` has been added to give better control
+ over ``FixNamespaceComments`` when determining a namespace length.
+
libclang
--------
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2210,14 +2210,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>``)
@@ -2367,7 +2369,7 @@
the record members, respecting the ``AccessModifierOffset``. Record
members are indented one level below the record.
When ``true``, access modifiers get their own indentation level. As a
- consequence, record members are indented 2 levels below the record,
+ consequence, record members are always indented 2 levels below the record,
regardless of the access modifier presence. Value of the
``AccessModifierOffset`` is ignored.
@@ -3040,43 +3042,72 @@
/* 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** (``SortIncludesOptions``)
Controls if and how clang-format will sort ``#includes``.
+ If ``Never``, includes are never sorted.
+ If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
+ insensitive fashion.
+ If ``CaseSensitive``, includes are sorted in an alphabetical or case
+ sensitive fashion.
- Possible Values:
+ Possible values:
- * ``SI_Never`` (in configuration ``Never``)
+ * ``SI_Never`` (in configuration: ``Never``)
Includes are never sorted.
.. code-block:: c++
- #include "B/A.h"
- #include "A/B.h"
- #include "a/b.h"
- #include "A/b.h"
- #include "B/a.h"
+ #include "B/A.h"
+ #include "A/B.h"
+ #include "a/b.h"
+ #include "A/b.h"
+ #include "B/a.h"
- * ``SI_CaseInsensitive`` (in configuration ``CaseInsensitive``)
+ * ``SI_CaseInsensitive`` (in configuration: ``CaseInsensitive``)
Includes are sorted in an ASCIIbetical or case insensitive fashion.
.. code-block:: c++
- #include "A/B.h"
- #include "A/b.h"
- #include "B/A.h"
- #include "B/a.h"
- #include "a/b.h"
+ #include "A/B.h"
+ #include "A/b.h"
+ #include "B/A.h"
+ #include "B/a.h"
+ #include "a/b.h"
- * ``SI_CaseSensitive`` (in configuration ``CaseSensitive``)
+ * ``SI_CaseSensitive`` (in configuration: ``CaseSensitive``)
Includes are sorted in an alphabetical or case sensitive fashion.
.. code-block:: c++
- #include "A/B.h"
- #include "A/b.h"
- #include "a/b.h"
- #include "B/A.h"
- #include "B/a.h"
+ #include "A/B.h"
+ #include "A/b.h"
+ #include "a/b.h"
+ #include "B/A.h"
+ #include "B/a.h"
+
+
**SortJavaStaticImport** (``SortJavaStaticImportOptions``)
When sorting Java imports, by default static imports are placed before
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
