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

Circumventing the common declaration of addPlugins #1213

Closed
huntc opened this issue Mar 26, 2014 · 27 comments
Closed

Circumventing the common declaration of addPlugins #1213

huntc opened this issue Mar 26, 2014 · 27 comments
Assignees
Milestone

Comments

@huntc
Copy link

huntc commented Mar 26, 2014

Given the proposed AutoPlugin mechanism where plugins are required to be added, it'd be nice to have a short cut to the following common expressions in build.sbt:

import com.typesafe.sbt.web.SbtWeb

lazy val root = project.in(file(".")).addPlugins(SbtWeb)

For example, if the project is a single module then may be sbt's processing of build.sbt can be magical and automatically perform the above. Why else would you call addSbtPlugin from plugins.sbt if your single module project wasn't going to use it?

Further to the above, and orthogonal, perhaps a short cut to the expression could be provided e.g.:

lazy val root = project.current.addPlugins(SbtWeb)

My major concern though is that all build.sbt files will have at least 3 extra lines over what they used to have. It isn't a massive concern, but if we are able to address it then I think it'll make a good UX.

@jsuereth jsuereth added this to the 0.13.5 milestone Mar 26, 2014
@jsuereth jsuereth self-assigned this Mar 27, 2014
@eed3si9n eed3si9n modified the milestones: 1.1, 0.13.5 Mar 27, 2014
@eed3si9n
Copy link
Member

We want to do this, but it's actually pretty hard to do this safely. Ideally we should have a read-only reference to the rootProject since sbt always supplies one. For now we are punting it to the future.

@huntc
Copy link
Author

huntc commented Mar 27, 2014

OK, but can we please at least have project.current? .in(file(".")) is damn ugly and often repeated.

@jsuereth
Copy link
Member

it's not true that sbt always supplies one. project.current implies things we couldn't support.

e.g. in a project build structure:

project/
   ...
build.sbt
projectA/
    build.sbt
projectB/
    build.sbt

project.current should refer to the local directory of the .sbt file. However, we don't have a safe way to do that.

Adding a rootPlugin method is possible, but leads to issues if someone re-uses that method more than once.

What about a project.in(cwd) option? That's far less of a risk.

@huntc
Copy link
Author

huntc commented Mar 28, 2014

project.in(cwd) is a definite improvement. @jroper - what'd think?

@eed3si9n
Copy link
Member

By the way, officially we're using dot notation or infix? I always write (project in file(.)) following @jsuereth's Coding in Style talks.

@huntc
Copy link
Author

huntc commented Mar 28, 2014

Unsure about "officially"... There's nothing in our internal plugin coding guidelines. :-)

@huntc
Copy link
Author

huntc commented Mar 28, 2014

...but you're probably correct - I should use the project in cwd form. Thus:

lazy val root = (project in cwd).addPlugins(SbtWeb)

... is that what you're recommending?

@eed3si9n
Copy link
Member

To me (project in file(".")) or (project in cwd) reads better.

@huntc
Copy link
Author

huntc commented Apr 10, 2014

@jsuereth and @eed3si9n - I've received two comments on this one now since Play 2.3-M1. The commonality is a desire to express:

lazy val root = (project in cwd).addPlugins(SbtWeb)

as just:

addPlugins(SbtWeb)

@julienrf
Copy link
Contributor

just:

addPlugins(SbtWeb)

Indeed, such a shortcut would be very convenient.

@jsuereth
Copy link
Member

Yeah, I think there may be a way to do it, but we haven't ironed out the best mechanism. My goal is to avoid having to load settings twice, so when we figure out how to do that and a have a nicer API, it'll be there.

@huntc
Copy link
Author

huntc commented Apr 10, 2014

Thanks Josh. In terms of timing I think we need to have something before Play 2.3-RC1 which is due at the end of April. Do you think that's achievable?

I suspect that we will receive too many complaints otherwise.

@jsuereth
Copy link
Member

@huntc Not sure. We'll see what we can do, it depends on how invasive it is to add.

@huntc
Copy link
Author

huntc commented Apr 10, 2014

We could re-instate Play's playJavaSettings and playScalaSettings or some other alias just for Play, but I'd prefer not to...

@jroper
Copy link
Member

jroper commented Apr 11, 2014

Perhaps (project in cwd).addPlugins(PlayScala) reads better, but project.in(cwd).addPlugins(PlayScala) writes better...

Ok... I wanna configure my project...

project

Right, I want the project in the current working directory

project in cwd

Now I want to add a plugin to it

project in cwd.ad

crap...

*backspace*
*backspace*
*backspace*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
*back arrow*
 (
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
*forward arrow*
 ).addPlugins(PlayScala)

Happens to me every time.

@jroper
Copy link
Member

jroper commented Apr 11, 2014

By the way, we could provide this in Play:

lazy val root = playScalaRootProject

playScalaRootProject would be an auto import from the Play plugin:

def playScalaRootProject = (project in cwd).addPlugins(PlayScala)

