[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