Insert a space at the start of a line comment in case it starts with an alphanumeric character.

Summary:
"//Test" becomes "// Test". This change is aimed to improve code
readability and conformance to certain coding styles. If a comment starts with a
non-alphanumeric character, the space isn't added, e.g. "//-*-c++-*-" stays
unchanged.

Reviewers: klimek

Reviewed By: klimek

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D949

llvm-svn: 183750
This commit is contained in:
Alexander Kornienko 2013-06-11 16:01:49 +00:00
parent 447d2d12f0
commit 555efc36d0
5 changed files with 93 additions and 48 deletions

View File

@ -16,6 +16,7 @@
#define DEBUG_TYPE "format-token-breaker"
#include "BreakableToken.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Format/Format.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Debug.h"
@ -118,15 +119,6 @@ unsigned BreakableSingleLineToken::getLineLengthAfterSplit(
encoding::getCodePointCount(Line.substr(Offset, Length), Encoding);
}
void BreakableSingleLineToken::insertBreak(unsigned LineIndex,
unsigned TailOffset, Split Split,
bool InPPDirective,
WhitespaceManager &Whitespaces) {
Whitespaces.breakToken(Tok, Prefix.size() + TailOffset + Split.first,
Split.second, Postfix, Prefix, InPPDirective,
StartColumn);
}
BreakableSingleLineToken::BreakableSingleLineToken(const FormatToken &Tok,
unsigned StartColumn,
StringRef Prefix,
@ -151,6 +143,15 @@ BreakableStringLiteral::getSplit(unsigned LineIndex, unsigned TailOffset,
Encoding);
}
void BreakableStringLiteral::insertBreak(unsigned LineIndex,
unsigned TailOffset, Split Split,
bool InPPDirective,
WhitespaceManager &Whitespaces) {
Whitespaces.replaceWhitespaceInToken(
Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
Prefix, InPPDirective, 1, StartColumn);
}
static StringRef getLineCommentPrefix(StringRef Comment) {
const char *KnownPrefixes[] = { "/// ", "///", "// ", "//" };
for (size_t i = 0, e = llvm::array_lengthof(KnownPrefixes); i != e; ++i)
@ -164,7 +165,16 @@ BreakableLineComment::BreakableLineComment(const FormatToken &Token,
encoding::Encoding Encoding)
: BreakableSingleLineToken(Token, StartColumn,
getLineCommentPrefix(Token.TokenText), "",
Encoding) {}
Encoding) {
OriginalPrefix = Prefix;
if (Token.TokenText.size() > Prefix.size() &&
isAlphanumeric(Token.TokenText[Prefix.size()])) {
if (Prefix == "//")
Prefix = "// ";
else if (Prefix == "///")
Prefix = "/// ";
}
}
BreakableToken::Split
BreakableLineComment::getSplit(unsigned LineIndex, unsigned TailOffset,
@ -173,6 +183,24 @@ BreakableLineComment::getSplit(unsigned LineIndex, unsigned TailOffset,
ColumnLimit, Encoding);
}
void BreakableLineComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
Split Split, bool InPPDirective,
WhitespaceManager &Whitespaces) {
Whitespaces.replaceWhitespaceInToken(
Tok, OriginalPrefix.size() + TailOffset + Split.first, Split.second,
Postfix, Prefix, InPPDirective, 1, StartColumn);
}
void
BreakableLineComment::replaceWhitespaceBefore(unsigned LineIndex,
unsigned InPPDirective,
WhitespaceManager &Whitespaces) {
if (OriginalPrefix != Prefix) {
Whitespaces.replaceWhitespaceInToken(Tok, OriginalPrefix.size(), 0, "", "",
false, 0, 1);
}
}
BreakableBlockComment::BreakableBlockComment(
const FormatStyle &Style, const FormatToken &Token, unsigned StartColumn,
unsigned OriginalStartColumn, bool FirstInLine, encoding::Encoding Encoding)
@ -299,8 +327,9 @@ void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
Text.data() - Tok.TokenText.data() + Split.first;
unsigned CharsToRemove = Split.second;
assert(IndentAtLineBreak >= Decoration.size());
Whitespaces.breakToken(Tok, BreakOffsetInToken, CharsToRemove, "", Prefix,
InPPDirective, IndentAtLineBreak - Decoration.size());
Whitespaces.replaceWhitespaceInToken(Tok, BreakOffsetInToken, CharsToRemove,
"", Prefix, InPPDirective, 1,
IndentAtLineBreak - Decoration.size());
}
void
@ -331,9 +360,9 @@ BreakableBlockComment::replaceWhitespaceBefore(unsigned LineIndex,
Lines[LineIndex].data() - Tok.TokenText.data() -
LeadingWhitespace[LineIndex];
assert(StartOfLineColumn[LineIndex] >= Prefix.size());
Whitespaces.breakToken(
Whitespaces.replaceWhitespaceInToken(
Tok, WhitespaceOffsetInToken, LeadingWhitespace[LineIndex], "", Prefix,
InPPDirective, StartOfLineColumn[LineIndex] - Prefix.size());
InPPDirective, 1, StartOfLineColumn[LineIndex] - Prefix.size());
}
unsigned

