================
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