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

Daniel Jasper djasper at google.com
Fri Nov 29 07:00:34 PST 2013


  Nice. I like this much better. A couple of nits, the rest looks good.


================
Comment at: lib/Format/Format.cpp:399
@@ +398,3 @@
+
+  for (unsigned i = 1; i < Styles.size(); ++i) {
+    if (Styles[i].Language == FormatStyle::LK_None && i != 1)
----------------
Add:

  // Ensures that only the first configuration can skip languages and
  // each language is configured at most once.

================
Comment at: lib/Format/Format.cpp:1295
@@ -1203,1 +1294,3 @@
 
+static const char *getLanguageName(FormatStyle::LanguageKind Language) {
+  switch (Language) {
----------------
Return StringRef.

================
Comment at: lib/Format/Format.cpp:1645
@@ -1540,1 +1644,3 @@
 
+static void fillLanguageByFileName(StringRef FileName, FormatStyle *Style) {
+  if (FileName.endswith_lower(".c") || FileName.endswith_lower(".h") ||
----------------
What happens by default, i.e. if the file extensions matches none of these?

================
Comment at: lib/Format/Format.cpp:407
@@ +406,3 @@
+  }
+  for (unsigned i = Styles.size() - 1; i > 0; --i) {
+    if (Styles[i].Language == Styles[0].Language ||
----------------
Please document what this does.

================
Comment at: lib/Format/Format.cpp:412
@@ +411,3 @@
+      Style->Language = Styles[0].Language;
+      return Input.error();
+    }
----------------
This will never return an error, right? (because of the check in line 396).. If so, how about just return "OK"?

================
Comment at: lib/Format/Format.cpp:409
@@ +408,3 @@
+    if (Styles[i].Language == Styles[0].Language ||
+        Styles[i].Language == FormatStyle::LK_None) {
+      *Style = Styles[i];
----------------
I find it weird to have this second condition inside the loop as it can only ever be true for i == 1. But I can't really come up with a better way to write this.


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

BRANCH
  svn

ARCANIST PROJECT
  clang



More information about the cfe-commits mailing list