View File

@ -84,8 +84,6 @@ public:
virtual unsigned getLineLengthAfterSplit(unsigned LineIndex,
unsigned TailOffset,
StringRef::size_type Length) const;
virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
bool InPPDirective, WhitespaceManager &Whitespaces);
protected:
BreakableSingleLineToken(const FormatToken &Tok, unsigned StartColumn,
@ -113,6 +111,9 @@ public:
virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
unsigned ColumnLimit) const;
virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
bool InPPDirective,
WhitespaceManager &Whitespaces);
};
class BreakableLineComment : public BreakableSingleLineToken {
@ -126,6 +127,15 @@ public:
virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
unsigned ColumnLimit) const;
virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
bool InPPDirective, WhitespaceManager &Whitespaces);
virtual void replaceWhitespaceBefore(unsigned LineIndex,
unsigned InPPDirective,
WhitespaceManager &Whitespaces);
private:
// The prefix without an additional space if one was added.
StringRef OriginalPrefix;
};
class BreakableBlockComment : public BreakableToken {

View File

@ -56,16 +56,15 @@ void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
InPPDirective && !Tok.IsFirst));
}
void WhitespaceManager::breakToken(const FormatToken &Tok, unsigned Offset,
unsigned ReplaceChars,
StringRef PreviousPostfix,
StringRef CurrentPrefix, bool InPPDirective,
unsigned Spaces) {
void WhitespaceManager::replaceWhitespaceInToken(
const FormatToken &Tok, unsigned Offset, unsigned ReplaceChars,
StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective,
unsigned Newlines, unsigned Spaces) {
Changes.push_back(Change(
true, SourceRange(Tok.getStartOfNonWhitespace().getLocWithOffset(Offset),
Tok.getStartOfNonWhitespace().getLocWithOffset(
Offset + ReplaceChars)),
Spaces, Spaces, 1, PreviousPostfix, CurrentPrefix,
Spaces, Spaces, Newlines, PreviousPostfix, CurrentPrefix,
// FIXME: Unify token adjustment, so we don't split it between
// BreakableToken and the WhitespaceManager. That would also allow us to
// correctly store a tok::TokenKind instead of rolling our own enum.
@ -214,10 +213,10 @@ void WhitespaceManager::generateChanges() {
std::string ReplacementText =
C.PreviousLinePostfix +
(C.ContinuesPPDirective
? getNewLineText(C.NewlinesBefore, C.Spaces,
? getNewlineText(C.NewlinesBefore, C.Spaces,
C.PreviousEndOfTokenColumn,
C.EscapedNewlineColumn)
: getNewLineText(C.NewlinesBefore, C.Spaces)) +
: getNewlineText(C.NewlinesBefore, C.Spaces)) +
C.CurrentLinePrefix;
storeReplacement(C.OriginalWhitespaceRange, ReplacementText);
}
@ -237,26 +236,26 @@ void WhitespaceManager::storeReplacement(const SourceRange &Range,
SourceMgr, CharSourceRange::getCharRange(Range), Text));
}
std::string WhitespaceManager::getNewLineText(unsigned NewLines,
std::string WhitespaceManager::getNewlineText(unsigned Newlines,
unsigned Spaces) {
return std::string(NewLines, '\n') + getIndentText(Spaces);
return std::string(Newlines, '\n') + getIndentText(Spaces);
}
std::string WhitespaceManager::getNewLineText(unsigned NewLines,
std::string WhitespaceManager::getNewlineText(unsigned Newlines,
unsigned Spaces,
unsigned PreviousEndOfTokenColumn,
unsigned EscapedNewlineColumn) {
std::string NewLineText;
if (NewLines > 0) {
std::string NewlineText;
if (Newlines > 0) {
unsigned Offset =
std::min<int>(EscapedNewlineColumn - 1, PreviousEndOfTokenColumn);
for (unsigned i = 0; i < NewLines; ++i) {
NewLineText += std::string(EscapedNewlineColumn - Offset - 1, ' ');
NewLineText += "\\\n";
for (unsigned i = 0; i < Newlines; ++i) {
NewlineText += std::string(EscapedNewlineColumn - Offset - 1, ' ');
NewlineText += "\\\n";
Offset = 0;
}
}
return NewLineText + getIndentText(Spaces);
return NewlineText + getIndentText(Spaces);
}
std::string WhitespaceManager::getIndentText(unsigned Spaces) {

View File

@ -52,17 +52,19 @@ public:
/// was not called.
void addUntouchableToken(const FormatToken &Tok, bool InPPDirective);
/// \brief Inserts a line break into the middle of a token.
/// \brief Inserts or replaces whitespace in the middle of a token.
///
/// Will break at \p Offset inside \p Tok, putting \p PreviousPostfix before
/// the line break and \p CurrentPrefix before the rest of the token starts in
/// the next line.
/// Inserts \p PreviousPostfix, \p Newlines, \p Spaces and \p CurrentPrefix
/// (in this order) at \p Offset inside \p Tok, replacing \p ReplaceChars
/// characters.
///
/// \p InPPDirective and \p Spaces are used to generate the correct line
/// break.
void breakToken(const FormatToken &Tok, unsigned Offset,
unsigned ReplaceChars, StringRef PreviousPostfix,
StringRef CurrentPrefix, bool InPPDirective, unsigned Spaces);
/// When \p InPPDirective is true, escaped newlines are inserted. \p Spaces is
/// used to align backslashes correctly.
void replaceWhitespaceInToken(const FormatToken &Tok, unsigned Offset,
unsigned ReplaceChars,
StringRef PreviousPostfix,
StringRef CurrentPrefix, bool InPPDirective,
unsigned Newlines, unsigned Spaces);
/// \brief Returns all the \c Replacements created during formatting.
const tooling::Replacements &generateReplacements();
@ -151,8 +153,8 @@ private:
/// \brief Stores \p Text as the replacement for the whitespace in \p Range.
void storeReplacement(const SourceRange &Range, StringRef Text);
std::string getNewLineText(unsigned NewLines, unsigned Spaces);
std::string getNewLineText(unsigned NewLines, unsigned Spaces,
std::string getNewlineText(unsigned Newlines, unsigned Spaces);
std::string getNewlineText(unsigned Newlines, unsigned Spaces,
unsigned PreviousEndOfTokenColumn,
unsigned EscapedNewlineColumn);
std::string getIndentText(unsigned Spaces);

View File

@ -860,10 +860,15 @@ TEST_F(FormatTest, SplitsLongCxxComments) {
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)));
EXPECT_EQ("// Add leading\n"
"// whitespace",
format("//Add leading whitespace", getLLVMStyleWithColumns(20)));
EXPECT_EQ("// whitespace", format("//whitespace", getLLVMStyle()));
EXPECT_EQ("// Even if it makes the line exceed the column\n"
"// limit",
format("//Even if it makes the line exceed the column limit",
getLLVMStyleWithColumns(51)));
EXPECT_EQ("//--But not here", format("//--But not here", getLLVMStyle()));
EXPECT_EQ("// A comment before\n"
"// a macro\n"
"// definition\n"