mprobst added inline comments. ================ Comment at: lib/Format/Format.cpp:1087 @@ +1086,3 @@ + (Style.Quotes == FormatStyle::QS_Single && + !FormatTok->TokenText.startswith("\"")) || + (Style.Quotes == FormatStyle::QS_Double && ---------------- djasper wrote: > Are there more than the two options? Otherwise, I would find: > > Style.Quotes == FormatStyle::QS_Single && > FormatTok->TokenText.startswith("\'") > > more intuitive. I would too, but think about `template strings`. Added a comment. I think it's safer to specifically check for the thing that we do handle/fix below, instead of just accepting everything we think is not correct.
================ 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';", ---------------- djasper wrote: > 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 "' + '". This was exposed by the char_literal/string_literal test, it aligns the behaviour with what would always happen for a double quoted string. It fits the no-alignment rule. I've changed the test to make sure it actually wraps on two lines. ================ 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)); ---------------- djasper wrote: > I don't think this is intended. It is. This was incorrectly handled before, the line is longer than 20 chars. It's because I switched single quoted strings from char_literal to string_literal. http://reviews.llvm.org/D17385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits