Hi djasper,
1. When splitting one-line block comment, use indentation and *s.
2. Remove trailing whitespace from all lines of a comment, not only the ones
being splitted.
3. Add backslashes for all lines if a comment is used insed a preprocessor
directive.
http://llvm-reviews.chandlerc.com/D557
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -151,8 +151,10 @@
if (Tok.Children.empty() && !isTrailingComment(Tok))
alignComments();
- if (Tok.Type == TT_BlockComment)
- indentBlockComment(Tok, Spaces, false);
+ if (Tok.Type == TT_BlockComment) {
+ int StartColumn = NewLines > 0 ? Spaces : WhitespaceStartColumn + Spaces;
+ indentBlockComment(Tok, Spaces, StartColumn, false);
+ }
storeReplacement(Tok.FormatTok, getNewLineText(NewLines, Spaces));
}
@@ -164,8 +166,10 @@
/// \c Newlines == 0.
void replacePPWhitespace(const AnnotatedToken &Tok, unsigned NewLines,
unsigned Spaces, unsigned WhitespaceStartColumn) {
- if (Tok.Type == TT_BlockComment)
- indentBlockComment(Tok, Spaces, true);
+ if (Tok.Type == TT_BlockComment) {
+ int StartColumn = NewLines > 0 ? Spaces : WhitespaceStartColumn + Spaces;
+ indentBlockComment(Tok, Spaces, StartColumn, true);
+ }
storeReplacement(Tok.FormatTok,
getNewLineText(NewLines, Spaces, WhitespaceStartColumn));
@@ -203,13 +207,11 @@
/// \brief Finds a common prefix of lines of a block comment to properly
/// indent (and possibly decorate with '*'s) added lines.
///
- /// The first line is ignored (it's special and starts with /*).
- /// When there are less than three lines, we don't have enough information, so
- /// better use no prefix.
+ /// The first line is ignored (it's special and starts with /*). The number of
+ /// lines should be more than one.
static StringRef findCommentLinesPrefix(ArrayRef<StringRef> Lines,
const char *PrefixChars = " *") {
- if (Lines.size() < 3)
- return "";
+ assert(Lines.size() > 1);
StringRef Prefix(Lines[1].data(), Lines[1].find_first_not_of(PrefixChars));
for (size_t i = 2; i < Lines.size(); ++i) {
for (size_t j = 0; j < Prefix.size() && j < Lines[i].size(); ++j) {
@@ -226,16 +228,12 @@
size_t StartColumn, StringRef LinePrefix,
bool InPPDirective, bool CommentHasMoreLines,
const char *WhiteSpaceChars = " ") {
- size_t ColumnLimit =
- Style.ColumnLimit - LinePrefix.size() - (InPPDirective ? 2 : 0);
-
- if (Line.size() <= ColumnLimit)
- return;
-
+ size_t ColumnLimit = Style.ColumnLimit - (InPPDirective ? 2 : 0);
const char *TokenStart = SourceMgr.getCharacterData(Tok.Tok.getLocation());
- while (Line.rtrim().size() > ColumnLimit) {
+ while (Line.rtrim().size() + StartColumn > ColumnLimit) {
// Try to break at the last whitespace before the column limit.
- size_t SpacePos = Line.find_last_of(WhiteSpaceChars, ColumnLimit + 1);
+ size_t SpacePos =
+ Line.find_last_of(WhiteSpaceChars, ColumnLimit - StartColumn + 1);
if (SpacePos == StringRef::npos) {
// Try to find any whitespace in the line.
SpacePos = Line.find_first_of(WhiteSpaceChars);
@@ -247,26 +245,30 @@
StringRef RemainingLine = Line.substr(SpacePos).ltrim();
if (RemainingLine.empty())
break;
+
+ if (RemainingLine == "*/" && LinePrefix.endswith("* "))
+ LinePrefix = LinePrefix.substr(0, LinePrefix.size() - 2);
+
Line = RemainingLine;
size_t ReplaceChars = Line.begin() - NextCut.end();
breakToken(Tok, NextCut.end() - TokenStart, ReplaceChars, "", LinePrefix,
InPPDirective, 0,
- NextCut.size() + LinePrefix.size() + StartColumn);
- StartColumn = 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() + LinePrefix.size());
+ TrimmedLine.size() + StartColumn);
}
}
void indentBlockComment(const AnnotatedToken &Tok, int Indent,
- bool InPPDirective) {
+ int StartColumn, bool InPPDirective) {
const SourceLocation TokenLoc = Tok.FormatTok.Tok.getLocation();
const int CurrentIndent = SourceMgr.getSpellingColumnNumber(TokenLoc) - 1;
const int IndentDelta = Indent - CurrentIndent;
@@ -299,22 +301,28 @@
}
// Split long lines in comments.
- const StringRef CurrentPrefix = findCommentLinesPrefix(Lines);
- size_t PrefixSize = CurrentPrefix.size();
- std::string NewPrefix =
- (IndentDelta < 0) ? CurrentPrefix.substr(-IndentDelta).str()
- : std::string(IndentDelta, ' ') + CurrentPrefix.str();
-
- if (CurrentPrefix.endswith("*")) {
- NewPrefix += " ";
- ++PrefixSize;
+ size_t PrefixSize = 0;
+ std::string NewPrefix;
+ if (Lines.size() > 1) {
+ StringRef CurrentPrefix = findCommentLinesPrefix(Lines);
+ PrefixSize = CurrentPrefix.size();
+ NewPrefix = (IndentDelta < 0)
+ ? CurrentPrefix.substr(-IndentDelta).str()
+ : std::string(IndentDelta, ' ') + CurrentPrefix.str();
+ if (CurrentPrefix.endswith("*")) {
+ NewPrefix += " ";
+ ++PrefixSize;
+ }
+ } else if (Tok.Parent == 0) {
+ NewPrefix = std::string(StartColumn, ' ') + " * ";
}
+ StartColumn += 2;
for (size_t i = 0; i < Lines.size(); ++i) {
- StringRef Line = (i == 0) ? Lines[i] : Lines[i].substr(PrefixSize);
- size_t StartColumn = (i == 0) ? CurrentIndent : 0;
+ StringRef Line = Lines[i].substr(i == 0 ? 2 : PrefixSize);
splitLineInComment(Tok.FormatTok, Line, StartColumn, NewPrefix,
InPPDirective, i != Lines.size() - 1);
+ StartColumn = NewPrefix.size();
}
}
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -667,11 +667,14 @@
TEST_F(FormatTest, SplitsLongLinesInComments) {
EXPECT_EQ("/* This is a long\n"
- "comment that doesn't\n"
- "fit on one line. */",
+ " * comment that\n"
+ " * doesn't\n"
+ " * fit on one line.\n"
+ " */",
format("/* "
"This is a long "
- "comment that doesn't "
+ "comment that "
+ "doesn't "
"fit on one line. */",
getLLVMStyleWithColumns(20)));
EXPECT_EQ("/*\n"
@@ -690,7 +693,7 @@
" * doesn't fit on\n"
" * one line.\n"
" */",
- format("/*\n"
+ format("/* \n"
" * This is a long "
" comment that "
" doesn't fit on "
@@ -761,8 +764,8 @@
TEST_F(FormatTest, SplitsLongLinesInCommentsInPreprocessor) {
EXPECT_EQ("#define X \\\n"
- // FIXME: Backslash should be added here.
- " /*\n"
+ " /* \\\n"
+ " Test \\\n"
" Macro comment \\\n"
" with a long \\\n"
" line \\\n"
@@ -773,19 +776,31 @@
" A + B",
format("#define X \\\n"
" /*\n"
+ " Test\n"
" Macro comment with a long line\n"
" */ \\\n"
" A + B",
getLLVMStyleWithColumns(20)));
EXPECT_EQ("#define X \\\n"
" /* Macro comment \\\n"
- // FIXME: Indent comment continuations when the comment is a first
- // token in a line.
- "with a long line \\\n"
+ " with a long \\\n"
+ // FIXME: We should look at the length of the last line of the token
+ // instead of the full token's length.
+ //" line */ \\\n"
+ " line */\\\n"
+ " A + B",
+ format("#define X \\\n"
+ " /* Macro comment with a long\n"
+ " line */ \\\n"
+ " A + B",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("#define X \\\n"
+ " /* Macro comment \\\n"
+ " * with a long \\\n"
// FIXME: We should look at the length of the last line of the token
// instead of the full token's length.
- //"*/ \\\n"
- "*/\\\n"
+ //" * line */ \\\n"
+ " * line */\\\n"
" A + B",
format("#define X \\\n"
" /* Macro comment with a long line */ \\\n"
@@ -2627,12 +2642,12 @@
EXPECT_EQ("/* */ /* */ /* */\n/* */ /* */ /* */",
format("/* *//* */ /* */\n/* *//* */ /* */"));
EXPECT_EQ("/* */ a /* */ b;", format(" /* */ a/* */ b;"));
- EXPECT_EQ("#define A /* */\\\n"
+ EXPECT_EQ("#define A /*123*/\\\n"
" b\n"
"/* */\n"
"someCall(\n"
" parameter);",
- format("#define A /* */ b\n"
+ format("#define A /*123*/ b\n"
"/* */\n"
"someCall(parameter);",
getLLVMStyleWithColumns(15)));
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
