[PATCH] Split long lines in multi-line comments.

Daniel Jasper djasper at google.com
Tue Mar 19 04:12:36 PDT 2013



================
Comment at: lib/Format/Format.cpp:311
@@ +310,3 @@
+    if (InPPDirective)
+      --ColumnLimit; // Reserve space for terminating backslash.
+
----------------
Another point for reusing more stuff. We have done the same before, but have then decided to always leave at least one space before "\". So, this should be ColumnLimit -= 2. Or, even better, find a way to reuse getColumnLimit()..

================
Comment at: lib/Format/Format.cpp:280
@@ +279,3 @@
+          break;
+        StringRef FirstPiece = Line.substr(0, SpacePos).rtrim();
+        Line = Line.substr(SpacePos).ltrim();
----------------
Alexander Kornienko wrote:
> Daniel Jasper wrote:
> > maybe s/FirstPiece/BeforeSpace/
> > 
> > Does rtrim() actually change the behavior? I would have thought that line 277 already ensures finding the first space ...
> I think FirstPiece is more suitable here.
> 
> Line 275 finds the last space before ColumnLimit, so we need to find the first one. In case line 277 gets executed, rtrim doesn't make a difference.
And I disagree on that one. Especially, because it is not always the first piece of the line.

So rtrim is used, if there are spaces at the end of the line, but not before the column limit? Does a test break if you remove it?

================
Comment at: lib/Format/Format.cpp:226
@@ +225,3 @@
+  void splitLineInComment(const FormatToken &Tok, StringRef Line,
+                          size_t ColumnLimit, const StringRef LinePrefix,
+                          bool InPPDirective, const FormatStyle &Style,
----------------
Don't hand in the column limit as it directly follows from \c LinePrefix.size(), \c Style and \c InPPDirective (if I am not mistaken). Instead, move (parts of) the calculation of the limit into a function getColumnLimit..


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



More information about the cfe-commits mailing list