[PATCH] clang-format: [JS] Fix enum handling.
Daniel Jasper
djasper at google.com
Wed Jun 17 02:49:16 PDT 2015
Comments mainly FYI. I have addressed them and submitted this as r239892-r239894. Thank you.
================
Comment at: lib/Format/TokenAnnotator.h:83
@@ -82,1 +82,3 @@
+ /// \c True if this line starts with the given tokens in order.
+ template <typename... Ts> bool StartsWith(Ts... rest) const {
----------------
I think now that we are writing such a fancy function, we should make it ignore comments. Otherwise, we would need to handle them everywhere.
================
Comment at: lib/Format/TokenAnnotator.h:84
@@ +83,3 @@
+ /// \c True if this line starts with the given tokens in order.
+ template <typename... Ts> bool StartsWith(Ts... rest) const {
+ return StartsWith(First, rest...);
----------------
In LLVM style, functions should use lowerCamelCase, variables on the other hand are UpperCamelCase (yeah, we should change that, but that's the way it is).
================
Comment at: lib/Format/TokenAnnotator.h:118
@@ +117,3 @@
+ bool StartsWith(FormatToken *token, A K1) const {
+ return token != NULL && token->is(K1);
+ }
----------------
If anything, use nullptr, but in LLVM style, we actually just use the bool conversion.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:776-778
@@ -776,2 +775,5 @@
case tok::kw_enum:
+ // We fall through to parsing a structural element afterwards, so that in
+ // enum A {} n, m;
+ // "} n, m;" will end up in one unwrapped line.
parseEnum();
----------------
Lets just use the same comment I have used for parseRecord.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:1515
@@ -1514,3 @@
-
- // We fall through to parsing a structural element afterwards, so that in
- // enum A {} n, m;
----------------
I think this comment should remain here. It explains why there is no call to addUnwrappedLine() here (maybe the comment should actually say so).
http://reviews.llvm.org/D10485
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list