Skip to content

Commit

Permalink
clang-format: [JS] fix indenting bound functions.
Browse files Browse the repository at this point in the history
Summary:
The previous fix to force build style wrapping if the previous token is a closing parenthesis broke a peculiar pattern where users parenthesize the function declaration in a bind call:
    fn((function() { ... }).bind(this));

This restores the previous behaviour by reverting that change, but narrowing the special case for unindenting closing parentheses to those followed by semicolons and opening braces, i.e. immediate calls and function declarations.

Reviewers: djasper

Subscribers: cfe-commits, klimek

Differential Revision: https://reviews.llvm.org/D33640

llvm-svn: 304135
  • Loading branch information
mprobst committed May 29, 2017
1 parent dd7d82c commit b2f06ea
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
15 changes: 13 additions & 2 deletions clang/lib/Format/ContinuationIndenter.cpp
Expand Up @@ -215,7 +215,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
// ...
// }.bind(...));
// FIXME: We should find a more generic solution to this problem.
!(State.Column <= NewLineColumn && Previous.isNot(tok::r_paren) &&
!(State.Column <= NewLineColumn &&
Style.Language == FormatStyle::LK_JavaScript))
return true;

Expand Down Expand Up @@ -689,7 +689,18 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
return State.Stack[State.Stack.size() - 2].LastSpace;
return State.FirstIndent;
}
if (Current.is(tok::r_paren) && State.Stack.size() > 1)
// Indent a closing parenthesis at the previous level if followed by a semi or
// opening brace. This allows indentations such as:
// foo(
// a,
// );
// function foo(
// a,
// ) {
// code(); //
// }
if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
(!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
return State.Stack[State.Stack.size() - 2].LastSpace;
if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
return State.Stack[State.Stack.size() - 2].LastSpace;
Expand Down
5 changes: 5 additions & 0 deletions clang/unittests/Format/FormatTestJS.cpp
Expand Up @@ -717,6 +717,11 @@ TEST_F(FormatTestJS, FunctionLiterals) {
" bar();\n"
"}.bind(this));");

verifyFormat("SomeFunction((function() {\n"
" foo();\n"
" bar();\n"
" }).bind(this));");

// FIXME: This is bad, we should be wrapping before "function() {".
verifyFormat("someFunction(function() {\n"
" doSomething(); // break\n"
Expand Down

0 comments on commit b2f06ea

Please sign in to comment.