[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