[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