[PATCH] D33640: clang-format: [JS] fix indenting bound functions.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
Herald added a subscriber: klimek.

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.


https://reviews.llvm.org/D33640

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestJS.cpp


Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -717,6 +717,11 @@
                "  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"
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -207,7 +207,7 @@
       //     ...
       //   }.bind(...));
       // FIXME: We should find a more generic solution to this problem.
-      !(State.Column <= NewLineColumn && Previous.isNot(tok::r_paren) &&
+      !(State.Column <= NewLineColumn && // Current.TokenText == "bind" &&
         Style.Language == FormatStyle::LK_JavaScript))
     return true;
 
@@ -676,7 +676,18 @@
       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,
+  //     );
+  //     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;



Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -717,6 +717,11 @@
                "  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"
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -207,7 +207,7 @@
       //     ...
       //   }.bind(...));
       // FIXME: We should find a more generic solution to this problem.
-      !(State.Column <= NewLineColumn && Previous.isNot(tok::r_paren) &&
+      !(State.Column <= NewLineColumn && // Current.TokenText == "bind" &&
         Style.Language == FormatStyle::LK_JavaScript))
     return true;
 
@@ -676,7 +676,18 @@
       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,
+  //     );
+  //     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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits