Skip to content

Commit

Permalink
[clang-format] Disable string breaking in JS for now (#66372)
Browse files Browse the repository at this point in the history 
See the discussion

[here](#66168 (comment)).

The functionality is not mature enough.
  • Loading branch information
@sstwcw
sstwcw committed Sep 15, 2023
1 parent 0069004 commit ae90f68
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 132 deletions.
10 changes: 5 additions & 5 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2783,17 +2783,17 @@ the configuration (without a prefix: ``Auto``).
const char* x =
"veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";

In C#, Java, and JavaScript:
In C# and Java:

.. code-block:: c++

true:
var x = "veryVeryVeryVeryVeryVe" +
"ryVeryVeryVeryVeryVery" +
"VeryLongString";
string x = "veryVeryVeryVeryVeryVe" +
"ryVeryVeryVeryVeryVery" +
"VeryLongString";

false:
var x =
string x =
"veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";

C# and JavaScript interpolated strings are not broken.
Expand Down
12 changes: 6 additions & 6 deletions clang/include/clang/Format/Format.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2062,19 +2062,19 @@ struct FormatStyle {
/// "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
/// \endcode
///
/// In C#, Java, and JavaScript:
/// In C# and Java:
/// \code
/// true:
/// var x = "veryVeryVeryVeryVeryVe" +
/// "ryVeryVeryVeryVeryVery" +
/// "VeryLongString";
/// string x = "veryVeryVeryVeryVeryVe" +
/// "ryVeryVeryVeryVeryVery" +
/// "VeryLongString";
///
/// false:
/// var x =
/// string x =
/// "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
/// \endcode
///
/// C# and JavaScript interpolated strings are not broken.
/// C# interpolated strings are not broken.
///
/// In Verilog:
/// \code
Expand Down
13 changes: 4 additions & 9 deletions clang/lib/Format/ContinuationIndenter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2237,15 +2237,10 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
LineState &State, bool AllowBreak) {
unsigned StartColumn = State.Column - Current.ColumnWidth;
if (Current.isStringLiteral()) {
// Strings in JSON can not be broken.
if (Style.isJson() || !Style.BreakStringLiterals || !AllowBreak)
return nullptr;

// Strings in TypeScript types and dictionary keys can not be broken.
if (Style.isJavaScript() &&
(Current.is(TT_SelectorName) ||
State.Line->startsWith(Keywords.kw_type) ||
State.Line->startsWith(tok::kw_export, Keywords.kw_type))) {
// Strings in JSON cannot be broken. Breaking strings in JavaScript is
// disabled for now.
if (Style.isJson() || Style.isJavaScript() || !Style.BreakStringLiterals ||
!AllowBreak) {
return nullptr;
}

Expand Down
114 changes: 2 additions & 112 deletions clang/unittests/Format/FormatTestJS.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1506,121 +1506,11 @@ TEST_F(FormatTestJS, StringLiteralConcatenation) {
verifyFormat("var literal = 'hello ' +\n"
" 'world';");

// Long strings should be broken.
// String breaking is disabled for now.
verifyFormat("var literal =\n"
" 'xxxxxxxx ' +\n"
" 'xxxxxxxx';",
" 'xxxxxxxx xxxxxxxx';",
"var literal = 'xxxxxxxx xxxxxxxx';",
getGoogleJSStyleWithColumns(17));
verifyFormat("var literal =\n"
" 'xxxxxxxx ' +\n"
" 'xxxxxxxx';",
"var literal = 'xxxxxxxx xxxxxxxx';",
getGoogleJSStyleWithColumns(18));
verifyFormat("var literal =\n"
" 'xxxxxxxx' +\n"
" ' xxxxxxxx';",
"var literal = 'xxxxxxxx xxxxxxxx';",
getGoogleJSStyleWithColumns(16));
// The quotes should be correct.
for (char OriginalQuote : {'\'', '"'}) {
auto VerifyQuotes = [=](FormatStyle::JavaScriptQuoteStyle StyleQuote,
char TargetQuote) {
auto Style = getGoogleJSStyleWithColumns(17);
Style.JavaScriptQuotes = StyleQuote;
std::string Target{"var literal =\n"
" \"xxxxxxxx \" +\n"
" \"xxxxxxxx\";"};
std::string Original{"var literal = \"xxxxxxxx xxxxxxxx\";"};
std::replace(Target.begin(), Target.end(), '"', TargetQuote);
std::replace(Original.begin(), Original.end(), '"', OriginalQuote);
verifyFormat(Target, Original, Style);
};
VerifyQuotes(FormatStyle::JSQS_Leave, OriginalQuote);
VerifyQuotes(FormatStyle::JSQS_Single, '\'');
VerifyQuotes(FormatStyle::JSQS_Double, '"');
}
// Parentheses should be added when necessary.
verifyFormat("var literal =\n"
" ('xxxxxxxx ' +\n"
" 'xxxxxx')[0];",
"var literal = 'xxxxxxxx xxxxxx'[0];",
getGoogleJSStyleWithColumns(18));
auto Style = getGoogleJSStyleWithColumns(20);
Style.SpacesInParens = FormatStyle::SIPO_Custom;
Style.SpacesInParensOptions.Other = true;
verifyFormat("var literal =\n"
" ( 'xxxxxxxx ' +\n"
" 'xxxxxx' )[0];",
"var literal = 'xxxxxxxx xxxxxx'[0];", Style);
// FIXME: When the part before the string literal is shorter than the
// continuation indentation, and the option AlignAfterOpenBracket is set to
// AlwaysBreak which is the default for the Google style, the unbroken string
// does not get to a new line while the broken string does due to the added
// parentheses. The formatter does not do it in one pass.
EXPECT_EQ(
"x = ('xxxxxxxx ' +\n"
" 'xxxxxx')[0];",
format("x = 'xxxxxxxx xxxxxx'[0];", getGoogleJSStyleWithColumns(18)));
verifyFormat("x =\n"
" ('xxxxxxxx ' +\n"
" 'xxxxxx')[0];",
getGoogleJSStyleWithColumns(18));
// Breaking of template strings and regular expressions is not implemented.
verifyFormat("var literal =\n"
" `xxxxxxxx xxxxxxxx`;",
getGoogleJSStyleWithColumns(18));
verifyFormat("var literal =\n"
" /xxxxxxxx xxxxxxxx/;",
getGoogleJSStyleWithColumns(18));
// There can be breaks in the code inside a template string.
verifyFormat("var literal = `xxxxxx ${\n"
" xxxxxxxxxx} xxxxxx`;",
"var literal = `xxxxxx ${xxxxxxxxxx} xxxxxx`;",
getGoogleJSStyleWithColumns(14));
verifyFormat("var literal = `xxxxxx ${\n"
" xxxxxxxxxx} xxxxxx`;",
"var literal = `xxxxxx ${xxxxxxxxxx} xxxxxx`;",
getGoogleJSStyleWithColumns(15));
// Identifiers inside the code inside a template string should not be broken
// even if the column limit is exceeded. This following behavior is not
// optimal. The part after the closing brace which exceeds the column limit
// can be put on a new line. Change this test when it is implemented.
verifyFormat("var literal = `xxxxxx ${\n"
" xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
"var literal = `xxxxxx ${xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
getGoogleJSStyleWithColumns(14));
verifyFormat("var literal = `xxxxxx ${\n"
" xxxxxxxxxxxxxxxxxxxxxx +\n"
" xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
"var literal = `xxxxxx ${xxxxxxxxxxxxxxxxxxxxxx + "
"xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
getGoogleJSStyleWithColumns(14));

// Strings in a TypeScript type declaration can't be broken.
verifyFormat("type x =\n"
" 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
getGoogleJSStyleWithColumns(20));
verifyFormat("/* type */ type x =\n"
" 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
getGoogleJSStyleWithColumns(20));
verifyFormat("export type x =\n"
" 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
getGoogleJSStyleWithColumns(20));
// Dictionary keys can't be broken. Values can be broken.
verifyFormat("var w = {\n"
" 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx':\n"
" 'xxxxxxxxxx' +\n"
" 'xxxxxxxxxx' +\n"
" 'xxxxxxxxxx' +\n"
" 'xxxxxxxxxx' +\n"
" 'xxx',\n"
"};",
"var w = {\n"
" 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx':\n"
" 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',\n"
"};",
getGoogleJSStyleWithColumns(20));
}

TEST_F(FormatTestJS, RegexLiteralClassification) {
Expand Down

0 comments on commit ae90f68

Please sign in to comment.