Skip to content
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

Use unmanagedSourceDirectories for sourceDirectories #43

Merged
merged 1 commit into from
Jul 15, 2017

Conversation

fthomas
Copy link
Contributor

@fthomas fthomas commented May 15, 2016

sourceDirectories in scalariformFormat is currently initialized
with List(scalaSource.value) which means that sbt-scalariform only
looks into one source directory per project. But a typical Scala.js
cross project has at least two source directories: js/ and shared/
or jvm/ and shared/. In other projects there are multiple source
directories for different Scala versions.

This change initializes sourceDirectories in scalariformFormat
with unmanagedSourceDirectories which contains all source
directories with manually created sources. This setting always
includes scalaSource because sbt defines it as:

unmanagedSourceDirectories := makeCrossSources(
  scalaSource.value,
  javaSource.value,
  scalaBinaryVersion.value,
  crossPaths.value)

Here are for example both settings for typelevel/cats:

coreJVM/scalaSource
[info] cats/core/.jvm/src/main/scala

coreJVM/unmanagedSourceDirectories
[info] List(
cats/core/.jvm/src/main/scala-2.11,
cats/core/.jvm/src/main/scala,
cats/core/.jvm/src/main/java,
cats/core/src/main/scala)

And since cats has no sources in cats/core/.jvm/src/main/scala
(all sources are in cats/core/src/main/scala), sbt-scalariform
won't format anything.

This change has been tested successfully with one of my projects
and allowed to get rid of this workaround
to pick up all relevant source directories.

`sourceDirectories in scalariformFormat` is currently initialized
with `List(scalaSource.value)` which means that sbt-scalariform only
looks into one source directory per project. But a typical Scala.js
cross project has at least two source directories: js/ and shared/
or jvm/ and shared/. In other projects there are multiple source
directories for different Scala versions.

This change initializes `sourceDirectories in scalariformFormat`
with `unmanagedSourceDirectories` which contains all source
directories with manually created sources. This setting always
includes `scalaSource` because sbt defines it as:

```scala
unmanagedSourceDirectories := makeCrossSources(
  scalaSource.value,
  javaSource.value,
  scalaBinaryVersion.value,
  crossPaths.value)
```

Here are for example both settings for typelevel/cats:

> coreJVM/scalaSource
[info] cats/core/.jvm/src/main/scala

> coreJVM/unmanagedSourceDirectories
[info] List(
  cats/core/.jvm/src/main/scala-2.11,
  cats/core/.jvm/src/main/scala,
  cats/core/.jvm/src/main/java,
  cats/core/src/main/scala)

And since cats has no sources in cats/core/.jvm/src/main/scala
(all sources are in cats/core/src/main/scala), sbt-scalariform
won't format anything.

This change has been tested successfully with one of my projects
and allowed to get rid of this [workaround](https://github.com/fthomas/refined/blob/8e5fad238524dc4bc32401e37d006705f937f280/build.sbt#L337-L340)
to pick up all relevant source directories.
@fthomas
Copy link
Contributor Author

fthomas commented May 15, 2016

It seems that #32 (comment) also suggests to use unmanagedSourceDirectories instead of scalaSource.

@godenji
Copy link
Collaborator

godenji commented Jul 11, 2017

@fthomas looks like you moved on to scalafmt.

Since Scalariform has been updated to 0.2.0 it makes sense to publish a new release for Sbt-Scalariform as well. Did you encounter any problems after submitting this PR, or were things stable wrt to addressing the scala/scala.js issue?

@fthomas
Copy link
Contributor Author

fthomas commented Jul 11, 2017

As far as I remember there where no problems. I did a similar change in the old scalafmt sbt plugin and it didn't caused any trouble there: scalameta/scalafmt#229

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants