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

Improve Intellij Idea support #351

Merged

Conversation

Baccata
Copy link
Contributor

@Baccata Baccata commented May 27, 2018

Improves the Intellij Idea support in various ways :

  • Cherrypicks the idea conf that need deleting rather than deleting the whole .idea directory. That directory contains elements of configuration like VCS reference that were annoying to set again every time mill regenerated idea config.
  • Attempts to retrieve libraries that the build depends on by inspecting the classloader of the top module
  • Attempts to group jars and sources together in order to have both in the same idea files, which appears to give better jump to definition
  • Hacks the library names for the libraries the build depends on, in order to match Intellij's ammonite support and not show red to the user about libraries that have successfuly been resolved. Also allows to jump to the library sources from the magic import. IntelliJ's magic import resolution (and its ability to not show it as red) is not robust. This seems like a problem to solve on the Intellij side

Solves #347

Improves the Intellij Idea support in various ways :

* Cherrypicks the idea conf that needs deleting rather than deleting
the whole .idea directory. That directory contains elements of
configuration like VCS reference that were annoying to set again
every time mill regenerated idea config.
* Attempts to retrieve libraries that the build depends on by inspecting
the classloader of the top module
* Attempts at grouping jars and sources together in order to have both
in the same idea files, which appears to give better jump to definition
* Hacks the library names for the libraries the build depends on, in
order to match Intellij's ammonite support and not show red to the user
about the library that has successfuly been resolved. Also allows to
jump to the library sources from the magic import.
@@ -34,7 +35,8 @@ object GenIdeaImpl {

val jdkInfo = extractCurrentJdk(pwd / ".idea" / "misc.xml").getOrElse(("JDK_1_8", "1.8 (1)"))

rm! pwd/".idea"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change makes the user's life more comfortable by not asking them to reset various pieces of mill-unrelated configuration

@@ -81,6 +83,19 @@ object GenIdeaImpl {
res.items.toList.map(_.path)
}

val buildDepsPaths = Try(evaluator
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attempts to retrieve magic imports

Tuple2(".idea"/'libraries/s"$name.xml", libraryXmlTemplate(name, url))
}

val buildLibraries = buildLibraryPaths.map{path =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildLibraryPaths is contained by allResolved

@@ -43,8 +43,6 @@ object GenIdeaTests extends TestSuite {
millSourcePath / "generated" / ".idea_modules" /"mill-build.iml",
"gen-idea/idea/libraries/scala-library-2.12.4.jar.xml" ->
millSourcePath / "generated" / ".idea" / "libraries" / "scala-library-2.12.4.jar.xml",
"gen-idea/idea/libraries/scala-library-2.12.4-sources.jar.xml" ->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sources are grouped in the same file as the jar

@lihaoyi
Copy link
Member

lihaoyi commented May 28, 2018

@rockjam feel free to review this one, i might not get to it soon


// Hack so that Intellij does not complain about unresolved magic
// imports in build.sc when in fact they are resolved
def sbtLibraryNameFromPom(pom : Path) : String = {
Copy link
Contributor Author

@Baccata Baccata May 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should maybe remove this, Intellij seems to be flaky as hell with this. It worked on a java library, doesn't seem to work on scala ones ...

Copy link
Contributor Author

@Baccata Baccata May 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to ed958b7. Should they want to, this allows users to avoid the magic appearing in RED by defining their artifacts with ":" rather than "::" (which implies adding _2.12 to the artifact name) .

Note that the magic imports being red does not have any consequence on whether the artifact is actually indexed by IntelliJ and the ide's ability to resolve names coming from this artifact.

@lihaoyi
Copy link
Member

lihaoyi commented May 31, 2018

looks good i guess

@lihaoyi lihaoyi merged commit 1b03026 into com-lihaoyi:master May 31, 2018
@He-Pin
Copy link
Contributor

He-Pin commented May 31, 2018

Thank you very much. @Baccata

@Baccata Baccata deleted the 347-idea-generate-build-deps-config branch June 4, 2018 13:06
@lefou lefou added this to the 0.2.3 milestone Apr 30, 2019
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.

None yet

4 participants