Basic support for new .mill and .mill.scala source files
#6752
Conversation
|
We might also need to update the Scala syntax extension and Metals vs code extension |
|
@lolgab are you going to continue with this? If not I might pick it up and try to get it over the finish line |
|
@lihaoyi You can complete it. Sorry for holding it so long. I was wanting for the extensions discussion to stabilize, but then I forgot about it. |
|
@lolgab no worries, I decided to put a bounty on it (com-lihaoyi/mill#3664), in case you or anyone else wants to give it a try. If nobody does I'll take a crack at it myself |
|
@lihaoyi Ok, let me try to finish it. |
| @@ -46,7 +46,7 @@ class SemanticdbTextDocumentProvider( | |||
|
|
|||
| val explicitDialect = if (filePath.isSbt) { | |||
| Some(dialects.Sbt1) | |||
| } else if (filePath.isScalaScript) { | |||
| } else if (filePath.isMill || filePath.isScalaScript) { | |||
There was a problem hiding this comment.
Even though we try to consider .mill not scala scripts, they are actually Scala 2.13 files with top level terms, so I believe this should stay.
lolgab
changed the title
.mill source files.mill and .mill.scala source files
| // hack for mill build sources until a proper BSP wrapped sources mapping gets into | ||
| // the official protocol |
There was a problem hiding this comment.
@tgodzik I managed to recreate MappedSources without BSP. The values seem to be set correctly, but still metals doesn't work correctly...
I think I need some help.
There was a problem hiding this comment.
Do we want to add the source mapping in this PR? I checked and it seems to work worse with it currently even when skipping checking the generated bit.
Maybe it's ok to merge without it for now. Things will work at the current level for Mill, we can improve slowly later.
| generatedSourcesDirectory <- sources | ||
| .find(item => | ||
| item | ||
| .getGenerated() && item.getKind() == b.SourceItemKind.DIRECTORY |
There was a problem hiding this comment.
Looks like it's not actually marked as generated.
There was a problem hiding this comment.
Hmm.... with Mill BSP it's actually generated correctly.
| .iterator() | ||
| .asScala | ||
| .collect { | ||
| case t if t.getName().endsWith("mill-build/") => |
There was a problem hiding this comment.
| case t if t.getName().endsWith("mill-build/") => | |
| case t if t.getName().endsWith("mill-build-") => |
weirdly it's this way for Bloop. With Mill BSP it works, but it seems BSP breaks on newest mill :sigh:
There was a problem hiding this comment.
This code is not production-ready yet. I was hacking it to make source mapping work, but I will leave it to another PR as you suggested.
|
Looks like after the changes the generated code typechecks but sometimes doesn't return completions, not sure why though. Might be worth creating a completion test and investigating, but as I said might not be neccessary for this PR. |
|
And btw. my experiments are here https://github.com/scalameta/metals/compare/main...tgodzik:metals:support-mill-suffix?expand=1 Things look to work most of the time (only the completions seem to be wrong) and those are pretty big hacks |
metals/src/main/scala/scala/meta/internal/metals/codeactions/MillifyDependencyCodeAction.scala
Show resolved
Hide resolved
|
@lolgab I am planning on doing a release soon, should I fix up your PR and merge or do you want to look further? |
|
Go ahead. We can further improve the support later. |
Mill is migrating from the `.sc` suffix to `.mill` to distinguish its files from ammonite and scala-cli script files. `.sc` will be still supported in a migration period
tgodzik
force-pushed
the
support-mill-suffix
branch
from
6160121
to
b9c189d
Compare
Mill is migrating from the
.scsuffix to.millto distinguish its files from ammonite and scala-cli script files..scwill be still supported in a migration periodPR which introduced the new suffix: com-lihaoyi/mill#3426