From c4ca20d2bfb9dd85304f4df9a77ef6e9c2a64216 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Fri, 3 Aug 2018 13:58:33 +0000 Subject: [PATCH] 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@338890 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/BreakableToken.cpp | 26 ++++++++++++++------------ unittests/Format/FormatTestJS.cpp | 8 ++++++++ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/Format/BreakableToken.cpp b/lib/Format/BreakableToken.cpp index 300e3f802c..e6ce01b520 100644 --- a/lib/Format/BreakableToken.cpp +++ b/lib/Format/BreakableToken.cpp @@ -90,19 +90,21 @@ static BreakableToken::Split getCommentSplit(StringRef Text, 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. diff --git a/unittests/Format/FormatTestJS.cpp b/unittests/Format/FormatTestJS.cpp index fe148393b9..58a900352a 100644 --- a/unittests/Format/FormatTestJS.cpp +++ b/unittests/Format/FormatTestJS.cpp @@ -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" " */", -- 2.40.0