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

Autoimported object has too many common names in scope #18

Open
jvican opened this issue Oct 18, 2017 · 5 comments
Open

Autoimported object has too many common names in scope #18

jvican opened this issue Oct 18, 2017 · 5 comments

Comments

@jvican
Copy link

jvican commented Oct 18, 2017

autoImport extends several traits that define lots of common operations like currentSbtVersion, currentScalaVersion, etcetera. https://github.com/agemooij/sbt-prompt/blob/master/src/main/scala/com/scalapenos/sbt/prompt/promptlets/promptlets.scala#L16-L36

This causes name clash problems with builds that do define these names in some way or another. In my case, it failed because I had a val currentScalaVersion = "2.12.3" in my build.sbt.

This is an important problem. Sbt autoimports all those names and that's why it's good practice to reduce the names in autoImport as much as possible.

Would it be possible that you remove all those clashing names from this excellent sbt plugin?

@agemooij
Copy link
Owner

Wow, good point! I never thought about that at the time and in the mean time I've actually learned to use autoImport better.

I'll try to fix this ASAP. For extra speed it would be great if you could send a PR.

@agemooij
Copy link
Owner

agemooij commented Oct 20, 2017

Reading up on how I did this, I now remember why it ended up like this.
I wanted all the DSL building block functions, like text and bg, to be auto imported so people could call them to construct prompts without needing an explicit import. The fact that these functions clash with general SBT settings is just me being an idiot and not considering the impact of putting these on the top-level scope.

The obvious solutions would be:

  • rename the relevant functions so they don't clash, perhaps using a prefix
  • or to remove these functions from the top-level autoImport and instead expose them nested under a Promptlets object for namespacing.

I like the second option best. Would you agree @jvican ?

@jvican
Copy link
Author

jvican commented Oct 20, 2017

I think the second option is better @agemooij, it addresses the issue at its root. The fewer names in autoImport, the better 😄. That is a breaking change, it would probably deserve a 2.0 release.

@agemooij
Copy link
Owner

I agree. I'll see if I can find some time to implement this. A PR would certainly help speed things up 😉

@jvican
Copy link
Author

jvican commented Oct 20, 2017

Would love to help, but I don't have time.

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

2 participants