[PATCH] Added LanguageStandard::LS_JavaScript to gate all JS-specific parsing.

Daniel Jasper djasper at google.com
Thu Nov 28 05:55:15 PST 2013


  I think we should add some integration testing ..


================
Comment at: include/clang/Format/Format.h:35
@@ +34,3 @@
+  enum Language {
+    /// C++
+    Language_Cpp,
----------------
I think these comments are fairly pointless ..

But maybe this should (for the moment be):
  // Use this for C, C++, ObjC and ObjC++.

================
Comment at: lib/Format/Format.cpp:39
@@ +38,3 @@
+  static void enumeration(IO &IO, clang::format::FormatStyle::Language &Value) {
+    IO.enumCase(Value, "Cpp", clang::format::FormatStyle::Language_Cpp);
+    IO.enumCase(Value, "JavaScript",
----------------
Maybe allow other ways to write this. Off the top of my head: C, C++, ObjC, ObjC++, ...

================
Comment at: lib/Format/Format.cpp:188
@@ +187,3 @@
+
+// Allows to read vector<FormatStyle> using the 0th element as a default value.
+template <>
----------------
Could you document this a bit more? I have trouble understanding what this actually does.

================
Comment at: lib/Format/Format.cpp:1032
@@ +1031,3 @@
+      // correct priority.
+      tryMergeTokens(JSIdentity) || tryMergeTokens(JSNotIdentity) ||
+          tryMergeTokens(JSShiftEqual);
----------------
I know that this is a need and compact way to write this, but could you use proper control flow?

Maybe:
  if (tryMergeTokens(JSIdentity))
    return;
  if (tryMergeTokens(JSNotIdentity))
    return;
  tryMergeTokens(JSShiftEqual);



================
Comment at: include/clang/Format/Format.h:41
@@ +40,3 @@
+    /// may be used for all languages. When passed to the reformat() function,
+    /// enables automatic language detection by file name.
+    Language_Any
----------------
I would not write anything about "automatic language detection" before this is actually implemented (this is turned into user-facing documentation).


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



More information about the cfe-commits mailing list