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

Alexander Kornienko alexfh at google.com
Thu Nov 28 08:06:40 PST 2013


  I'm adding the file name-based language detection and tests for it.


================
Comment at: include/clang/Format/Format.h:35
@@ +34,3 @@
+  enum Language {
+    /// C++
+    Language_Cpp,
----------------
Daniel Jasper wrote:
> I think these comments are fairly pointless ..
> 
> But maybe this should (for the moment be):
>   // Use this for C, C++, ObjC and ObjC++.
Yes, comments on this enum were not meant to be final ;)
Updated.

================
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
----------------
Daniel Jasper wrote:
> I would not write anything about "automatic language detection" before this is actually implemented (this is turned into user-facing documentation).
"Automatic detection" left from the previous implementation. Removed.

================
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",
----------------
Daniel Jasper wrote:
> Maybe allow other ways to write this. Off the top of my head: C, C++, ObjC, ObjC++, ...
I wouldn't do this, as it would make people think, that we handle these languages differently. It wouldn't round-trip well, and it wouldn't be properly specified in the auto-generated documentation.

LanguageStandard parsing below supports alternative spellings only for backward compatibility.

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

================
Comment at: lib/Format/Format.cpp:1032
@@ +1031,3 @@
+      // correct priority.
+      tryMergeTokens(JSIdentity) || tryMergeTokens(JSNotIdentity) ||
+          tryMergeTokens(JSShiftEqual);
----------------
Daniel Jasper wrote:
> 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);
> 
> 
Done.


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



More information about the cfe-commits mailing list