================
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];
----------------
Daniel Jasper wrote:
> 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.
Yeah, the whole loop could be replaced with a couple of if's, while we have 
just two languages ;)

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

================
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 ||
----------------
Daniel Jasper wrote:
> Please document what this does.
Done.

================
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)
----------------
Daniel Jasper wrote:
> Add:
> 
>   // Ensures that only the first configuration can skip languages and
>   // each language is configured at most once.
Done.

================
Comment at: lib/Format/Format.cpp:1295
@@ -1203,1 +1294,3 @@
 
+static const char *getLanguageName(FormatStyle::LanguageKind Language) {
+  switch (Language) {
----------------
Daniel Jasper wrote:
> Return StringRef.
I was thinking about this, but it seemed, that the difference is almost 
negligible in this case, so I've decided to go with the simpler version. But 
yes, if you prefer StringRef here, it totally makes sense to change it.

================
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") ||
----------------
Daniel Jasper wrote:
> What happens by default, i.e. if the file extensions matches none of these?
We don't fill the Language here, and use the language from the predefined 
style. But we can also set it to LK_None as a hint, that we probably don't want 
to format a file with unknown language. We'll probably have to default to C++, 
when formatting standard input, though.


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

BRANCH
  svn

ARCANIST PROJECT
  clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to