[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: lib/Format/NamespaceEndCommentsFixer.h:24 +// Finds the namespace token corresponding to a closing namespace `}`, if that +// is to be formatted. I don't understand the "if that is to be formatted part". What do you mean? Repository: rC Clang https://reviews.llvm.org/D45373 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings
krasimir marked an inline comment as done. krasimir added inline comments. Comment at: lib/Format/NamespaceEndCommentsFixer.h:24 +// Finds the namespace token corresponding to a closing namespace `}`, if that +// is to be formatted. djasper wrote: > I don't understand the "if that is to be formatted part". What do you mean? I'm documenting the current behavior of the implementation, getting into more detail in the next paragraph of the comments: this returns `nullptr` if `Line` is not `Affected`, even if otherwise would have a namespace token. Repository: rC Clang https://reviews.llvm.org/D45373 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings
This revision was automatically updated to reflect the committed changes. krasimir marked an inline comment as done. Closed by commit rL330324: [clang-format] Don't remove empty lines before namespace endings (authored by krasimir, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45373?vs=141352&id=143081#toc Repository: rL LLVM https://reviews.llvm.org/D45373 Files: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp cfe/trunk/lib/Format/UnwrappedLineFormatter.h cfe/trunk/unittests/Format/FormatTest.cpp Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp === --- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp +++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp @@ -107,6 +107,7 @@ << llvm::toString(std::move(Err)) << "\n"; } } +} // namespace const FormatToken * getNamespaceToken(const AnnotatedLine *line, @@ -131,7 +132,6 @@ return nullptr; return NamespaceTok; } -} // namespace NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env, const FormatStyle &Style) Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.h === --- cfe/trunk/lib/Format/UnwrappedLineFormatter.h +++ cfe/trunk/lib/Format/UnwrappedLineFormatter.h @@ -49,8 +49,9 @@ /// \brief Add a new line and the required indent before the first Token /// of the \c UnwrappedLine if there was no structural parsing error. void formatFirstToken(const AnnotatedLine &Line, -const AnnotatedLine *PreviousLine, unsigned Indent, -unsigned NewlineIndent); +const AnnotatedLine *PreviousLine, +const SmallVectorImpl &Lines, +unsigned Indent, unsigned NewlineIndent); /// \brief Returns the column limit for a line, taking into account whether we /// need an escaped newline due to a continued preprocessor directive. Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp === --- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp +++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp @@ -7,6 +7,7 @@ // //===--===// +#include "NamespaceEndCommentsFixer.h" #include "UnwrappedLineFormatter.h" #include "WhitespaceManager.h" #include "llvm/Support/Debug.h" @@ -1050,8 +1051,7 @@ if (ShouldFormat && TheLine.Type != LT_Invalid) { if (!DryRun) { bool LastLine = Line->First->is(tok::eof); -formatFirstToken(TheLine, PreviousLine, - Indent, +formatFirstToken(TheLine, PreviousLine, Lines, Indent, LastLine ? LastStartColumn : NextStartColumn + Indent); } @@ -1095,7 +1095,7 @@ TheLine.LeadingEmptyLinesAffected); // Format the first token. if (ReformatLeadingWhitespace) - formatFirstToken(TheLine, PreviousLine, + formatFirstToken(TheLine, PreviousLine, Lines, TheLine.First->OriginalColumn, TheLine.First->OriginalColumn); else @@ -1117,10 +1117,10 @@ return Penalty; } -void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line, - const AnnotatedLine *PreviousLine, - unsigned Indent, - unsigned NewlineIndent) { +void UnwrappedLineFormatter::formatFirstToken( +const AnnotatedLine &Line, const AnnotatedLine *PreviousLine, +const SmallVectorImpl &Lines, unsigned Indent, +unsigned NewlineIndent) { FormatToken &RootToken = *Line.First; if (RootToken.is(tok::eof)) { unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u); @@ -1134,7 +1134,9 @@ // Remove empty lines before "}" where applicable. if (RootToken.is(tok::r_brace) && (!RootToken.Next || - (RootToken.Next->is(tok::semi) && !RootToken.Next->Next))) + (RootToken.Next->is(tok::semi) && !RootToken.Next->Next)) && + // Do not remove empty lines before namespace closing "}". + !getNamespaceToken(&Line, Lines)) Newlines = std::min(Newlines, 1u); // Remove empty lines at the start of nested blocks (lambdas/arrow functions) if (PreviousLine == nullptr && Line.Level > 0) Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h === --- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h +++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h @@ -21,6 +21,16 @@
[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings
krasimir created this revision. krasimir added a reviewer: djasper. Herald added subscribers: cfe-commits, klimek. This implements an alternative to r327861, namely preserving empty lines before namespace endings. Repository: rC Clang https://reviews.llvm.org/D45373 Files: lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/NamespaceEndCommentsFixer.h lib/Format/UnwrappedLineFormatter.cpp lib/Format/UnwrappedLineFormatter.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -278,19 +278,41 @@ "\n" "}")); - // FIXME: This is slightly inconsistent. + // Don't remove empty lines before namespace endings. FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle(); LLVMWithNoNamespaceFix.FixNamespaceComments = false; EXPECT_EQ("namespace {\n" "int i;\n" +"\n" "}", format("namespace {\n" "int i;\n" "\n" "}", LLVMWithNoNamespaceFix)); EXPECT_EQ("namespace {\n" "int i;\n" "}", +format("namespace {\n" + "int i;\n" + "}", LLVMWithNoNamespaceFix)); + EXPECT_EQ("namespace {\n" +"int i;\n" +"\n" +"};", +format("namespace {\n" + "int i;\n" + "\n" + "};", LLVMWithNoNamespaceFix)); + EXPECT_EQ("namespace {\n" +"int i;\n" +"};", +format("namespace {\n" + "int i;\n" + "};", LLVMWithNoNamespaceFix)); + EXPECT_EQ("namespace {\n" +"int i;\n" +"\n" +"}", format("namespace {\n" "int i;\n" "\n" Index: lib/Format/UnwrappedLineFormatter.h === --- lib/Format/UnwrappedLineFormatter.h +++ lib/Format/UnwrappedLineFormatter.h @@ -49,8 +49,9 @@ /// \brief Add a new line and the required indent before the first Token /// of the \c UnwrappedLine if there was no structural parsing error. void formatFirstToken(const AnnotatedLine &Line, -const AnnotatedLine *PreviousLine, unsigned Indent, -unsigned NewlineIndent); +const AnnotatedLine *PreviousLine, +const SmallVectorImpl &Lines, +unsigned Indent, unsigned NewlineIndent); /// \brief Returns the column limit for a line, taking into account whether we /// need an escaped newline due to a continued preprocessor directive. Index: lib/Format/UnwrappedLineFormatter.cpp === --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -7,6 +7,7 @@ // //===--===// +#include "NamespaceEndCommentsFixer.h" #include "UnwrappedLineFormatter.h" #include "WhitespaceManager.h" #include "llvm/Support/Debug.h" @@ -1050,8 +1051,7 @@ if (ShouldFormat && TheLine.Type != LT_Invalid) { if (!DryRun) { bool LastLine = Line->First->is(tok::eof); -formatFirstToken(TheLine, PreviousLine, - Indent, +formatFirstToken(TheLine, PreviousLine, Lines, Indent, LastLine ? LastStartColumn : NextStartColumn + Indent); } @@ -1095,7 +1095,7 @@ TheLine.LeadingEmptyLinesAffected); // Format the first token. if (ReformatLeadingWhitespace) - formatFirstToken(TheLine, PreviousLine, + formatFirstToken(TheLine, PreviousLine, Lines, TheLine.First->OriginalColumn, TheLine.First->OriginalColumn); else @@ -1117,10 +1117,10 @@ return Penalty; } -void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line, - const AnnotatedLine *PreviousLine, - unsigned Indent, - unsigned NewlineIndent) { +void UnwrappedLineFormatter::formatFirstToken( +const AnnotatedLine &Line, const AnnotatedLine *PreviousLine, +const SmallVectorImpl &Lines, unsigned Indent, +unsigned NewlineIndent) { FormatToken &RootToken = *Line.First; if (RootToken.is(tok::eof)) { unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u); @@ -1134,7 +1134,9 @@ // Remove empty lines before "}" where applicable. if (RootToken.is(tok::r_brace) && (!RootToken.Next || - (RootToken.Next->is(tok::semi) && !RootToken.Next->Next))) + (RootToken.Nex