r338890 - clang-format: [JS] don't break comments before any '{'

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Aug  3 06:58:33 2018
New Revision: 338890

URL: http://llvm.org/viewvc/llvm-project?rev=338890&view=rev
Log:
clang-format: [JS] don't break comments before any '{'

Summary:
Previously, clang-format would avoid breaking before the first `{`
found, but then happily break before subsequent '{'s on the line. This
change fixes that by looking for the first location that has no opening
curly, if any.

This fixes the original commit by correcting the loop condition.

This reverts commit 66dc646e09b795b943668179c33d09da71a3b6bc.

Reviewers: krasimir

Subscribers: cfe-commits

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

Modified:
    cfe/trunk/lib/Format/BreakableToken.cpp
    cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=338890&r1=338889&r2=338890&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Fri Aug  3 06:58:33 2018
@@ -90,19 +90,21 @@ static BreakableToken::Split getCommentS
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-      kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-    SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-      SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-      Text[SpaceOffset + 1] == '{')
-    SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+    // Do not split before a number followed by a dot: this would be interpreted
+    // as a numbered list, which would prevent re-flowing in subsequent passes.
+    if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+      SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+    // In JavaScript, some @tags can be followed by {, and machinery that parses
+    // these comments will fail to understand the comment if followed by a line
+    // break. So avoid ever breaking before a {.
+    else if (Style.Language == FormatStyle::LK_JavaScript &&
+             SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+      SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+    else
+      break;
+  }
 
   if (SpaceOffset == StringRef::npos ||
       // Don't break at leading whitespace.

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=338890&r1=338889&r2=338890&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Aug  3 06:58:33 2018
@@ -2067,6 +2067,14 @@ TEST_F(FormatTestJS, JSDocAnnotations) {
                " * @param {canWrap onSpace}\n"
                " */",
                getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+               " * @lala {lala {lalala\n"
+               " */\n",
+               "/**\n"
+               " * @lala {lala {lalala\n"
+               " */\n",
+               getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
                " * @see http://very/very/long/url/is/long\n"
                " */",