djasper added inline comments.
================
Comment at: include/clang/Format/Format.h:606
@@ +605,3 @@
+ /// \brief The QuoteStyle to use for JavaScript strings.
+ QuoteStyle Quotes;
+
----------------
As this only affects JavaScript and doesn't make any sense for Java or C++, I'd
like to have JS/JavaScript in the option and enum name.
================
Comment at: lib/Format/Format.cpp:76
@@ +75,3 @@
+ static void enumeration(IO &IO, FormatStyle::QuoteStyle &Value) {
+ IO.enumCase(Value, "leave", FormatStyle::QS_Leave);
+ IO.enumCase(Value, "single", FormatStyle::QS_Single);
----------------
For almost all the other enums we use upper case.
================
Comment at: lib/Format/Format.cpp:1087
@@ +1086,3 @@
+ (Style.Quotes == FormatStyle::QS_Single &&
+ !FormatTok->TokenText.startswith("\"")) ||
+ (Style.Quotes == FormatStyle::QS_Double &&
----------------
Are there more than the two options? Otherwise, I would find:
Style.Quotes == FormatStyle::QS_Single &&
FormatTok->TokenText.startswith("\'")
more intuitive.
================
Comment at: lib/Format/Format.cpp:1094
@@ +1093,3 @@
+
+ StringRef Input = FormatTok->TokenText;
+ size_t ColumnWidth = FormatTok->TokenText.size();
----------------
Move this up so you can use it in the if-statement?
================
Comment at: unittests/Format/FormatTestJS.cpp:638
@@ -637,3 +637,3 @@
TEST_F(FormatTestJS, StringLiteralConcatenation) {
- verifyFormat("var literal = 'hello ' +\n"
- " 'world';");
+ verifyFormat("var literal =\n"
+ " 'hello ' + 'world';",
----------------
Why change this test? I think this was meant to test that we always add a line
break between two consecutive string literals as otherwise the user could just
have written this without the "' + '".
================
Comment at: unittests/Format/FormatTestJS.cpp:878
@@ -876,3 +877,3 @@
" Y,\n"
- "} from 'some/long/module.js';",
+ "} from\n 'some/long/module.js';",
getGoogleJSStyleWithColumns(20));
----------------
I don't think this is intended.
http://reviews.llvm.org/D17385
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
