Function value (curlies) should not be split over multiple lines #323

Closed
hseeberger opened this Issue Jun 19, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@hseeberger

Original:

def startSharding(system: ActorSystem, mediator: ActorRef, shardCount: Int): Unit = ClusterSharding(system).start(
  className[Flow],
  props(mediator),
  ClusterShardingSettings(system),
  { case (name: String, payload) => (name, payload) },
  { case (name: String, _) => (name.hashCode % shardCount).toString }
)

Scalafmt:

def startSharding(system: ActorSystem, mediator: ActorRef, shardCount: Int): Unit =
  ClusterSharding(system).start(
      className[Flow],
      props(mediator),
      ClusterShardingSettings(system), {
        case (name: String, payload) => (name, payload)
      }, { case (name: String, _)    => (name.hashCode % shardCount).toString }
  )

I would like the input to look like this:

def startSharding(system: ActorSystem, mediator: ActorRef, shardCount: Int): Unit =
  ClusterSharding(system).start(
      className[Flow],
      props(mediator),
      ClusterShardingSettings(system),
      { case (name: String, payload) => (name, payload) },
      { case (name: String, _)       => (name.hashCode % shardCount).toString }
  )

Using:

  • 0.2.5
  • --style defaultWithAlign --maxColumn 120 --spacesInImportCurlyBraces true
@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jun 19, 2016

Member

I agree that the expected output looks nicer. A quick experiment shows that it should be possible to let scalafmt keep the block on a single line if the block is already on a single line and it fits within the column limit. I'm gonna label this 0.2.7 since there's already lots of issues planned for the 0.2.6 release, which I would like to get out asap.

Member

olafurpg commented Jun 19, 2016

I agree that the expected output looks nicer. A quick experiment shows that it should be possible to let scalafmt keep the block on a single line if the block is already on a single line and it fits within the column limit. I'm gonna label this 0.2.7 since there's already lots of issues planned for the 0.2.6 release, which I would like to get out asap.

@olafurpg olafurpg added this to the 0.2.7 milestone Jun 19, 2016

@hseeberger

This comment has been minimized.

Show comment
Hide comment
@hseeberger

hseeberger Jun 20, 2016

@olafurpg, thanks for the quick response. BTW, I don't expect alignment of => here (like shown in the expected output), but if it works, it would be nice.

@olafurpg, thanks for the quick response. BTW, I don't expect alignment of => here (like shown in the expected output), but if it works, it would be nice.

olafurpg added a commit that referenced this issue Aug 6, 2016

@olafurpg olafurpg self-assigned this Aug 6, 2016

@olafurpg olafurpg added the in progress label Aug 6, 2016

@olafurpg olafurpg removed their assignment Aug 6, 2016

@olafurpg olafurpg removed this from the 0.2.13 milestone Aug 6, 2016

@olafurpg olafurpg added help wanted and removed in progress labels Aug 6, 2016

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Aug 6, 2016

Member

For some reason, I'm struggling to make this work for tricker cases like

  writeRead(
    // Use list because Array has a shitty toString
    { (b: List[Byte], os) => os.writePosVarInt(b.size); os.writeBytes(b.toArray) },
    { is =>
      val bytes = new Array[Byte](is.readPosVarInt)
      is.readFully(bytes)
      bytes.toList
    })

I don't want to spend more time on this issue since I am not so sure how common this coding pattern is. I'm gonna remove the milestone and add the help wanted label. The bug appears to be hidden somewhere deep inside the best-first search, which I try to avoid touching these days. If someone wants to take a stab at it you can work from my 323 branch. However, you have been warned that the best-first search is the hairiest part of the codebase, which I hope to refactor some day or (even better) remove entirely by using dynamic programming like rfmt does.

Member

olafurpg commented Aug 6, 2016

For some reason, I'm struggling to make this work for tricker cases like

  writeRead(
    // Use list because Array has a shitty toString
    { (b: List[Byte], os) => os.writePosVarInt(b.size); os.writeBytes(b.toArray) },
    { is =>
      val bytes = new Array[Byte](is.readPosVarInt)
      is.readFully(bytes)
      bytes.toList
    })

I don't want to spend more time on this issue since I am not so sure how common this coding pattern is. I'm gonna remove the milestone and add the help wanted label. The bug appears to be hidden somewhere deep inside the best-first search, which I try to avoid touching these days. If someone wants to take a stab at it you can work from my 323 branch. However, you have been warned that the best-first search is the hairiest part of the codebase, which I hope to refactor some day or (even better) remove entirely by using dynamic programming like rfmt does.

olafurpg added a commit that referenced this issue Aug 6, 2016

@olafurpg olafurpg added enhancement and removed help wanted labels Aug 6, 2016

@olafurpg olafurpg added this to the 0.2.13 milestone Aug 6, 2016

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Aug 6, 2016

Member

Nevermind my previous comment, I went out for a run and figured out the issue. It had nothing to do with the best-first search :)

Scalafmt will now allow curly block arguments to stay on a single line if they fit, the block will be formatted like before if it doesn't fit on a single line.

Member

olafurpg commented Aug 6, 2016

Nevermind my previous comment, I went out for a run and figured out the issue. It had nothing to do with the best-first search :)

Scalafmt will now allow curly block arguments to stay on a single line if they fit, the block will be formatted like before if it doesn't fit on a single line.

@olafurpg olafurpg closed this in 3cef851 Aug 6, 2016

@hseeberger

This comment has been minimized.

Show comment
Hide comment
@hseeberger

hseeberger Aug 7, 2016

Running always helps ;-)

Running always helps ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment