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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
