Skip to content

Commit

Permalink
State: fix overflow of trailing comments
Browse files Browse the repository at this point in the history
Remove special case for binpacked comments, allow standalone comments.
For trailing-wrapped comments, rather than waiving penalty completely,
use the column that opening "/*" occupies.
  • Loading branch information
kitbellew committed Dec 28, 2021
1 parent 415f0f5 commit 7cee034
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import scala.annotation.tailrec
import scala.meta.tokens.Token

import org.scalafmt.config.Comments
import org.scalafmt.config.Docstrings
import org.scalafmt.config.ScalafmtConfig
import org.scalafmt.util.TokenOps
import org.scalafmt.util.TreeOps._
Expand Down Expand Up @@ -92,28 +91,17 @@ final case class State(
val (penalty, nextDelayedPenalty) =
if (columnOnCurrentLine <= style.maxColumn) noOverflowPenalties
else if (right.is[Token.Comment]) {
val rtext = tok.meta.right.text
if (nextSplit.isNL && rtext.length >= (style.maxColumn - nextIndent))
noOverflowPenalties
else {
val waive = nextTok.hasBreak && {
if (TokenOps.isDocstring(rtext))
(style.docstrings.wrap ne Docstrings.Wrap.no) && nextSplit.isNL
else
(style.comments.wrap eq Comments.Wrap.trailing) ||
style.comments.willWrap && nextSplit.isNL
} && (nextSplit.isNL || {
// do not waive overflow in binpacking case
val owner = tok.meta.rightOwner
val ownerLast = tokens.getLastNonTrivial(owner).left
val isBracket = ownerLast.is[Token.RightBracket]
!(isBracket || ownerLast.is[Token.RightParen]) ||
style.binPack.defnSite(isBracket).isNever && isDefnSite(owner) ||
style.binPack.callSite(isBracket).isNever && isCallSite(owner)
})
if (waive) noOverflowPenalties
def trailing = nextTok.hasBreak // newline after comment
if (nextSplit.isNL) { // newline before comment
val rtext = tok.meta.right.text
if (rtext.length >= (style.maxColumn - nextIndent) || trailing)
noOverflowPenalties
else overflowPenalties(columnOnCurrentLine)
}
} else if (style.comments.wrap.eq(Comments.Wrap.trailing) && trailing) {
val minColumn = startColumn + 2 // 2 is for "/*"
if (minColumn <= style.maxColumn) noOverflowPenalties
else overflowPenalties(minColumn)
} else overflowPenalties(columnOnCurrentLine)
} else overflowPenalties(columnOnCurrentLine)

val splitWithPenalty = nextSplit.withPenalty(penalty)
Expand Down
46 changes: 23 additions & 23 deletions scalafmt-tests/src/test/resources/unit/Comment.stat
Original file line number Diff line number Diff line change
Expand Up @@ -537,9 +537,9 @@ private object ClientMetrics {
>>>
private object ClientMetrics {
//empty since it is not an adapter call
private val AdapterCallType = "" /*
* empty since it is not an
* adapter call */
private val AdapterCallType =
"" /* empty since it is not an
* adapter call */
}
<<< binpack=oneline, with trailing-wrapped comment
maxColumn = 100
Expand All @@ -559,16 +559,16 @@ object a {
}
>>>
object a {
val requestedIds =
Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' and 'ATT' only
CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete),
CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), /* Next 2 will
* not be taken into account since 'filterDoNotSellBy' is not set */
CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell),
CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell)
/* Only last record will be taken into account but not the next one since filtering of 'ATT'
* IDs is requsted for MD5 and SHA1 and not for LUID */
)
val requestedIds = Seq( /* First 2 will not be taken into account since this app for 'Do Not
* Sell' and 'ATT' only */
CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete),
CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), /* Next 2 will
* not be taken into account since 'filterDoNotSellBy' is not set */
CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell),
CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell)
/* Only last record will be taken into account but not the next one since filtering of 'ATT'
* IDs is requsted for MD5 and SHA1 and not for LUID */
)
}
<<< binpack=oneline, with trailing-wrapped comment, narrower
maxColumn = 90
Expand All @@ -589,16 +589,16 @@ object a {
}
>>>
object a {
val foo =
Seq( // First 2 will not be taken into account since this app for 'Do Not Sell'
CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete),
CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), /* Next
* 2 will not be taken into account since 'filterDoNotSellBy' is not set */
CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell),
CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell)
/* Only last record will be taken into account but not the next one since filtering
* of 'ATT' IDs is requsted for MD5 and SHA1 and not for LUID */
)
val foo = Seq( /* First 2 will not be taken into account since this app for 'Do Not
* Sell' */
CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete),
CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), /* Next 2
* will not be taken into account since 'filterDoNotSellBy' is not set */
CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell),
CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell)
/* Only last record will be taken into account but not the next one since filtering of
* 'ATT' IDs is requsted for MD5 and SHA1 and not for LUID */
)
}
<<< binpack=oneline, standalone-wrapped comment, narrower
maxColumn = 90
Expand Down

0 comments on commit 7cee034

Please sign in to comment.