PoC for presentation compiler based errors as you type. #7430
Conversation
|
open for early feedback :) /cc @olafurpg |
| compilers.didChange(path) | ||
| compilers | ||
| .didChange(path) | ||
| .map(diagnostics.publishDiagnosticsNotAdjusted(path, _)) |
There was a problem hiding this comment.
Diagnostics coming from the PC are already correctly positioned relative to in-memory buffers. Not sure if there is a cleaner way than this.
| @@ -1221,3 +1222,10 @@ class MetalsGlobal( | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| class MetalsReporter(initSettings: Settings) extends InteractiveReporter { | |||
There was a problem hiding this comment.
Perhaps not the best solution, but the presentation compiler is using this class to populate the problems field in ComplilationUnit. The annoying part is that the compiler needs a reporter in its constructor, and the reporter needs a reference to the compiler so we need to set it after the constructor call
|
Cool idea! 👍 Is this somehow connected with scala/scala3#22259? 🙂 |
… you type. A few notable changes: - used an instance of InteractiveReporter in order to collect problems per compilation unit - implemented MetalsSourceFile to make sure sources are not duplicated (equality was not correctly implemented for virtual files) - Added a method to report un-adjusted diagnostics All of the above can be challenged by the OSS project, we might need to revisit these decisions later.
12b1925 to
db3d16e
Compare
Not exactly... I'm trying to get Metals working for our large in-house monorepo and one direction is to rely less on expensive builds (via the build tool) and instead provide as much feedback as we can via the presentation compiler. I'm familiar with the Scala 2 presentation compiler, having worked extensively with it on the scala-ide project, but that's almost a decade ago... trying to piece together something and see how far we can go. |
| val sources = buildTargets.buildTargetSources(targetId) | ||
| val modifiedFiles = | ||
| buffers.open.filter(buf => sources.exists(buf.startWith)).toList | ||
| pprint.pprintln( |
There was a problem hiding this comment.
debug message, will remove.
| @@ -1318,7 +1333,22 @@ class Compilers( | |||
| targetId: BuildTargetIdentifier | |||
| ): Option[PresentationCompiler] = | |||
| withKeyAndDefault(targetId) { case (key, getCompiler) => | |||
| Option(jcache.computeIfAbsent(key, { _ => getCompiler() }).await) | |||
| val sources = buildTargets.buildTargetSources(targetId) | |||
| val modifiedFiles = | |||
There was a problem hiding this comment.
Is it modified files or any open files within a target? In case of the first option OutlineFilesProvider already has that data.
| @@ -578,6 +597,7 @@ case class ScalaPresentationCompiler( | |||
| val vd = new VirtualDirectory("(memory)", None) | |||
| val settings = new Settings | |||
| settings.Ymacroexpand.value = "discard" | |||
| settings.YpresentationDelay.value = 500 | |||
There was a problem hiding this comment.
Why is this option needed? Will this not cause the other features to become sluggish as well?
A few notable changes:
To try it out:
publishLocalfrom this branch1.5.3-SNAPSHOT(or whatever is the version you built).package.jsonin your Metals installation directory to show the new setting. You should find it in~/.vscode/extensions/scalameta.metals-1.49.0/package.jsonon Mac Os. Then, add the snippet below where it makes sense (for example, after"metals.enableSemanticHighlighting"around line 375.