From 98fa795a2eb598fafe04128ccf90846e7ae71319 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Mon, 28 Mar 2022 12:23:37 +0200 Subject: [PATCH 1/7] refactor: move Doctor related stuff to one package --- .../scala/scala/meta/internal/metals/{ => doctor}/Doctor.scala | 0 .../meta/internal/metals/{ => doctor}/DoctorExplanation.scala | 0 .../scala/meta/internal/metals/{ => doctor}/DoctorResults.scala | 0 .../internal/{troubleshoot => metals/doctor}/JavaProblem.scala | 0 .../{troubleshoot => metals/doctor}/ProblemResolver.scala | 0 .../internal/{troubleshoot => metals/doctor}/ScalaProblem.scala | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename metals/src/main/scala/scala/meta/internal/metals/{ => doctor}/Doctor.scala (100%) rename metals/src/main/scala/scala/meta/internal/metals/{ => doctor}/DoctorExplanation.scala (100%) rename metals/src/main/scala/scala/meta/internal/metals/{ => doctor}/DoctorResults.scala (100%) rename metals/src/main/scala/scala/meta/internal/{troubleshoot => metals/doctor}/JavaProblem.scala (100%) rename metals/src/main/scala/scala/meta/internal/{troubleshoot => metals/doctor}/ProblemResolver.scala (100%) rename metals/src/main/scala/scala/meta/internal/{troubleshoot => metals/doctor}/ScalaProblem.scala (100%) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala similarity index 100% rename from metals/src/main/scala/scala/meta/internal/metals/Doctor.scala rename to metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/DoctorExplanation.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorExplanation.scala similarity index 100% rename from metals/src/main/scala/scala/meta/internal/metals/DoctorExplanation.scala rename to metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorExplanation.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/DoctorResults.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala similarity index 100% rename from metals/src/main/scala/scala/meta/internal/metals/DoctorResults.scala rename to metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala diff --git a/metals/src/main/scala/scala/meta/internal/troubleshoot/JavaProblem.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/JavaProblem.scala similarity index 100% rename from metals/src/main/scala/scala/meta/internal/troubleshoot/JavaProblem.scala rename to metals/src/main/scala/scala/meta/internal/metals/doctor/JavaProblem.scala diff --git a/metals/src/main/scala/scala/meta/internal/troubleshoot/ProblemResolver.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/ProblemResolver.scala similarity index 100% rename from metals/src/main/scala/scala/meta/internal/troubleshoot/ProblemResolver.scala rename to metals/src/main/scala/scala/meta/internal/metals/doctor/ProblemResolver.scala diff --git a/metals/src/main/scala/scala/meta/internal/troubleshoot/ScalaProblem.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/ScalaProblem.scala similarity index 100% rename from metals/src/main/scala/scala/meta/internal/troubleshoot/ScalaProblem.scala rename to metals/src/main/scala/scala/meta/internal/metals/doctor/ScalaProblem.scala From c764b8ae441919b976d771e1f4e3c5f0a2f41782 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Tue, 29 Mar 2022 08:53:14 +0200 Subject: [PATCH 2/7] feat: add doctor visibility --- .../internal/metals/ClientConfiguration.scala | 2 ++ .../internal/metals/MetalsLanguageServer.scala | 9 +++++++++ .../internal/metals/MetalsServerConfig.scala | 2 ++ .../meta/internal/metals/doctor/Doctor.scala | 16 +++++++++++++++- 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala index e4272d37418..e4453a9c16d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala @@ -162,6 +162,8 @@ case class ClientConfiguration(initialConfig: MetalsServerConfig) { def disableColorOutput(): Boolean = initializationOptions.disableColorOutput.getOrElse(false) + def isVscode: Boolean = MetalsServerConfig.isClientVscode + def codeLenseRefreshSupport(): Boolean = { val codeLenseRefreshSupport: Option[Boolean] = for { capabilities <- clientCapabilities diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index 85e80bd747a..7e0bdcd60af 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -1716,6 +1716,7 @@ class MetalsLanguageServer( .asJavaObject case ServerCommands.RunDoctor() => Future { + doctor.onVisibilityDidChange(true) doctor.executeRunDoctor() }.asJavaObject case ServerCommands.ListBuildTargets() => @@ -1968,6 +1969,14 @@ class MetalsLanguageServer( } } + @JsonNotification("metals/doctorVisibilityDidChange") + def doctorVisibilityDidChange( + params: DoctorVisibilityDidChangeParams + ): CompletableFuture[Unit] = + Future { + doctor.onVisibilityDidChange(params.visible) + }.asJava + @JsonRequest("metals/treeViewChildren") def treeViewChildren( params: TreeViewChildrenParams diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala index 0208cdacfa4..df81f0cb2ab 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala @@ -133,6 +133,8 @@ object MetalsServerConfig { System.getProperty("metals.client") ) + val isClientVscode: Boolean = metalsClientType.contains("vscode") + def base: MetalsServerConfig = MetalsServerConfig() def default: MetalsServerConfig = { metalsClientType.getOrElse("default") match { diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index c7213ba0db3..7e18e8a773e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -40,6 +40,7 @@ final class Doctor( mtagsResolver: MtagsResolver, javaHome: () => Option[String] )(implicit ec: ExecutionContext) { + private val isVisible = new AtomicBoolean(false) private val hasProblems = new AtomicBoolean(false) private val problemResolver = new ProblemResolver( @@ -50,6 +51,11 @@ final class Doctor( () => clientConfig.isTestExplorerProvider() ) + def onVisibilityDidChange(newState: Boolean): Unit = { + pprint.log(newState) + isVisible.set(newState) + } + /** * Returns a full HTML page for the HTTP client. */ @@ -114,7 +120,11 @@ final class Doctor( case DoctorFormat.Html => buildTargetsHtml() } val params = clientCommand.toExecuteCommandParams(output) - languageClient.metalsExecuteClientCommand(params) + val shouldDisplayForVsCode = (clientConfig.isVscode && isVisible.get()) + pprint.log(shouldDisplayForVsCode) + if (shouldDisplayForVsCode || !clientConfig.isVscode) { + languageClient.metalsExecuteClientCommand(params) + } } else { httpServer() match { case Some(server) => @@ -539,3 +549,7 @@ final class Doctor( private val noBuildTargetRecTwo = "Try removing the directories .metals/ and .bloop/, then restart metals And import the build again." } + +case class DoctorVisibilityDidChangeParams( + visible: Boolean +) From 985f4c3ea43e8b4c72f30fc15c8d78f867984b11 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Tue, 29 Mar 2022 10:00:57 +0200 Subject: [PATCH 3/7] feat: add subject to decouple compilations from callbacks --- .../scala/meta/internal/metals/Compilations.scala | 4 ++-- .../internal/metals/MetalsLanguageServer.scala | 15 +++++++++++++-- .../scala/meta/internal/metals/Subject.scala | 7 +++++++ .../meta/internal/metals/doctor/Doctor.scala | 2 -- 4 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/metals/Subject.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala index 677b299e27e..a3723e57341 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala @@ -20,7 +20,7 @@ final class Compilations( classes: BuildTargetClasses, workspace: () => AbsolutePath, languageClient: MetalsLanguageClient, - refreshTestSuites: () => Unit, + compilationCallbacks: Subject[Unit], isCurrentlyFocused: b.BuildTargetIdentifier => Boolean, compileWorksheets: Seq[AbsolutePath] => Future[Unit] )(implicit ec: ExecutionContext) { @@ -222,7 +222,7 @@ final class Compilations( classes.rebuildIndex( targets, () => { - refreshTestSuites() + compilationCallbacks.notifyObservers() if (targets.exists(isCurrentlyFocused)) { languageClient.refreshModel() } diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index 7e0bdcd60af..367c79f0153 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -178,12 +178,14 @@ class MetalsLanguageServer( buffers, buildTargets ) + + private val compilationCallbacks = new Subject[Unit] {} val compilations: Compilations = new Compilations( buildTargets, buildTargetClasses, () => workspace, languageClient, - () => testProvider.refreshTestSuites(), + compilationCallbacks, buildTarget => focusedDocumentBuildTarget.get() == buildTarget, worksheets => onWorksheetChanged(worksheets) ) @@ -424,7 +426,6 @@ class MetalsLanguageServer( report => { didCompileTarget(report) compilers.didCompile(report) - doctor.check() }, onBuildTargetDidCompile = { target => treeView.onBuildTargetDidCompile(target) @@ -613,6 +614,7 @@ class MetalsLanguageServer( () => userConfig, trees ) + testProvider = new TestSuitesProvider( buildTargets, buildTargetClasses, @@ -624,6 +626,12 @@ class MetalsLanguageServer( () => userConfig, languageClient ) + + compilationCallbacks.addObserver { _ => + testProvider.refreshTestSuites() + () + } + semanticDBIndexer = new SemanticdbIndexer( List( referencesProvider, @@ -707,6 +715,7 @@ class MetalsLanguageServer( diagnostics, languageClient ) + doctor = new Doctor( workspace, buildTargets, @@ -720,6 +729,8 @@ class MetalsLanguageServer( mtagsResolver, () => userConfig.javaHome ) + compilationCallbacks.addObserver(_ => doctor.executeRefreshDoctor()) + fileDecoderProvider = new FileDecoderProvider( workspace, compilers, diff --git a/metals/src/main/scala/scala/meta/internal/metals/Subject.scala b/metals/src/main/scala/scala/meta/internal/metals/Subject.scala new file mode 100644 index 00000000000..6be8251b806 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/Subject.scala @@ -0,0 +1,7 @@ +package scala.meta.internal.metals + +trait Subject[S] { + private var observers: List[S => Unit] = Nil + def addObserver(observer: S => Unit): Unit = observers = observer :: observers + def notifyObservers(value: S): Unit = observers.foreach(_.apply(value)) +} diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index 7e18e8a773e..ccc3affb20c 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -52,7 +52,6 @@ final class Doctor( ) def onVisibilityDidChange(newState: Boolean): Unit = { - pprint.log(newState) isVisible.set(newState) } @@ -121,7 +120,6 @@ final class Doctor( } val params = clientCommand.toExecuteCommandParams(output) val shouldDisplayForVsCode = (clientConfig.isVscode && isVisible.get()) - pprint.log(shouldDisplayForVsCode) if (shouldDisplayForVsCode || !clientConfig.isVscode) { languageClient.metalsExecuteClientCommand(params) } From 3b4722dcd112df89606cb1cee871dac534e70686 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Tue, 29 Mar 2022 10:18:33 +0200 Subject: [PATCH 4/7] fix: do not extract refresh tests to subject --- .../scala/meta/internal/metals/Compilations.scala | 6 ++++-- .../meta/internal/metals/MetalsLanguageServer.scala | 11 +++-------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala index 94cf7c5c17b..4ee78bd75f4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala @@ -20,7 +20,8 @@ final class Compilations( classes: BuildTargetClasses, workspace: () => AbsolutePath, languageClient: MetalsLanguageClient, - compilationCallbacks: Subject[Unit], + refreshTestSuites: () => Unit, + compilationCallbacks: Subject[Try[b.CompileResult]], isCurrentlyFocused: b.BuildTargetIdentifier => Boolean, compileWorksheets: Seq[AbsolutePath] => Future[Unit] )(implicit ec: ExecutionContext) { @@ -222,12 +223,13 @@ final class Compilations( val result = compilation.asScala .andThen { case result => updateCompiledTargetState(result) + compilationCallbacks.notifyObservers(result) // See https://github.com/scalacenter/bloop/issues/1067 classes.rebuildIndex( targets, () => { - compilationCallbacks.notifyObservers() + refreshTestSuites() if (targets.exists(isCurrentlyFocused)) { languageClient.refreshModel() } diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index 367c79f0153..3833a00dbd2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -22,6 +22,7 @@ import scala.concurrent.duration.Duration import scala.concurrent.duration._ import scala.util.Failure import scala.util.Success +import scala.util.Try import scala.util.control.NonFatal import scala.meta.internal.bsp.BspConfigGenerationStatus._ @@ -179,12 +180,13 @@ class MetalsLanguageServer( buildTargets ) - private val compilationCallbacks = new Subject[Unit] {} + private val compilationCallbacks = new Subject[Try[b.CompileResult]] {} val compilations: Compilations = new Compilations( buildTargets, buildTargetClasses, () => workspace, languageClient, + () => testProvider.refreshTestSuites(), compilationCallbacks, buildTarget => focusedDocumentBuildTarget.get() == buildTarget, worksheets => onWorksheetChanged(worksheets) @@ -614,7 +616,6 @@ class MetalsLanguageServer( () => userConfig, trees ) - testProvider = new TestSuitesProvider( buildTargets, buildTargetClasses, @@ -626,12 +627,6 @@ class MetalsLanguageServer( () => userConfig, languageClient ) - - compilationCallbacks.addObserver { _ => - testProvider.refreshTestSuites() - () - } - semanticDBIndexer = new SemanticdbIndexer( List( referencesProvider, From 2dfe3fd29500db9ec3f6bbb7dd8e54d79f7a8cf6 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Tue, 29 Mar 2022 14:17:49 +0200 Subject: [PATCH 5/7] feat: add doctorVisibilityProvider to the client options --- .../internal/metals/ClientConfiguration.scala | 3 +- .../metals/InitializationOptions.scala | 9 +++- .../metals/MetalsLanguageServer.scala | 6 ++- .../internal/metals/MetalsServerConfig.scala | 2 - .../meta/internal/metals/doctor/Doctor.scala | 47 ++++++++++--------- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala index e4453a9c16d..8d6bd2f66e9 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala @@ -162,7 +162,8 @@ case class ClientConfiguration(initialConfig: MetalsServerConfig) { def disableColorOutput(): Boolean = initializationOptions.disableColorOutput.getOrElse(false) - def isVscode: Boolean = MetalsServerConfig.isClientVscode + def isDoctorVisibilityProvider(): Boolean = + initializationOptions.doctorVisibilityProvider.getOrElse(false) def codeLenseRefreshSupport(): Boolean = { val codeLenseRefreshSupport: Option[Boolean] = for { diff --git a/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala b/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala index 25ad60f6369..fcc4e54bd77 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala @@ -48,6 +48,7 @@ import org.eclipse.{lsp4j => l} * copy results to the local buffer. * @param disableColorOutput in the situation where your DAP client may not handle color codes in * the output, you can enable this to strip them. + * @param doctorVisibilityProvider if the clients implements `metals/doctorVisibilityDidChange` */ final case class InitializationOptions( compilerOptions: CompilerInitializationOptions, @@ -73,7 +74,8 @@ final case class InitializationOptions( testExplorerProvider: Option[Boolean], openNewWindowProvider: Option[Boolean], copyWorksheetOutputProvider: Option[Boolean], - disableColorOutput: Option[Boolean] + disableColorOutput: Option[Boolean], + doctorVisibilityProvider: Option[Boolean] ) { def doctorFormat: Option[DoctorFormat.DoctorFormat] = doctorProvider.flatMap(DoctorFormat.fromString) @@ -109,6 +111,7 @@ object InitializationOptions { None, None, None, + None, None ) @@ -163,7 +166,9 @@ object InitializationOptions { openNewWindowProvider = jsonObj.getBooleanOption("openNewWindowProvider"), copyWorksheetOutputProvider = jsonObj.getBooleanOption("copyWorksheetOutputProvider"), - disableColorOutput = jsonObj.getBooleanOption("disableColorOutput") + disableColorOutput = jsonObj.getBooleanOption("disableColorOutput"), + doctorVisibilityProvider = + jsonObj.getBooleanOption("doctorVisibilityProvider") ) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index 3833a00dbd2..454f896e938 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -724,7 +724,11 @@ class MetalsLanguageServer( mtagsResolver, () => userConfig.javaHome ) - compilationCallbacks.addObserver(_ => doctor.executeRefreshDoctor()) + compilationCallbacks.addObserver { _ => + if (clientConfig.isDoctorVisibilityProvider()) + doctor.executeRefreshDoctor() + else () + } fileDecoderProvider = new FileDecoderProvider( workspace, diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala index df81f0cb2ab..0208cdacfa4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala @@ -133,8 +133,6 @@ object MetalsServerConfig { System.getProperty("metals.client") ) - val isClientVscode: Boolean = metalsClientType.contains("vscode") - def base: MetalsServerConfig = MetalsServerConfig() def default: MetalsServerConfig = { metalsClientType.getOrElse("default") match { diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index 123893c2d57..66858cbb3af 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -78,7 +78,7 @@ final class Doctor( def executeRunDoctor(): Unit = { executeDoctor( ClientCommands.RunDoctor, - server => { + onServer = server => { Urls.openBrowser(server.address + "/doctor") } ) @@ -102,36 +102,41 @@ final class Doctor( def executeRefreshDoctor(): Unit = { executeDoctor( ClientCommands.ReloadDoctor, - server => { + onServer = server => { server.reload() } ) } + /** + * @param clientCommand RunDoctor or ReloadDoctor + * @param onServer piece of logic that will be executed when http server is enabled + */ private def executeDoctor( clientCommand: ParametrizedCommand[String], onServer: MetalsHttpServer => Unit ): Unit = { - if ( - clientConfig.isExecuteClientCommandProvider && !clientConfig.isHttpEnabled - ) { - val output = clientConfig.doctorFormat match { - case DoctorFormat.Json => buildTargetsJson() - case DoctorFormat.Html => buildTargetsHtml() - } - val params = clientCommand.toExecuteCommandParams(output) - val shouldDisplayForVsCode = (clientConfig.isVscode && isVisible.get()) - if (shouldDisplayForVsCode || !clientConfig.isVscode) { + val isVisibilityProvider = clientConfig.isDoctorVisibilityProvider() + val shouldDisplay = isVisibilityProvider && isVisible.get() + if (shouldDisplay || !isVisibilityProvider) { + if ( + clientConfig.isExecuteClientCommandProvider && !clientConfig.isHttpEnabled + ) { + val output = clientConfig.doctorFormat match { + case DoctorFormat.Json => buildTargetsJson() + case DoctorFormat.Html => buildTargetsHtml() + } + val params = clientCommand.toExecuteCommandParams(output) languageClient.metalsExecuteClientCommand(params) - } - } else { - httpServer() match { - case Some(server) => - onServer(server) - case None => - scribe.warn( - "Unable to run doctor. Make sure `isHttpEnabled` is set to `true`." - ) + } else { + httpServer() match { + case Some(server) => + onServer(server) + case None => + scribe.warn( + "Unable to run doctor. Make sure `isHttpEnabled` is set to `true`." + ) + } } } } From 3e23dec76fed6a71d750c855984de04b03b99384 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Tue, 29 Mar 2022 18:51:29 +0200 Subject: [PATCH 6/7] docs, refactor: apply code review sugestions --- docs/integrations/new-editor.md | 84 +++++++++++-------- .../meta/internal/metals/Compilations.scala | 2 +- .../metals/MetalsLanguageServer.scala | 6 +- .../scala/meta/internal/metals/Subject.scala | 13 ++- 4 files changed, 61 insertions(+), 44 deletions(-) diff --git a/docs/integrations/new-editor.md b/docs/integrations/new-editor.md index 9b72b0bc1b5..bbe563b9b87 100644 --- a/docs/integrations/new-editor.md +++ b/docs/integrations/new-editor.md @@ -99,46 +99,48 @@ about below, and others used to be server properties that have been migrated to The currently available settings for `InitializationOptions` are listed below. -```js - "InitializationOptions": { - "compilerOptions":{ - "completionCommand": string, - "isCompletionItemDetailEnabled": boolean, - "isCompletionItemDocumentationEnabled": boolean, - "isCompletionItemResolve": boolean, - "isHoverDocumentationEnabled": boolean, - "isSignatureHelpDocumentationEnabled": boolean, - "overrideDefFormat": "ascii" | "unicode", - "parameterHintsCommand": string, - "snippetAutoIndent": boolean, +```typescript + interface InitializationOptions: { + compilerOptions: { + completionCommand?: string; + isCompletionItemDetailEnabled?: boolean; + isCompletionItemDocumentationEnabled?: boolean; + isCompletionItemResolve?: boolean; + isHoverDocumentationEnabled?: boolean; + isSignatureHelpDocumentationEnabled?: boolean; + overrideDefFormat?: "ascii" | "unicode"; + parameterHintsCommand?: string; + snippetAutoIndent?: boolean; } - "copyWorksheetOutputProvider": boolean, - "debuggingProvider": boolean, - "decorationProvider": boolean, - "didFocusProvider": boolean, - "disableColorOutput" boolean, - "doctorProvider": "json" | "html", - "executeClientCommandProvider": boolean, - "globSyntax": "vscode" | "uri" - "icons": "octicons" | "vscode" | "unicode", - "inlineDecorationProvider": boolean, - "inputBoxProvider": boolean, - "isExitOnShutdown" : boolean, - "isHttpEnabled": boolean, - "commandInHtmlFormat": "vscode" | "sublime", - "openFilesOnRenameProvider": boolean, - "openNewWindowProvider": boolean, - "quickPickProvider": boolean, - "renameFileThreshold": number, - "slowTaskProvider": boolean, - "statusBarProvider": "on" | "off" | "show-message" | "log-message", - "treeViewProvider": boolean + debuggingProvider?: boolean; + decorationProvider?: boolean; + inlineDecorationProvider?: boolean; + didFocusProvider?: boolean; + doctorProvider?: "json" | "html"; + executeClientCommandProvider?: boolean; + globSyntax?: "vscode" | "uri"; + icons?: "vscode" | "octicons" | "atom" | "unicode"; + inputBoxProvider?: boolean; + isVirtualDocumentSupported?: boolean; + isExitOnShutdown?: boolean; + isHttpEnabled?: boolean; + openFilesOnRenameProvider?: boolean; + quickPickProvider?: boolean; + renameFileThreshold?: number; + slowTaskProvider?: boolean; + statusBarProvider?: "on" | "off" | "log-message" | "show-message"; + treeViewProvider?: boolean; + testExplorerProvider?: boolean; + openNewWindowProvider?: boolean; + copyWorksheetOutputProvider?: boolean; + commandInHtmlFormat?: "vscode" | "sublime"; + doctorVisibilityProvider?: boolean; } ``` You can also always check these in the [`InitializationOptions.scala`](https://github.com/scalameta/metals/blob/main/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala) -file where you'll find all of the options and descriptions. +file where you'll find all of the options and descriptions. Alternatively you can check out the typescript equivalent - [`MetalsInitializationOptions.ts`](https://github.com/scalameta/metals-languageclient/blob/main/src/interfaces/MetalsInitializationOptions.ts) ##### `compilerOptions.completionCommand` @@ -403,6 +405,16 @@ Boolean value signifying whether or not the client supports the Default value: `false` +##### `testExplorerProvider` + +Boolean value to signify whether or not the client implements the Test Explorer. + +##### `doctorVisibilityProvider` + +Boolean value to signify whether or not the client implements the `"metals/doctorVisibilityDidChange"`. +This JSON notification is used to keep track of doctor state. If client implements this provider then Metals server +will send updates to the doctor view. + #### Experimental Capabilities All of the features that used to be set with `experimental` can now all be set @@ -511,6 +523,7 @@ Returns `Hover` for specified text document and position - [lsp spec](https://mi Metals also support an extended version of this method that supports hover for selection range. The extended stucture of request params is the following: + ```ts interface HoverExtParams { textDocument: TextDocumentIdentifier; @@ -520,7 +533,6 @@ interface HoverExtParams { } ``` - ### `workspace/didChangeWatchedFiles` Optional. Metals uses a built-in file watcher for critical functionality such as @@ -1014,7 +1026,7 @@ _Request_: /** * Currenly, only `pattern` field is used for search. * See: https://github.com/scalameta/metals/issues/3234 - */ + */ interface TextSearchQuery { /** * The text pattern to search for. diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala index 4ee78bd75f4..aafdffbea01 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala @@ -223,7 +223,7 @@ final class Compilations( val result = compilation.asScala .andThen { case result => updateCompiledTargetState(result) - compilationCallbacks.notifyObservers(result) + compilationCallbacks.push(result) // See https://github.com/scalacenter/bloop/issues/1067 classes.rebuildIndex( diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index 454f896e938..35ed901d78b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -180,14 +180,14 @@ class MetalsLanguageServer( buildTargets ) - private val compilationCallbacks = new Subject[Try[b.CompileResult]] {} + private val compilationSubject = new Subject[Try[b.CompileResult]] val compilations: Compilations = new Compilations( buildTargets, buildTargetClasses, () => workspace, languageClient, () => testProvider.refreshTestSuites(), - compilationCallbacks, + compilationSubject, buildTarget => focusedDocumentBuildTarget.get() == buildTarget, worksheets => onWorksheetChanged(worksheets) ) @@ -724,7 +724,7 @@ class MetalsLanguageServer( mtagsResolver, () => userConfig.javaHome ) - compilationCallbacks.addObserver { _ => + compilationSubject.subscribe { _ => if (clientConfig.isDoctorVisibilityProvider()) doctor.executeRefreshDoctor() else () diff --git a/metals/src/main/scala/scala/meta/internal/metals/Subject.scala b/metals/src/main/scala/scala/meta/internal/metals/Subject.scala index 6be8251b806..0913ad5f560 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Subject.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Subject.scala @@ -1,7 +1,12 @@ package scala.meta.internal.metals -trait Subject[S] { - private var observers: List[S => Unit] = Nil - def addObserver(observer: S => Unit): Unit = observers = observer :: observers - def notifyObservers(value: S): Unit = observers.foreach(_.apply(value)) +trait Observable[T] { + def subscribe(fn: T => Unit): Unit +} + +final class Subject[T] extends Observable[T] { + private var subscribers: List[T => Unit] = Nil + def subscribe(fn: T => Unit): Unit = + subscribers = fn :: subscribers + def push(value: T): Unit = subscribers.foreach(_.apply(value)) } From f583e8a3f86bb320f66dcddd44769f4779fc86d8 Mon Sep 17 00:00:00 2001 From: Kamil Podsiadlo Date: Tue, 29 Mar 2022 20:03:15 +0200 Subject: [PATCH 7/7] remove subject --- .../scala/meta/internal/metals/Compilations.scala | 4 ++-- .../internal/metals/MetalsLanguageServer.scala | 15 ++++++--------- .../scala/meta/internal/metals/Subject.scala | 12 ------------ 3 files changed, 8 insertions(+), 23 deletions(-) delete mode 100644 metals/src/main/scala/scala/meta/internal/metals/Subject.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala index aafdffbea01..d5ff971e74b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala @@ -21,7 +21,7 @@ final class Compilations( workspace: () => AbsolutePath, languageClient: MetalsLanguageClient, refreshTestSuites: () => Unit, - compilationCallbacks: Subject[Try[b.CompileResult]], + afterCompilation: () => Unit, isCurrentlyFocused: b.BuildTargetIdentifier => Boolean, compileWorksheets: Seq[AbsolutePath] => Future[Unit] )(implicit ec: ExecutionContext) { @@ -223,7 +223,7 @@ final class Compilations( val result = compilation.asScala .andThen { case result => updateCompiledTargetState(result) - compilationCallbacks.push(result) + afterCompilation() // See https://github.com/scalacenter/bloop/issues/1067 classes.rebuildIndex( diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index 35ed901d78b..08fb5edde58 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -22,7 +22,6 @@ import scala.concurrent.duration.Duration import scala.concurrent.duration._ import scala.util.Failure import scala.util.Success -import scala.util.Try import scala.util.control.NonFatal import scala.meta.internal.bsp.BspConfigGenerationStatus._ @@ -167,6 +166,7 @@ class MetalsLanguageServer( buildTargets.addData(mainBuildTargetsData) private val buildTargetClasses = new BuildTargetClasses(buildTargets) + private var doctor: Doctor = _ private val scalaVersionSelector = new ScalaVersionSelector( () => userConfig, @@ -180,14 +180,17 @@ class MetalsLanguageServer( buildTargets ) - private val compilationSubject = new Subject[Try[b.CompileResult]] val compilations: Compilations = new Compilations( buildTargets, buildTargetClasses, () => workspace, languageClient, () => testProvider.refreshTestSuites(), - compilationSubject, + () => { + if (clientConfig.isDoctorVisibilityProvider()) + doctor.executeRefreshDoctor() + else () + }, buildTarget => focusedDocumentBuildTarget.get() == buildTarget, worksheets => onWorksheetChanged(worksheets) ) @@ -280,7 +283,6 @@ class MetalsLanguageServer( var tables: Tables = _ var statusBar: StatusBar = _ private var embedded: Embedded = _ - private var doctor: Doctor = _ var httpServer: Option[MetalsHttpServer] = None var treeView: TreeViewProvider = NoopTreeViewProvider var worksheetProvider: WorksheetProvider = _ @@ -724,11 +726,6 @@ class MetalsLanguageServer( mtagsResolver, () => userConfig.javaHome ) - compilationSubject.subscribe { _ => - if (clientConfig.isDoctorVisibilityProvider()) - doctor.executeRefreshDoctor() - else () - } fileDecoderProvider = new FileDecoderProvider( workspace, diff --git a/metals/src/main/scala/scala/meta/internal/metals/Subject.scala b/metals/src/main/scala/scala/meta/internal/metals/Subject.scala deleted file mode 100644 index 0913ad5f560..00000000000 --- a/metals/src/main/scala/scala/meta/internal/metals/Subject.scala +++ /dev/null @@ -1,12 +0,0 @@ -package scala.meta.internal.metals - -trait Observable[T] { - def subscribe(fn: T => Unit): Unit -} - -final class Subject[T] extends Observable[T] { - private var subscribers: List[T => Unit] = Nil - def subscribe(fn: T => Unit): Unit = - subscribers = fn :: subscribers - def push(value: T): Unit = subscribers.foreach(_.apply(value)) -}