Skip to content

Commit

Permalink
Bump penalty for select chains of length 1.
Browse files Browse the repository at this point in the history
Although the change itself is small, this commit changes thej
formatted output for a significant amount of code. An example
diff can be seen here: olafurpg/scala-repos#14

Fixes #599.
  • Loading branch information
olafurpg committed Dec 17, 2016
1 parent 79e17f8 commit 3dfe0c5
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 5 deletions.
21 changes: 21 additions & 0 deletions core/src/main/scala/org/scalafmt/config/Newlines.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,33 @@ import metaconfig.ConfigReader
* def foo = 2
* }
* )
*
* @param penalizeSingleSelectMultiArgList
* If true, adds a penalty to newlines before a dot starting a select
* chain of length one and argument list. The penalty matches the number
* of arguments to the select chain application.
* {{{
* // If true, favor
* logger.elem(a,
* b,
* c)
* // instead of
* logger
* .elem(a, b, c)
*
* // penalty is proportional to argument count, example:
* logger.elem(a, b, c) // penalty 2
* logger.elem(a, b, c, d) // penalty 3, etc.
* }}}
*
* If false, matches <0.5 behavior. Note. this option may be removed in 1.0.
*/
@ConfigReader
case class Newlines(
neverInDanglingParenthesesSingleLineArgList: Boolean = false,
neverInResultType: Boolean = false,
neverBeforeJsNative: Boolean = false,
sometimesBeforeColonInMethodReturnType: Boolean = true,
penalizeSingleSelectMultiArgList: Boolean = true,
alwaysBeforeCurlyBraceLambdaParams: Boolean = false
)
21 changes: 16 additions & 5 deletions core/src/main/scala/org/scalafmt/internal/Router.scala
Original file line number Diff line number Diff line change
Expand Up @@ -863,12 +863,23 @@ class Router(formatOps: FormatOps) {
val newlinePolicy = breakOnEveryDot
.andThen(penalizeNewlinesInApply.f)
.copy(expire = lastToken.end)
Seq(
Split(NoSplit,
0,
ignoreIf = style.breakChainOnFirstMethodDot && newlines > 0)
val ignoreNoSplit = style.breakChainOnFirstMethodDot && newlines > 0
val chainLengthPenalty =
if (!style.bestEffortInDeeplyNestedCode && // breaks tests.
style.newlines.penalizeSingleSelectMultiArgList &&
chain.length < 2) {
// penalize by the number of arguments in the rhs open apply.
// I know, it's a bit arbitrary, but my manual experiments seem
// to show that it produces OK output.
splitApplyIntoLhsAndArgsLifted(owners(next(next(tok)).right)).map {
case (_, lst) => Math.max(0, lst.length - 1)
}.getOrElse(0)
} else 0
Seq(
Split(NoSplit, 0, ignoreIf = ignoreNoSplit)
.withPolicy(noSplitPolicy),
Split(Newline.copy(acceptNoSplit = true), 2 + nestedPenalty)
Split(Newline.copy(acceptNoSplit = true),
2 + nestedPenalty + chainLengthPenalty)
.withPolicy(newlinePolicy)
.withIndent(2, optimalToken, Left)
)
Expand Down
19 changes: 19 additions & 0 deletions core/src/test/resources/default/Select.stat
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,22 @@ object a {
computeOrReadCheckpoint(partition, context)
})
}
<<< #599
{{
logger
.elem(threp1, threp2, threp3, tax.total.value, tax.usedPersonalDiscount)
}}
>>>
{
{
logger.elem(threp1,
threp2,
threp3,
tax.total.value,
tax.usedPersonalDiscount)
}
}
<<< No args #599
keys.clear() // we need to remove the selected keys from the set, otherwise they remain selected
>>>
keys.clear() // we need to remove the selected keys from the set, otherwise they remain selected

0 comments on commit 3dfe0c5

Please sign in to comment.