[cfe-commits] [PATCH] Initial version of formatting library

Daniel Jasper djasper at google.com
Fri Nov 16 11:46:46 PST 2012



================
Comment at: lib/Format/ContinuationParser.h:49-55
@@ +48,9 @@
+
+/// \brief A continuation is a sequence of \c Token, that we would like to put
+/// on a single line if there was no column limit.
+///
+/// This is used as a main interface between the \c ContinuationParser and the
+/// \c ContinuationFormatter. The key property is that changing the formatting
+/// within a continuation does not affect any other continuation.
+struct Continuation {
+  Continuation() : Level(0) {}
----------------
Sean Silva wrote:
> Why is this called "continuation"? The name doesn't seem to have anything to do with the description; the only meaning of "continuation" as a noun that I can think of is "line continuation", which is the separating characters that join the logical line across physical source lines (e.g. http://msdn.microsoft.com/en-us/library/aa711641(v=vs.71).aspx).
> 
> How about "LogicalLine"?
> 
> Your sentence "the key property" is pure gold, btw.
After discussions with Chandler and Richard, renamed it to UnwrappedLine. Logical seems too generic.

================
Comment at: lib/Format/ContinuationParser.h:58-59
@@ +57,4 @@
+
+  /// \brief The \c Token comprising this continuation.
+  std::vector<FormatToken> Tokens;
+
----------------
Sean Silva wrote:
> Prefer SmallVector
Done.

================
Comment at: lib/Format/ContinuationParser.h:9-12
@@ +8,6 @@
+//===----------------------------------------------------------------------===//
+//
+//  This is EXPERIMENTAL code under heavy development. It is not in a state yet,
+//  where it can be used to format real code.
+//
+//===----------------------------------------------------------------------===//
----------------
Sean Silva wrote:
> http://llvm.org/docs/CodingStandards.html#file-headers
> 
> Describe the file. Describe the "theory of operations".
Done. Won't describe theory of operations just yet as it is too likely to change.

================
Comment at: lib/Format/ContinuationParser.cpp:9-12
@@ +8,6 @@
+//===----------------------------------------------------------------------===//
+//
+//  This is EXPERIMENTAL code under heavy development. It is not in a state yet,
+//  where it can be used to format real code.
+//
+//===----------------------------------------------------------------------===//
----------------
Sean Silva wrote:
> Proper file header needed here too.
Done.

================
Comment at: lib/Format/Format.cpp:45
@@ +44,3 @@
+    State.Indent.push_back(Cont.Level * 2 + 4);
+    for (unsigned i = 1, e = Cont.Tokens.size(); i != e; ++i) {
+      bool InsertNewLine = Cont.Tokens[i].NewlinesBefore > 0;
----------------
Sean Silva wrote:
> http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> 
> I and N are the canonical names for integer loop variables (I and E moreso for iterators).
> 
> also, comment why you are iterating starting at 1.
Done.

================
Comment at: lib/Format/Format.cpp:86
@@ +85,3 @@
+
+  // Append the token at 'Index' to the IndentState 'State'.
+  void addToken(unsigned Index, bool Newline, bool DryRun, IndentState &State) {
----------------
Sean Silva wrote:
> Doxygenify
Done.


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



More information about the cfe-commits mailing list