@jsuereth
Copy link
Member

We decided against something like that because people might think they can call it twice.

Maybe if it were called createPlayScalaRootProject.

IN any case, I'm debating adding addPlugins/disablePlugins directly to the build.sbt dsl...

@jsuereth jsuereth mentioned this issue Apr 11, 2014
@jsuereth
Copy link
Member

OK, so I think the current thinking is we can add a hook in the build.sbt file that lets you add project manipulations against the same project referred to by ThisProject using ThisProject

SO, concretely, a build.sbt for dead simple play project would be:

ThisProject.enablePlugins(PlayScala)  // Assuming we rename add -> enable

ThisProject disablePlugins (LameJoshPlugin)

Because no one wants any of my plugins.

The DSL grammr would then look sort of like this :

import :=  "import" restOfImport EOL
definition := ( "val" | "def" | "lazy val" ) restofScalaExpression
projectManipulation := "ThisProject" restofScalaExpression
setting := scalaExpression
dslLine :=  (definition | projectManipulation | setting) blankLine
buildSbt := import*  dslLine*

Any expression starting with ThisProject is wrapped as:

{ ThisProject: Project => <expr> }

Then we store these functions for each .sbt file in a sequence and apply them to the appropriate projects after load/discovery of .scala + .sbt build files but before applying settings (and autoplugins).

This type of transformation won't be supported in build.scala (nor does it make any sense because in build.scala you already have access to the Project instance directly and just call functions on it).

The reasoning behind ThisProject is that it mirrors the ThisProject usable in the setting DSL, so the term isn't overloaded in a confusing way.

@huntc
Copy link
Author

huntc commented Apr 11, 2014

This looks like the implementation details of sbt leaking through into the UX. I understand that we're trying to attain binary compatibility, but perhaps the version should be 0.14 if it is getting in the way.

My problem is that all settings here are w.r.t. to the current project... and then, for one group of declarations, you must use ThisProject. That doesn't grok IMHO.

@jsuereth
Copy link
Member

It's not a binary compatibility issue necessarily, but how we do loading. The set of enabled plugins is not a setting for the build, because you need its value to determine what settings are used.

There's a class of these "thing which are used to determine which settings are loaded" of which enabledPlugins belongs. What I'm trying to do is make it feel like a setting even if it isn't.

ThisProject.<thing which helps define how settings are loaded> matches the .scala mechanism of:

object SomeBuild extends Build {
   val foo = project.<thing which helps define how settings are loaded>
}

So, even if we were on version 0.14, the issue remains the same.

@havocp
Copy link
Contributor

havocp commented Apr 15, 2014

is it possible to special case "enablePlugins" and "disablePlugins" so you don't need the ThisProject in front? Then the only remaining "noise" is that we have both addSbtPlugin and enablePlugins, but that noise is sort of inevitable since we want to control plugins per-project instead of per-build.

@jsuereth
Copy link
Member

Yeah, it's possible, just ugly, but Maybe that's what we need to do.

@havocp
Copy link
Contributor

havocp commented Apr 15, 2014

A problem with ThisProject is that the name doesn't really communicate the difference between things that have ThisProject in front and things that don't. That is, everything in a build.sbt is already scoped to this project automatically... so why do some have ThisProject? It seems confusing to have ThisProject when everything in the file is already for this project. People could easily start to think that regular settings are not for this project.

Project.current is a little bit prettier since it conveys (to me anyway) that there's a "project object instance" that I need to get a handle to. And it doesn't look like a key scope; ThisProject looks like the scope syntax, but isn't. Still, Project.current probably doesn't make much sense to people who aren't familiar with the .scala setup.

If we want to communicate in the user model that some things are "outside of" settings and happen before we add the settings, then maybe some syntax that better conveys that idea, but I don't know what it would be... maybe a project {} preamble block?

But do we even need to communicate that I wonder? I'm not sure it matters that much to people that we process enablePlugins before we load settings. Mostly things should just work in a way that makes sense....

@jsuereth jsuereth modified the milestones: 0.13.6, 1.1 Apr 17, 2014
@huntc
Copy link
Author

huntc commented May 1, 2014

So in future:

lazy val root = (project in file(".")).addPlugins(SbtWeb)

will become:

enablePlugins(SbtWeb)

?

@jsuereth
Copy link
Member

jsuereth commented May 6, 2014

Note: This required completely redoing the core of the "Load.scala" algorithm for grabbing Project instances from config files, but the meat of it is here: 52e3e6f

Once I flush out any extraneous issues, I'll issue a pull request.

@huntc
Copy link
Author

huntc commented May 8, 2014

Awesome work guys!

@jaceklaskowski
Copy link
Contributor

BTW, lazy val root = (project in file(".")).addPlugins(SbtWeb) can also be lazy val root = project in file(".") addPlugins SbtWeb (that doesn't necessarily help in IDE or @jroper)

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

No branches or pull requests

7 participants