[PATCH] D16087: Add some overview doxygen comments to lib/Format.

[Daniel Jasper via cfe-commits]

djasper added inline comments.

================
Comment at: lib/Format/ContinuationIndenter.h:35
@@ -34,1 +34,3 @@
 
+/// \brief Helper class for breaking a logical line into multiple physical
+/// lines.  Manages moving state from one physical line to the next.
----------------
You are the first to introduce the term "pyhsical" line and I don't actually think it is the right term. Also, this is not a "helper class".

Maybe:

If an unwrapped line needs to be split over multiple lines to fit into the column limit, this class can be used to keep track of the indentation for the different levels of parentheses and to get the desired indentation for any given line break.

================
Comment at: lib/Format/Format.cpp:761
@@ -760,2 +760,3 @@
 
+/// \brief Responsible for splitting up a file into \c FormatTokens.
 class FormatTokenLexer {
----------------
  /// \brief Splits a file into FormatTokens.

Also, this comment adds zero information.

================
Comment at: lib/Format/Format.cpp:1426
@@ +1425,3 @@
+/// -# A \c FormatTokenLexer splits the file into raw tokens
+/// -# A \c UnwrappedLineParser converts those into logical lines
+/// -# A \c TokenAnnotator computes metadata for the tokens in each line
----------------
If we wanted to call them logical lines, we would have. Stick to the term unwrapped line.

================
Comment at: lib/Format/Format.cpp:1429
@@ -1420,1 +1428,3 @@
+/// -# A \c UnwrappedLineFormatter then produces whitespace adjustments
+///    which result in the desired logical lines
 class Formatter : public UnwrappedLineConsumer {
----------------
Same here, although I think you don't even mean logical lines.

================
Comment at: lib/Format/FormatToken.h:110-111
@@ -109,4 +109,4 @@
 
-/// \brief A wrapper around a \c Token storing information about the
-/// whitespace characters preceding it.
+/// \brief A wrapper around a \c Token storing additional information useful
+/// for formatting.
 struct FormatToken {
----------------
What information does this comment add? I am not against adding one, but then describe in a bit more detail what is going on.

================
Comment at: lib/Format/TokenAnnotator.h:39
@@ -38,2 +38,3 @@
 
+/// \brief Stores additional information for an \c UnwrappedLine.
 class AnnotatedLine {
----------------
What's the added info?

================
Comment at: lib/Format/TokenAnnotator.h:152
@@ -150,2 +151,3 @@
 
+  /// \brief Compute metadata of \c AnnotatedLine itself, like \c LineType.
   void annotate(AnnotatedLine &Line);
----------------
s/like/such as/

Also, I think that the comment is misleading. The LineType is the least important thing that this function does. It annotates all the tokens, for that parses all the expressions in it, assigned TokenTypes to the format tokens, and also annotates all Children of this line.

================
Comment at: lib/Format/TokenAnnotator.h:155-156
@@ -152,1 +154,4 @@
+
+  /// \brief For all the tokens in \c Line, compute if a line break is
+  /// needed for that token, and similar information.
   void calculateFormattingInformation(AnnotatedLine &Line);
----------------
For all tokens in \c Line, use the additional information gathered during annotation to calculate formatting hints such as required line breaks, spaces and split penalties.

================
Comment at: lib/Format/UnwrappedLineFormatter.h:31
@@ -30,1 +30,3 @@
 
+/// \brief Responsible for taking a list of logical lines, and for producing
+/// a set of whitespace adjustments to turn them into physical lines so
----------------
Go through all of your comments and s/logical/unwrapped/

================
Comment at: lib/Format/UnwrappedLineFormatter.h:32
@@ +31,3 @@
+/// \brief Responsible for taking a list of logical lines, and for producing
+/// a set of whitespace adjustments to turn them into physical lines so
+/// that each physical line is below a column limit.
----------------
And the same with s/physical//. If that removes all the information, rephrase. E.g. here:

Takes a list of unwrapped lines and produces whitespace adjustments to lay them out within the column limit.

================
Comment at: lib/Format/UnwrappedLineParser.h:61-62
@@ -60,2 +60,4 @@
 
+/// \brief Takes a sequence of tokens, and splits it into chunks of tokens
+/// that would be a single line each if there was no column limit.
 class UnwrappedLineParser {
----------------
This might be a good place to introduce the term unwrapped line. The most notable fact about unwrapped lines btw. is not that you would put them on a single line if there was no column limit. This is a nice thought model and we should also write it, but it isn't always true. More importantly, formatting decisions in one unwrapped line do not influence formatting decisions in a different unwrapped line.


http://reviews.llvm.org/D16087