New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression: spaces.beforeSeqWildcard is gone but it's false #973

Closed
eed3si9n opened this Issue Jun 13, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@eed3si9n

eed3si9n commented Jun 13, 2017

  • Version: 1.0.0-RC2
  • Integration: commandline
  • Configuration:
maxColumn = 100
project.git = true
project.excludeFilters = [ /sbt-test/, /input_sources/, /contraband-scala/ ]

# http://docs.scala-lang.org/style/scaladoc.html recommends the JavaDoc style.
# scala/scala is written that way too https://github.com/scala/scala/blob/v2.12.2/src/library/scala/Predef.scala
docstrings = JavaDoc

# This also seems more idiomatic to include whitespace in import x.{ yyy }
spaces.inImportCurlyBraces = true

# Vertical alignment only => for pattern matching
align.tokens.add = [
  { code = "=>",  owner = "Case"  }
]

Steps

I ran scalafmt like this:

scalafmt

Problem

Scalafmt formats code like this:

       s.remainingCommands match {
         case List()            => exit(true)
         case List(x, xs @ _ *) => runCmd(x, xs.toList)
       }

Expectation

I would like the formatted output to look like this:

       s.remainingCommands match {
         case List()           => exit(true)
         case List(x, xs @ _*) => runCmd(x, xs.toList)
       }

Notes

Ref 300e6f8

-      case FormatToken(Underscore(), Ident("*"), _)
-          if style.spaces.beforeSeqWildcard =>
-        Seq(
-          Split(NoSplit, 0)
-        )
@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jun 13, 2017

Member

Thank you for reporting! It seems the option name was flipped, spaces.beforeSeqWildcard = true meant "no space before seq wildcard". I assumed the reverse. I will try to fix this asap and cut another release (and hopefully find time to finally write up the changelog).

Member

olafurpg commented Jun 13, 2017

Thank you for reporting! It seems the option name was flipped, spaces.beforeSeqWildcard = true meant "no space before seq wildcard". I assumed the reverse. I will try to fix this asap and cut another release (and hopefully find time to finally write up the changelog).

@olafurpg olafurpg added the bug label Jun 13, 2017

@olafurpg olafurpg added this to the v1.0 milestone Jun 13, 2017

olafurpg added a commit to olafurpg/scalafmt that referenced this issue Jun 13, 2017

@olafurpg olafurpg closed this in 600c15d Jun 13, 2017

olafurpg added a commit that referenced this issue Jun 13, 2017

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Jun 16, 2017

I'm still seeing this using 1.0.0-RC3 via sbt plugin.

eed3si9n commented Jun 16, 2017

I'm still seeing this using 1.0.0-RC3 via sbt plugin.

eed3si9n added a commit to eed3si9n/sbt that referenced this issue Jun 16, 2017

sbt-scalafmt 1.3
See scalameta/scalafmt#973 for the formatting change.

eed3si9n added a commit to eed3si9n/sbt that referenced this issue Jun 16, 2017

sbt-scalafmt 1.3
See scalameta/scalafmt#973 for the formatting change.
@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Jun 16, 2017

Could it be an issue on neo-sbt-scalafmt side?

eed3si9n commented Jun 16, 2017

Could it be an issue on neo-sbt-scalafmt side?

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Jun 16, 2017

I just used CLI and I'm saw this diff:

-      case elem @ Elem(prefix, "dependency", attributes, scope, children @ _*)
+      case elem @ Elem(prefix, "dependency", attributes, scope, children @ _ *)

eed3si9n commented Jun 16, 2017

I just used CLI and I'm saw this diff:

-      case elem @ Elem(prefix, "dependency", attributes, scope, children @ _*)
+      case elem @ Elem(prefix, "dependency", attributes, scope, children @ _ *)
@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jun 17, 2017

Member

I am able to reproduce with 1.0.0-RC3 but not on master. scalafmt --build-info shows a commit 27cf302 that's not in master's history. I suspect the release got broken. I just changed the publish script to clean before release and cut a RC4 release.

$ cat 973.scala
object faa {
  x match { case List(x @ _ *) => }
}
$ coursier launch com.geirsson:scalafmt-cli_2.11:1.0.0-RC4 -r sonatype:releases --main org.scalafmt.cli.Cli -- --stdout 973.scala
object faa {
  x match { case List(x @ _*) => }
}
Member

olafurpg commented Jun 17, 2017

I am able to reproduce with 1.0.0-RC3 but not on master. scalafmt --build-info shows a commit 27cf302 that's not in master's history. I suspect the release got broken. I just changed the publish script to clean before release and cut a RC4 release.

$ cat 973.scala
object faa {
  x match { case List(x @ _ *) => }
}
$ coursier launch com.geirsson:scalafmt-cli_2.11:1.0.0-RC4 -r sonatype:releases --main org.scalafmt.cli.Cli -- --stdout 973.scala
object faa {
  x match { case List(x @ _*) => }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment