]> granicus.if.org Git - clang/commitdiff
[clang-format] comment reflow: add last line's penalty when ending broken
authorKrasimir Georgiev <krasimir@google.com>
Tue, 7 Aug 2018 10:23:24 +0000 (10:23 +0000)
committerKrasimir Georgiev <krasimir@google.com>
Tue, 7 Aug 2018 10:23:24 +0000 (10:23 +0000)
Summary:
This fixes a bug in clang-format where the last line's penalty is not
taken into account when its ending is broken. Usually the last line's penalty
is handled by addNextStateToQueue, but in cases where the trailing `*/` is put
on a newline, the contents of the last line have to be considered for penalizing.

Reviewers: mprobst

Reviewed By: mprobst

Subscribers: cfe-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@339123 91177308-0d34-0410-b5e6-96231b3b80d8

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

index 7ca588a675b5ae5c879ac41f053d957ef7440063..0c202565051d593450819c0d41f05a1c1e2f00de 100644 (file)
@@ -1840,7 +1840,8 @@ ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
         // No break opportunity - update the penalty and continue with the next
         // logical line.
         if (LineIndex < EndIndex - 1)
-          // The last line's penalty is handled in addNextStateToQueue().
+          // The last line's penalty is handled in addNextStateToQueue() or when
+          // calling replaceWhitespaceAfterLastLine below.
           Penalty += Style.PenaltyExcessCharacter *
                      (ContentStartColumn + RemainingTokenColumns - ColumnLimit);
         LLVM_DEBUG(llvm::dbgs() << "    No break opportunity.\n");
@@ -2095,6 +2096,12 @@ ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
       Token->getSplitAfterLastLine(TailOffset);
   if (SplitAfterLastLine.first != StringRef::npos) {
     LLVM_DEBUG(llvm::dbgs() << "Replacing whitespace after last line.\n");
+
+    // We add the last line's penalty here, since that line is going to be split
+    // now.
+    Penalty += Style.PenaltyExcessCharacter *
+               (ContentStartColumn + RemainingTokenColumns - ColumnLimit);
+
     if (!DryRun)
       Token->replaceWhitespaceAfterLastLine(TailOffset, SplitAfterLastLine,
                                             Whitespaces);
index 58a900352a2470e5172b7714a9fbd00aef62137d..8346783738952e3b9419a1c6953b392a7910b26a 100644 (file)
@@ -2284,5 +2284,25 @@ TEST_F(FormatTestJS, BackslashesInComments) {
                "formatMe( );\n");
 }
 
+TEST_F(FormatTestJS, AddsLastLinePenaltyIfEndingIsBroken) {
+  EXPECT_EQ(
+      "a = function() {\n"
+      "  b = function() {\n"
+      "    this.aaaaaaaaaaaaaaaaaaa[aaaaaaaaaaa] = aaaa.aaaaaa ?\n"
+      "        aaaa.aaaaaa : /** @type "
+      "{aaaa.aaaa.aaaaaaaaa.aaaaaaaaaaaaaaaaaaa} */\n"
+      "        (aaaa.aaaa.aaaaaaaaa.aaaaaaaaaaaaa.aaaaaaaaaaaaaaaaa);\n"
+      "  };\n"
+      "};",
+      format("a = function() {\n"
+             "  b = function() {\n"
+             "    this.aaaaaaaaaaaaaaaaaaa[aaaaaaaaaaa] = aaaa.aaaaaa ? "
+             "aaaa.aaaaaa : /** @type "
+             "{aaaa.aaaa.aaaaaaaaa.aaaaaaaaaaaaaaaaaaa} */\n"
+             "        (aaaa.aaaa.aaaaaaaaa.aaaaaaaaaaaaa.aaaaaaaaaaaaaaaaa);\n"
+             "  };\n"
+             "};"));
+}
+
 } // end namespace tooling
 } // end namespace clang