Re: [PATCH] D24257: clang-format: [JS] ignore comments when wrapping returns.

[Martin Probst via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL280730: clang-format: [JS] ignore comments when wrapping 
returns. (authored by mprobst).

Changed prior to commit:
  https://reviews.llvm.org/D24257?vs=70405=70444#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24257

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp

Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2381,7 +2381,12 @@
   Keywords.kw_implements))
   return true;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
-if (Left.is(tok::kw_return))
+const FormatToken *NonComment = Right.getPreviousNonComment();
+if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+ tok::kw_throw) ||
+(NonComment &&
+ NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+ tok::kw_throw)))
   return false; // Otherwise a semicolon is inserted.
 if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
   return false;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -686,7 +686,9 @@
   // would change due to automatic semicolon insertion.
   // See http://www.ecma-international.org/ecma-262/5.1/#sec-7.9.1.
   verifyFormat("return a;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("return /* hello! */ a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("continue a;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("continue /* hello! */ a;", 
getGoogleJSStyleWithColumns(10));
   verifyFormat("break a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("throw a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("a++;", getGoogleJSStyleWithColumns(10));


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2381,7 +2381,12 @@
   Keywords.kw_implements))
   return true;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
-if (Left.is(tok::kw_return))
+const FormatToken *NonComment = Right.getPreviousNonComment();
+if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+ tok::kw_throw) ||
+(NonComment &&
+ NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+ tok::kw_throw)))
   return false; // Otherwise a semicolon is inserted.
 if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
   return false;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -686,7 +686,9 @@
   // would change due to automatic semicolon insertion.
   // See http://www.ecma-international.org/ecma-262/5.1/#sec-7.9.1.
   verifyFormat("return a;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("return /* hello! */ a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("continue a;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("continue /* hello! */ a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("break a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("throw a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("a++;", getGoogleJSStyleWithColumns(10));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24257: clang-format: [JS] ignore comments when wrapping returns.

[Daniel Jasper via cfe-commits]

djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.



Comment at: lib/Format/TokenAnnotator.cpp:2384
@@ -2383,2 +2383,3 @@
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
-if (Left.is(tok::kw_return))
+const FormatToken *NonComment = Right.getPreviousNonComment();
+if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,

mprobst wrote:
> Should the entire method generally consider the previous non-comment, instead 
> of just the previous token? Not sure if that makes sense in all cases, and I 
> don't think we always have a previous non-comment token either.
Lets leave this as is for now. Yes, some of these should likely look past 
comments, but probably not all of them.


https://reviews.llvm.org/D24257



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

Re: [PATCH] D24257: clang-format: [JS] ignore comments when wrapping returns.

[Martin Probst via cfe-commits]

mprobst added inline comments.


Comment at: lib/Format/TokenAnnotator.cpp:2384
@@ -2383,2 +2383,3 @@
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
-if (Left.is(tok::kw_return))
+const FormatToken *NonComment = Right.getPreviousNonComment();
+if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,

Should the entire method generally consider the previous non-comment, instead 
of just the previous token? Not sure if that makes sense in all cases, and I 
don't think we always have a previous non-comment token either.


https://reviews.llvm.org/D24257



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