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

Derived new GenIdea module to support meta-builds #2638

Merged
merged 21 commits into from
Aug 21, 2023
Merged

Derived new GenIdea module to support meta-builds #2638

merged 21 commits into from
Aug 21, 2023

Conversation

lefou
Copy link
Member

@lefou lefou commented Jun 27, 2023

Moved the implementation from mill.scalalib to a new module mill.idea. The reason is simple, we need access to the runner module, but runner itself depends on scalalib to provide compilation support. Generating an IDE setup has a much broader scope so a dedicated module is IMHO the best. This is analog to mill.bsp, which also provides IDE support as a dedicated module.

Deprecated the old mill.scalalib.GenIdea module.

Enhanced the lookup for modules and root modules to reflect the new meta-build capabilities of Mill.

Reworked the XML generator to re-respect the GenIdeaModule.intellijModulePath setting and ensured we combine multiple source folders under the same source root if appropriate.

@lefou lefou force-pushed the genidea-meta branch 2 times, most recently from 7ae829a to 092fce8 Compare June 29, 2023 21:08
@lefou lefou changed the base branch from main to refactor-find-root-modules July 4, 2023 14:49
@lefou lefou force-pushed the refactor-find-root-modules branch from ce3f846 to 552d312 Compare July 14, 2023 07:31
@lefou lefou changed the base branch from refactor-find-root-modules to main August 16, 2023 11:45
@lefou
Copy link
Member Author

lefou commented Aug 18, 2023

@lihaoyi Since we no longer need access to the runner module (due to #2692, which superseded #2648), the newly created idea module seem unnecessary. But I'd like to keep it, as we now have the ability to access ScalaJS/ScalaNative specific things. It's also more convenient to have the GenIdea module not under the binary compatibility guarantees, as it is rather unstructured atm, and there is room for improvement.

@lihaoyi
Copy link
Member

lihaoyi commented Aug 18, 2023

Sure that seems reasonable

@lefou
Copy link
Member Author

lefou commented Aug 20, 2023

This PR is now functional, yet the presentation of the project tree flipped from a more natural project representation to a more synthetic one.

It looks almost arbitrary to me, why and when IntelliJ IDEA decides to use the other mode. But it's reproducible. I saw this kind of layout previously for some other projects, so it's not only determined by the current changes.

This is the current (expected) layout for Mill repo:

grafik

After applying this PR, the project layout looks like this:

grafik

@lihaoyi
Copy link
Member

lihaoyi commented Aug 20, 2023

Could it be caused by differences in the generated XML files? I assume since the XML files are the only data passed from Mill to IntelliJ, any changes in Intellij's behavior must correspond to some change in the XML

@lefou
Copy link
Member Author

lefou commented Aug 21, 2023

Could it be caused by differences in the generated XML files? I assume since the XML files are the only data passed from Mill to IntelliJ, any changes in Intellij's behavior must correspond to some change in the XML

Probably yes, though not directly. I think it may be caused by the missing of a single root module, that sits at the project directory (mill-build is now in mill-build dir). But as I saw this layout also before, with older Mill versions, it could be something different, e.g. overlapping sources or something like this, so that IJ needs to provide a layout that better allows grouping of sources which belong together. This is all just guessing. I haven't found any documentation of the .idea XML files yet.

I will experiment a bit more with a top-level mill-build module.

@lihaoyi
Copy link
Member

lihaoyi commented Aug 21, 2023

Could we create a dummy root module to see if it goes back to the original layout? I've seen the new layout sometimes too, and I find it very unintuitive to not have the whole project's filesystem structure available on the left, e.g. things like the `out/' folder that aren't mill modules but I still need to dig through sometimes

@lefou
Copy link
Member Author

lefou commented Aug 21, 2023

The real difficulty is, that we don't have a specific way to tell IJ where a module lives. In very older versions, we used the millSourcePath or intellijModulePath as content root, but that didn't work well with nested modules (or with generated sources, I don't remember exactly), so we changed that. That may also be the reason why we see this strange layout sometimes.

I'd like to try a few more things, like mapping an empty content root, or mapping the RootModule of frame 0, in case it is not already a JavaModule.

@lefou
Copy link
Member Author

lefou commented Aug 21, 2023

I think I have found a nice compromise.

I now add an additional but empty content root for each module, pointing at the intellijModulePath. The result looks as follows:

grafik

You can see the restored natural Project view layout. Please note, that we now also have the tree nodes pointing to the various modules (not their sources) in bold, which might be unusual but IMHO isn't a bad thing.

@lefou lefou marked this pull request as ready for review August 21, 2023 15:09
@lefou
Copy link
Member Author

lefou commented Aug 21, 2023

I reworked the generated modules a bit more and consolidated all content roots. I also tested this with a large-ish production-grade project, which uses lots of Mill plugins, meta-builds and customizations. Project layout, code navigation and meta-builds all work as expected and much better than before.

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.

Meta-Build is ignored by GenIdea
2 participants