r311988 - clang-format: [JS] simplify template string wrapping.
Martin Probst via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 29 01:30:07 PDT 2017
Author: mprobst
Date: Tue Aug 29 01:30:07 2017
New Revision: 311988
URL: http://llvm.org/viewvc/llvm-project?rev=311988&view=rev
Log:
clang-format: [JS] simplify template string wrapping.
Summary:
Previously, clang-format would try to wrap template string substitutions
by indenting relative to the openening `${`. This helped with
indenting structured strings, such as strings containing HTML, as the
substitutions would be aligned according to the structure of the string.
However it turns out that the overwhelming majority of template string +
substitution usages are for substitutions into non-structured strings,
e.g. URLs or just plain messages. For these situations, clang-format
would often produce very ugly indents, in particular for strings
containing no line breaks:
return `<a href='http://google3/${file}?l=${row}'>${file}</a>(${
row
},${
col
}): `;
This change makes clang-format indent template string substitutions as
if they were string concatenation operations. It wraps +4 on overlong
lines and keeps all operands on the same line:
return `<a href='http://google3/${file}?l=${row}'>${file}</a>(${
row},${col}): `;
While this breaks some lexical continuity between the `${` and `row}`
here, the overall effects are still a huge improvement, and users can
still manually break the string using `+` if desired.
Reviewers: djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D37142
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=311988&r1=311987&r2=311988&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Aug 29 01:30:07 2017
@@ -661,9 +661,7 @@ unsigned ContinuationIndenter::addTokenO
// before the corresponding } or ].
if (PreviousNonComment &&
(PreviousNonComment->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
- opensProtoMessageField(*PreviousNonComment, Style) ||
- (PreviousNonComment->is(TT_TemplateString) &&
- PreviousNonComment->opensScope())))
+ opensProtoMessageField(*PreviousNonComment, Style)))
State.Stack.back().BreakBeforeClosingBrace = true;
if (State.Stack.back().AvoidBinPacking) {
@@ -925,11 +923,6 @@ unsigned ContinuationIndenter::moveState
moveStatePastFakeLParens(State, Newline);
moveStatePastScopeCloser(State);
- if (Current.is(TT_TemplateString) && Current.opensScope())
- State.Stack.back().LastSpace =
- (Current.IsMultiline ? Current.LastLineColumnWidth
- : State.Column + Current.ColumnWidth) -
- strlen("${");
bool CanBreakProtrudingToken = !State.Stack.back().NoLineBreak &&
!State.Stack.back().NoLineBreakInOperand;
moveStatePastScopeOpener(State, Newline);
@@ -1101,18 +1094,6 @@ void ContinuationIndenter::moveStatePast
LastSpace = std::max(LastSpace, State.Stack.back().Indent);
}
- // JavaScript template strings are special as we always want to indent
- // nested expressions relative to the ${}. Otherwise, this can create quite
- // a mess.
- if (Current.is(TT_TemplateString)) {
- unsigned Column = Current.IsMultiline
- ? Current.LastLineColumnWidth
- : State.Column + Current.ColumnWidth;
- NewIndent = Column;
- LastSpace = Column;
- NestedBlockIndent = Column;
- }
-
bool EndsInComma =
Current.MatchingParen &&
Current.MatchingParen->getPreviousNonComment() &&
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=311988&r1=311987&r2=311988&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Aug 29 01:30:07 2017
@@ -1793,32 +1793,28 @@ TEST_F(FormatTestJS, TemplateStrings) {
verifyFormat("var x = someFunction(`${})`) //\n"
" .oooooooooooooooooon();");
verifyFormat("var x = someFunction(`${aaaa}${\n"
- " aaaaa( //\n"
- " aaaaa)\n"
- " })`);");
+ " aaaaa( //\n"
+ " aaaaa)})`);");
}
TEST_F(FormatTestJS, TemplateStringMultiLineExpression) {
verifyFormat("var f = `aaaaaaaaaaaaaaaaaa: ${\n"
- " aaaaa + //\n"
- " bbbb\n"
- " }`;",
+ " aaaaa + //\n"
+ " bbbb}`;",
"var f = `aaaaaaaaaaaaaaaaaa: ${aaaaa + //\n"
" bbbb}`;");
verifyFormat("var f = `\n"
" aaaaaaaaaaaaaaaaaa: ${\n"
- " aaaaa + //\n"
- " bbbb\n"
- " }`;",
+ " aaaaa + //\n"
+ " bbbb}`;",
"var f = `\n"
" aaaaaaaaaaaaaaaaaa: ${ aaaaa + //\n"
" bbbb }`;");
verifyFormat("var f = `\n"
" aaaaaaaaaaaaaaaaaa: ${\n"
- " someFunction(\n"
- " aaaaa + //\n"
- " bbbb)\n"
- " }`;",
+ " someFunction(\n"
+ " aaaaa + //\n"
+ " bbbb)}`;",
"var f = `\n"
" aaaaaaaaaaaaaaaaaa: ${someFunction (\n"
" aaaaa + //\n"
@@ -1827,9 +1823,9 @@ TEST_F(FormatTestJS, TemplateStringMulti
// It might be preferable to wrap before "someFunction".
verifyFormat("var f = `\n"
" aaaaaaaaaaaaaaaaaa: ${someFunction({\n"
- " aaaa: aaaaa,\n"
- " bbbb: bbbbb,\n"
- " })}`;",
+ " aaaa: aaaaa,\n"
+ " bbbb: bbbbb,\n"
+ "})}`;",
"var f = `\n"
" aaaaaaaaaaaaaaaaaa: ${someFunction ({\n"
" aaaa: aaaaa,\n"
More information about the cfe-commits
mailing list