djasper added a comment.
Sorry for being so slow to respond. Feel free to ping me more frequently ;-).
================
Comment at: lib/Format/WhitespaceManager.cpp:197
@@ +196,3 @@
+ // Keep track of the scope depth of matching tokens. We will only align a
+ // sequence of matching token that share the same scope depth.
+ unsigned DepthOfPrevToken = 0;
----------------
Maybe be slightly more specific about what constitutes a "scope".
Also, FormatTokens already have this information stored in NestingLevel. Would
it maybe make sense to copy that information into each Change? In the long run,
we might actually wanna switch to storing a back-reference to the actual token
in a change to have access to all the information. But that would probably
better be done in a subsequent change.
================
Comment at: lib/Format/WhitespaceManager.cpp:208
@@ +207,3 @@
+ // Whether a matching token has been found on the current line.
+ bool FoundOnLine = false;
+
----------------
FoundOnLine seems to be missing some crucial information. Would you might
changing to FoundMathcingTokenOnLine or FoundMatchOnLine?
================
Comment at: lib/Format/WhitespaceManager.cpp:239
@@ -321,20 +238,3 @@
- if (Changes[i].Kind == tok::r_brace) {
- if (!FoundLeftBraceOnLine)
- AlignSequence();
- FoundLeftBraceOnLine = false;
- } else if (Changes[i].Kind == tok::l_brace) {
- FoundLeftBraceOnLine = true;
- if (!FoundDeclarationOnLine)
- AlignSequence();
- } else if (Changes[i].Kind == tok::r_paren) {
- if (!FoundLeftParenOnLine)
- AlignSequence();
- FoundLeftParenOnLine = false;
- } else if (Changes[i].Kind == tok::l_paren) {
- FoundLeftParenOnLine = true;
- if (!FoundDeclarationOnLine)
- AlignSequence();
- } else if (!FoundDeclarationOnLine && !FoundLeftBraceOnLine &&
- !FoundLeftParenOnLine && Changes[i].IsStartOfDeclName) {
- FoundDeclarationOnLine = true;
+ if (Changes[i].Kind == tok::comma) {
+ ++CommasBeforeToken;
----------------
I think this would be slightly easier to read if you used "continue" (basically
instead of each "else" here)
http://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
================
Comment at: unittests/Format/FormatTest.cpp:8662
@@ -8661,3 +8661,3 @@
"public:\n"
- " int i = 1;\n"
+ " int i = 1;\n"
" virtual void f() = 0;\n"
----------------
I am not sure that this is actually desirable, but then again, I also don't
think it matters. If people don't like it, they can add a blank line or a
comment in-between.
http://reviews.llvm.org/D14325
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
