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

revamp the sbt plugin #1081

Closed
fommil opened this issue Nov 25, 2017 · 6 comments
Closed

revamp the sbt plugin #1081

fommil opened this issue Nov 25, 2017 · 6 comments
Assignees

Comments

@fommil
Copy link

fommil commented Nov 25, 2017

The default sbt-scalafmt plugin is not fit for purpose: there is no configuration and it blindly formats everything (no concept of unmanaged sources, etc, etc, although this can all be worked around manually). These problems were addressed in neo-sbt-scalafmt but it is really slow.

Now that sbt 1.0 supports scala 2.12 there is no need for the classloader hacks in https://github.com/lucidsoftware/neo-sbt-scalafmt/blob/master/sbt-scalafmt/src/main/scala/com/lucidchart/sbt/scalafmt/ScalafmtCorePlugin.scala

The 1.0 plugin should depend on scalafmt-core directly, no coursier or CLI nonsense, and retain the following Settings / Tasks (which are scoped per project / config) from the neo plugin:

  • scalafmt
  • scalafmtOnCompile
  • scalafmtConfig
  • scalafmt:test but renamed to scalafmtCheck to avoid creating a config
  • sbt:scalafmt but renamed to scalafmtSbt with a corresponding scalafmtSbtCheck. This should format *.sbt files (with the appropriate dialect) and the project/*.scala files.
  • scalafmtOnly a new task, which could be easily automated from emacs/vim/etc which takes a single file and formats it. This is ideal for "format on save" hooks.

The first cut can skip all the caching and simply do a brute-force format.

@sjrd
Copy link

sjrd commented Nov 25, 2017

If you plan to base the source detection on unmanagedSources (which I approve), how do you suggest dealing with various configurations? I see two options:

  • Either scalafmt and the other tasks are scoped per config (so it's actually compile:scalafmt), which makes it easy to depend on sources in (Compile, scalafmt) itself depending on unmanagedSourceDirectories in (Compile, scalafmt), but means I need to explicitly perform test:scalafmt to format my test suite,
  • Or scalafmt and the other tasks are scoped in the Zero config, which means I only need scalafmt to format main/ and test/, but makes it unclear where to fetch the sources from. In particular, how to deal with the it: configuration, or worse, custom configurations?

@fommil
Copy link
Author

fommil commented Nov 25, 2017

Yup, first choice... compile:scalafmt etc. In my projects at work I create an alias fmt which formats all known configurations and I use format on compile so I never in reality need to run it (CI runs the fmtTest alias).

@olafurpg
Copy link
Member

Very nice! I approve 👍 @leonardehrenfried may also be interested.

Some practicalities, com.geirsson:sbt-scalafmt is currently released for sbt 1.0 without using any classloading/coursier hacks. The plugin does very little however, it only exposes a scalafmt command that invokes the cli.

Command.args("scalafmt", "run the scalafmt command line interface.") {

The build runs scripted tests on sbt 1.0 and a pre-release of the plugin is released on every merge to bintray. All that is missing is a more idiomatic sbt user experience :) I personally don't use sbt-scalafmt so I will leave it to you to decide how to manage configurations/unmanagedSources/...

@olafurpg
Copy link
Member

btw, there may be some utilities in scalafmt-cli that are useful for the sbt plugin. We can move them to the core module or the sbt plugin can depend on the cli module, either way you prefer.

@leonardehrenfried
Copy link

I would be quite interested however currently I'm focusing my "speeding up sbt" open source work on trying to bring coursier into sbt itself: coursier/coursier#601 (comment)

I would still be happy to become the plugin's maintainer but cannot actually develop this upgrade myself.

fommil referenced this issue in scalaz/scalaz Jan 1, 2018
@olafurpg
Copy link
Member

olafurpg commented Jan 1, 2018

Fixed in #1085 @fommil ?

@fommil fommil closed this as completed Jan 1, 2018
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

No branches or pull requests

5 participants