r178133 - Split line comments
Alexander Kornienko
alexfh at google.com
Wed Mar 27 04:52:19 PDT 2013
Author: alexfh
Date: Wed Mar 27 06:52:18 2013
New Revision: 178133
URL: http://llvm.org/viewvc/llvm-project?rev=178133&view=rev
Log:
Split line comments
Summary:
Split line comments that exceed column limit + fixed leading whitespace
handling when splitting block comments.
Reviewers: djasper, klimek
Reviewed By: djasper
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D577
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=178133&r1=178132&r2=178133&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Mar 27 06:52:18 2013
@@ -23,6 +23,7 @@
#include "clang/Format/Format.h"
#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Debug.h"
#include <queue>
@@ -103,6 +104,12 @@ static unsigned getLengthToMatchingParen
return End->TotalLength - Tok.TotalLength + 1;
}
+static size_t
+calculateColumnLimit(const FormatStyle &Style, bool InPPDirective) {
+ // In preprocessor directives reserve two chars for trailing " \"
+ return Style.ColumnLimit - (InPPDirective ? 2 : 0);
+}
+
/// \brief Manages the whitespaces around tokens and their replacements.
///
/// This includes special handling for certain constructs, e.g. the alignment of
@@ -120,31 +127,32 @@ public:
if (NewLines >= 2)
alignComments();
+ SourceLocation TokenLoc = Tok.FormatTok.Tok.getLocation();
+ bool LineExceedsColumnLimit = Spaces + WhitespaceStartColumn +
+ Tok.FormatTok.TokenLength > Style.ColumnLimit;
+
// Align line comments if they are trailing or if they continue other
// trailing comments.
if (isTrailingComment(Tok)) {
// Remove the comment's trailing whitespace.
if (Tok.FormatTok.Tok.getLength() != Tok.FormatTok.TokenLength)
Replaces.insert(tooling::Replacement(
- SourceMgr, Tok.FormatTok.Tok.getLocation().getLocWithOffset(
- Tok.FormatTok.TokenLength),
+ SourceMgr, TokenLoc.getLocWithOffset(Tok.FormatTok.TokenLength),
Tok.FormatTok.Tok.getLength() - Tok.FormatTok.TokenLength, ""));
// Align comment with other comments.
- if (Tok.Parent != NULL || !Comments.empty()) {
- if (Style.ColumnLimit >=
- Spaces + WhitespaceStartColumn + Tok.FormatTok.TokenLength) {
- StoredComment Comment;
- Comment.Tok = Tok.FormatTok;
- Comment.Spaces = Spaces;
- Comment.NewLines = NewLines;
- Comment.MinColumn =
- NewLines > 0 ? Spaces : WhitespaceStartColumn + Spaces;
- Comment.MaxColumn = Style.ColumnLimit - Tok.FormatTok.TokenLength;
- Comment.Untouchable = false;
- Comments.push_back(Comment);
- return;
- }
+ if ((Tok.Parent != NULL || !Comments.empty()) &&
+ !LineExceedsColumnLimit) {
+ StoredComment Comment;
+ Comment.Tok = Tok.FormatTok;
+ Comment.Spaces = Spaces;
+ Comment.NewLines = NewLines;
+ Comment.MinColumn =
+ NewLines > 0 ? Spaces : WhitespaceStartColumn + Spaces;
+ Comment.MaxColumn = Style.ColumnLimit - Tok.FormatTok.TokenLength;
+ Comment.Untouchable = false;
+ Comments.push_back(Comment);
+ return;
}
}
@@ -152,8 +160,19 @@ public:
if (Tok.Children.empty() && !isTrailingComment(Tok))
alignComments();
- if (Tok.Type == TT_BlockComment)
+ if (Tok.Type == TT_BlockComment) {
indentBlockComment(Tok, Spaces, WhitespaceStartColumn, NewLines, false);
+ } else if (Tok.Type == TT_LineComment && LineExceedsColumnLimit) {
+ StringRef Line(SourceMgr.getCharacterData(TokenLoc),
+ Tok.FormatTok.TokenLength);
+ int StartColumn = Spaces + (NewLines == 0 ? WhitespaceStartColumn : 0);
+ StringRef Prefix = getLineCommentPrefix(Line);
+ std::string NewPrefix = std::string(StartColumn, ' ') + Prefix.str();
+ splitLineInComment(Tok.FormatTok, Line.substr(Prefix.size()),
+ StartColumn + Prefix.size(), NewPrefix,
+ /*InPPDirective=*/ false,
+ /*CommentHasMoreLines=*/ false);
+ }
storeReplacement(Tok.FormatTok, getNewLineText(NewLines, Spaces));
}
@@ -209,6 +228,14 @@ public:
}
private:
+ static StringRef getLineCommentPrefix(StringRef Comment) {
+ const char *KnownPrefixes[] = { "/// ", "///", "// ", "//" };
+ for (size_t i = 0; i < llvm::array_lengthof(KnownPrefixes); ++i)
+ if (Comment.startswith(KnownPrefixes[i]))
+ return KnownPrefixes[i];
+ return "";
+ }
+
/// \brief Finds a common prefix of lines of a block comment to properly
/// indent (and possibly decorate with '*'s) added lines.
///
@@ -229,13 +256,38 @@ private:
return Prefix;
}
+ /// \brief Splits one line in a line or block comment, if it doesn't fit to
+ /// provided column limit. Removes trailing whitespace in each line.
+ ///
+ /// \param Line points to the line contents without leading // or /*.
+ ///
+ /// \param StartColumn is the column where the first character of Line will be
+ /// located after formatting.
+ ///
+ /// \param LinePrefix is inserted after each line break.
+ ///
+ /// When \param InPPDirective is true, each line break will be preceded by a
+ /// backslash in the last column to make line breaks inside the comment
+ /// visually consistent with line breaks outside the comment. This only makes
+ /// sense for block comments.
+ ///
+ /// When \param CommentHasMoreLines is false, no line breaks/trailing
+ /// backslashes will be inserted after it.
void splitLineInComment(const FormatToken &Tok, StringRef Line,
size_t StartColumn, StringRef LinePrefix,
bool InPPDirective, bool CommentHasMoreLines,
const char *WhiteSpaceChars = " ") {
- size_t ColumnLimit = Style.ColumnLimit - (InPPDirective ? 2 : 0);
+ size_t ColumnLimit = calculateColumnLimit(Style, InPPDirective);
const char *TokenStart = SourceMgr.getCharacterData(Tok.Tok.getLocation());
- while (Line.rtrim().size() + StartColumn > ColumnLimit) {
+
+ StringRef TrimmedLine = Line.rtrim();
+ int TrailingSpaceLength = Line.size() - TrimmedLine.size();
+
+ // Don't touch leading whitespace.
+ Line = TrimmedLine.ltrim();
+ StartColumn += TrimmedLine.size() - Line.size();
+
+ while (Line.size() + StartColumn > ColumnLimit) {
// Try to break at the last whitespace before the column limit.
size_t SpacePos =
Line.find_last_of(WhiteSpaceChars, ColumnLimit - StartColumn + 1);
@@ -258,24 +310,25 @@ private:
size_t ReplaceChars = Line.begin() - NextCut.end();
breakToken(Tok, NextCut.end() - TokenStart, ReplaceChars, "", LinePrefix,
- InPPDirective, 0,
- NextCut.size() + StartColumn);
+ InPPDirective, 0, NextCut.size() + StartColumn);
StartColumn = LinePrefix.size();
}
- StringRef TrimmedLine = Line.rtrim();
- if (TrimmedLine != Line || (InPPDirective && CommentHasMoreLines)) {
- // Remove trailing whitespace/insert backslash.
- breakToken(Tok, TrimmedLine.end() - TokenStart,
- Line.size() - TrimmedLine.size() + 1, "", "", InPPDirective, 0,
- TrimmedLine.size() + StartColumn);
+ if (TrailingSpaceLength > 0 || (InPPDirective && CommentHasMoreLines)) {
+ // Remove trailing whitespace/insert backslash. + 1 is for \n
+ breakToken(Tok, Line.end() - TokenStart, TrailingSpaceLength + 1, "", "",
+ InPPDirective, 0, Line.size() + StartColumn);
}
}
+ /// \brief Changes indentation of all lines in a block comment by Indent,
+ /// removes trailing whitespace from each line, splits lines that end up
+ /// exceeding the column limit.
void indentBlockComment(const AnnotatedToken &Tok, int Indent,
int WhitespaceStartColumn, int NewLines,
bool InPPDirective) {
- int StartColumn = NewLines > 0 ? Indent : WhitespaceStartColumn + Indent;
+ assert(Tok.Type == TT_BlockComment);
+ int StartColumn = Indent + (NewLines == 0 ? WhitespaceStartColumn : 0);
const SourceLocation TokenLoc = Tok.FormatTok.Tok.getLocation();
const int CurrentIndent = SourceMgr.getSpellingColumnNumber(TokenLoc) - 1;
const int IndentDelta = Indent - CurrentIndent;
@@ -308,17 +361,17 @@ private:
}
// Split long lines in comments.
- size_t PrefixSize = 0;
+ size_t OldPrefixSize = 0;
std::string NewPrefix;
if (Lines.size() > 1) {
StringRef CurrentPrefix = findCommentLinesPrefix(Lines);
- PrefixSize = CurrentPrefix.size();
+ OldPrefixSize = CurrentPrefix.size();
NewPrefix = (IndentDelta < 0)
? CurrentPrefix.substr(-IndentDelta).str()
: std::string(IndentDelta, ' ') + CurrentPrefix.str();
if (CurrentPrefix.endswith("*")) {
NewPrefix += " ";
- ++PrefixSize;
+ ++OldPrefixSize;
}
} else if (Tok.Parent == 0) {
NewPrefix = std::string(StartColumn, ' ') + " * ";
@@ -326,7 +379,7 @@ private:
StartColumn += 2;
for (size_t i = 0; i < Lines.size(); ++i) {
- StringRef Line = Lines[i].substr(i == 0 ? 2 : PrefixSize);
+ StringRef Line = Lines[i].substr(i == 0 ? 2 : OldPrefixSize);
splitLineInComment(Tok.FormatTok, Line, StartColumn, NewPrefix,
InPPDirective, i != Lines.size() - 1);
StartColumn = NewPrefix.size();
@@ -974,7 +1027,7 @@ private:
}
unsigned getColumnLimit() {
- return Style.ColumnLimit - (Line.InPPDirective ? 2 : 0);
+ return calculateColumnLimit(Style, Line.InPPDirective);
}
/// \brief An edge in the solution space from \c Previous->State to \c State,
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=178133&r1=178132&r2=178133&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Mar 27 06:52:18 2013
@@ -681,6 +681,26 @@ TEST_F(FormatTest, AlignsMultiLineCommen
" */"));
}
+TEST_F(FormatTest, SplitsLongCxxComments) {
+ EXPECT_EQ("// A comment that\n"
+ "// doesn't fit on\n"
+ "// one line",
+ format("// A comment that doesn't fit on one line",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("if (true) // A comment that\n"
+ " // doesn't fit on\n"
+ " // one line",
+ format("if (true) // A comment that doesn't fit on one line ",
+ getLLVMStyleWithColumns(30)));
+ EXPECT_EQ("// Don't_touch_leading_whitespace",
+ format("// Don't_touch_leading_whitespace",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ(
+ "//Don't add leading\n"
+ "//whitespace",
+ format("//Don't add leading whitespace", getLLVMStyleWithColumns(20)));
+}
+
TEST_F(FormatTest, SplitsLongLinesInComments) {
EXPECT_EQ("/* This is a long\n"
" * comment that\n"
@@ -727,10 +747,10 @@ TEST_F(FormatTest, SplitsLongLinesInComm
" */",
getLLVMStyleWithColumns(20)));
EXPECT_EQ("/*\n"
- " * This_comment_can_not_be_broken_into_lines\n"
+ " * This_comment_can_not_be_broken_into_lines\n"
" */",
format("/*\n"
- " * This_comment_can_not_be_broken_into_lines\n"
+ " * This_comment_can_not_be_broken_into_lines\n"
" */",
getLLVMStyleWithColumns(20)));
EXPECT_EQ("{\n"
@@ -1166,13 +1186,13 @@ TEST_F(FormatTest, HandlePreprocessorDir
"#define A( \\\n"
" A, B)\n"
"#include \"b.h\"\n"
- "// some comment\n",
+ "// somecomment\n",
format(" // some comment\n"
" #include \"a.h\"\n"
"#define A(A,\\\n"
" B)\n"
" #include \"b.h\"\n"
- " // some comment\n",
+ " // somecomment\n",
getLLVMStyleWithColumns(13)));
}
@@ -2442,7 +2462,7 @@ TEST_F(FormatTest, IncorrectCodeMissingS
TEST_F(FormatTest, IndentationWithinColumnLimitNotPossible) {
verifyFormat("int aaaaaaaa =\n"
- " // Overly long comment\n"
+ " // Overlylongcomment\n"
" b;",
getLLVMStyleWithColumns(20));
verifyFormat("function(\n"
More information about the cfe-commits
mailing list