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

[Alexander Kornienko]

Hi klimek,

"//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.

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

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: lib/Format/BreakableToken.cpp
===================================================================
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -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"
@@ -164,15 +165,43 @@
                                            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,
                                unsigned ColumnLimit) const {
   return getCommentSplit(Line.substr(TailOffset), StartColumn + Prefix.size(),
                          ColumnLimit, Encoding);
 }
 
+void BreakableLineComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
+                                       Split Split, bool InPPDirective,
+                                       WhitespaceManager &Whitespaces) {
+  Whitespaces.breakToken(Tok, OriginalPrefix.size() + TailOffset + Split.first,
+                         Split.second, Postfix, Prefix, InPPDirective,
+                         StartColumn);
+}
+
+void
+BreakableLineComment::replaceWhitespaceBefore(unsigned LineIndex,
+                                              unsigned InPPDirective,
+                                              WhitespaceManager &Whitespaces) {
+  if (OriginalPrefix != Prefix) {
+    SourceLocation Loc =
+        Tok.getStartOfNonWhitespace().getLocWithOffset(OriginalPrefix.size());
+    Whitespaces.storeReplacement(SourceRange(Loc, Loc), " ");
+  }
+}
+
 BreakableBlockComment::BreakableBlockComment(
     const FormatStyle &Style, const FormatToken &Token, unsigned StartColumn,
     unsigned OriginalStartColumn, bool FirstInLine, encoding::Encoding Encoding)
Index: lib/Format/BreakableToken.h
===================================================================
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -116,6 +116,9 @@
 };
 
 class BreakableLineComment : public BreakableSingleLineToken {
+  // The prefix without an additional space if one was added.
+  StringRef OriginalPrefix;
+
 public:
   /// \brief Creates a breakable token for a line comment.
   ///
@@ -126,6 +129,11 @@
 
   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);
 };
 
 class BreakableBlockComment : public BreakableToken {
Index: lib/Format/WhitespaceManager.h
===================================================================
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -67,6 +67,9 @@
   /// \brief Returns all the \c Replacements created during formatting.
   const tooling::Replacements &generateReplacements();
 
+  /// \brief Stores \p Text as the replacement for the whitespace in \p Range.
+  void storeReplacement(const SourceRange &Range, StringRef Text);
+
 private:
   /// \brief Represents a change before a token, a break inside a token,
   /// or the layout of an unchanged token (or whitespace within).
@@ -149,8 +152,6 @@
   /// \brief Fill \c Replaces with the replacements for all effective changes.
   void generateChanges();
 
-  /// \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,
                              unsigned PreviousEndOfTokenColumn,
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -860,10 +860,15 @@
   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"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D949.1.patch
Type: text/x-patch
Size: 5531 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130610/b75876ea/attachment.bin>