<div dir="ltr">On Mon, Jun 10, 2013 at 8:58 PM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi klimek,<br>
<br>
"//Test" becomes "// Test". This change is aimed to improve code<br>
readability and conformance to certain coding styles. If a comment starts with a<br>
non-alphanumeric character, the space isn't added, e.g. "//-*-c++-*-" stays<br>
unchanged.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D949" target="_blank">http://llvm-reviews.chandlerc.com/D949</a><br>
<br>
Files:<br>
lib/Format/BreakableToken.cpp<br>
lib/Format/BreakableToken.h<br>
lib/Format/WhitespaceManager.h<br>
unittests/Format/FormatTest.cpp<br>
<br>
Index: lib/Format/BreakableToken.cpp<br>
===================================================================<br>
--- lib/Format/BreakableToken.cpp<br>
+++ lib/Format/BreakableToken.cpp<br>
@@ -16,6 +16,7 @@<br>
#define DEBUG_TYPE "format-token-breaker"<br>
<br>
#include "BreakableToken.h"<br>
+#include "clang/Basic/CharInfo.h"<br>
#include "clang/Format/Format.h"<br>
#include "llvm/ADT/STLExtras.h"<br>
#include "llvm/Support/Debug.h"<br>
@@ -164,15 +165,43 @@<br>
encoding::Encoding Encoding)<br>
: BreakableSingleLineToken(Token, StartColumn,<br>
getLineCommentPrefix(Token.TokenText), "",<br>
- Encoding) {}<br>
+ Encoding) {<br>
+ OriginalPrefix = Prefix;<br>
+ if (Token.TokenText.size() > Prefix.size() &&<br>
+ isAlphanumeric(Token.TokenText[Prefix.size()])) {<br>
+ if (Prefix == "//")<br>
+ Prefix = "// ";<br>
+ else if (Prefix == "///")<br>
+ Prefix = "/// ";<br>
+ }<br>
+}<br>
<br>
BreakableToken::Split<br>
BreakableLineComment::getSplit(unsigned LineIndex, unsigned TailOffset,<br>
unsigned ColumnLimit) const {<br>
return getCommentSplit(Line.substr(TailOffset), StartColumn + Prefix.size(),<br>
ColumnLimit, Encoding);<br>
}<br>
<br>
+void BreakableLineComment::insertBreak(unsigned LineIndex, unsigned TailOffset,<br>
+ Split Split, bool InPPDirective,<br>
+ WhitespaceManager &Whitespaces) {<br>
+ Whitespaces.breakToken(Tok, OriginalPrefix.size() + TailOffset + Split.first,<br>
+ Split.second, Postfix, Prefix, InPPDirective,<br>
+ StartColumn);<br>
+}<br>
+<br>
+void<br>
+BreakableLineComment::replaceWhitespaceBefore(unsigned LineIndex,<br>
+ unsigned InPPDirective,<br>
+ WhitespaceManager &Whitespaces) {<br>
+ if (OriginalPrefix != Prefix) {<br>
+ SourceLocation Loc =<br>
+ Tok.getStartOfNonWhitespace().getLocWithOffset(OriginalPrefix.size());<br>
+ Whitespaces.storeReplacement(SourceRange(Loc, Loc), " ");<br>
+ }<br>
+}<br>
+<br>
BreakableBlockComment::BreakableBlockComment(<br>
const FormatStyle &Style, const FormatToken &Token, unsigned StartColumn,<br>
unsigned OriginalStartColumn, bool FirstInLine, encoding::Encoding Encoding)<br>
Index: lib/Format/BreakableToken.h<br>
===================================================================<br>
--- lib/Format/BreakableToken.h<br>
+++ lib/Format/BreakableToken.h<br>
@@ -116,6 +116,9 @@<br>
};<br>
<br>
class BreakableLineComment : public BreakableSingleLineToken {<br>
+ // The prefix without an additional space if one was added.<br>
+ StringRef OriginalPrefix;<br>
+<br>
public:<br>
/// \brief Creates a breakable token for a line comment.<br>
///<br>
@@ -126,6 +129,11 @@<br>
<br>
virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,<br>
unsigned ColumnLimit) const;<br>
+ virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,<br>
+ bool InPPDirective, WhitespaceManager &Whitespaces);<br>
+ virtual void replaceWhitespaceBefore(unsigned LineIndex,<br>
+ unsigned InPPDirective,<br>
+ WhitespaceManager &Whitespaces);<br>
};<br>
<br>
class BreakableBlockComment : public BreakableToken {<br>
Index: lib/Format/WhitespaceManager.h<br>
===================================================================<br>
--- lib/Format/WhitespaceManager.h<br>
+++ lib/Format/WhitespaceManager.h<br>
@@ -67,6 +67,9 @@<br>
/// \brief Returns all the \c Replacements created during formatting.<br>
const tooling::Replacements &generateReplacements();<br>
<br>
+ /// \brief Stores \p Text as the replacement for the whitespace in \p Range.<br>
+ void storeReplacement(const SourceRange &Range, StringRef Text);<br>
+<br></blockquote><div><br></div><div style>The plan is to only generate replacements in one place. I'd rename breakToken with replaceWhitespaceInToken and add the Newlines parameter there...</div><div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
private:<br>
/// \brief Represents a change before a token, a break inside a token,<br>
/// or the layout of an unchanged token (or whitespace within).<br>
@@ -149,8 +152,6 @@<br>
/// \brief Fill \c Replaces with the replacements for all effective changes.<br>
void generateChanges();<br>
<br>
- /// \brief Stores \p Text as the replacement for the whitespace in \p Range.<br>
- void storeReplacement(const SourceRange &Range, StringRef Text);<br>
std::string getNewLineText(unsigned NewLines, unsigned Spaces);<br>
std::string getNewLineText(unsigned NewLines, unsigned Spaces,<br>
unsigned PreviousEndOfTokenColumn,<br>
Index: unittests/Format/FormatTest.cpp<br>
===================================================================<br>
--- unittests/Format/FormatTest.cpp<br>
+++ unittests/Format/FormatTest.cpp<br>
@@ -860,10 +860,15 @@<br>
EXPECT_EQ("// Don't_touch_leading_whitespace",<br>
format("// Don't_touch_leading_whitespace",<br>
getLLVMStyleWithColumns(20)));<br>
- EXPECT_EQ(<br>
- "//Don't add leading\n"<br>
- "//whitespace",<br>
- format("//Don't add leading whitespace", getLLVMStyleWithColumns(20)));<br>
+ EXPECT_EQ("// Add leading\n"<br>
+ "// whitespace",<br>
+ format("//Add leading whitespace", getLLVMStyleWithColumns(20)));<br>
+ EXPECT_EQ("// whitespace", format("//whitespace", getLLVMStyle()));<br>
+ EXPECT_EQ("// Even if it makes the line exceed the column\n"<br>
+ "// limit",<br>
+ format("//Even if it makes the line exceed the column limit",<br>
+ getLLVMStyleWithColumns(51)));<br>
+ EXPECT_EQ("//--But not here", format("//--But not here", getLLVMStyle()));<br>
EXPECT_EQ("// A comment before\n"<br>
"// a macro\n"<br>
"// definition\n"<br>
</blockquote></div><br></div></div>