From 7dde85eb2d410d888240f6bb005a2d43d6bfcacb Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 19 Feb 2024 15:05:24 +0100 Subject: [PATCH 01/12] use slow task instead of status bar --- .../src/main/scala/bench/PcBenchmark.scala | 9 +- .../meta/internal/bsp/BspConnector.scala | 6 +- .../internal/builds/NewProjectProvider.scala | 8 +- .../meta/internal/builds/ShellRunner.scala | 6 +- .../meta/internal/metals/Compilers.scala | 6 +- .../scala/meta/internal/metals/Embedded.scala | 4 +- .../internal/metals/FormattingProvider.scala | 3 +- .../metals/ForwardingMetalsBuildClient.scala | 53 +++-- .../scala/meta/internal/metals/Indexer.scala | 3 +- .../internal/metals/MetalsLspService.scala | 22 ++- .../internal/metals/ScalafixProvider.scala | 6 +- .../scala/meta/internal/metals/SlowTask.scala | 111 +++++++++++ .../meta/internal/metals/StatusBar.scala | 181 ++---------------- .../internal/metals/WorkspaceLspService.scala | 13 +- .../internal/metals/ammonite/Ammonite.scala | 4 +- .../language/DelegatingLanguageClient.scala | 2 +- .../internal/metals/debug/DebugProvider.scala | 8 +- .../internal/metals/debug/DebugProxy.scala | 10 +- .../internal/metals/scalacli/ScalaCli.scala | 6 +- .../worksheets/WorksheetProvider.scala | 10 +- .../metals/lsp/DelegatingScalaService.scala | 4 + .../meta/metals/lsp/LanguageServer.scala | 1 + .../scala/meta/metals/lsp/WindowService.scala | 9 + .../main/scala/tests/TestScala3Compiler.scala | 11 +- .../src/test/scala/tests/StatusBarSuite.scala | 91 +-------- 25 files changed, 238 insertions(+), 349 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala create mode 100644 metals/src/main/scala/scala/meta/metals/lsp/WindowService.scala diff --git a/metals-bench/src/main/scala/bench/PcBenchmark.scala b/metals-bench/src/main/scala/bench/PcBenchmark.scala index f1fdb90f7e2..b8c76d7c86f 100644 --- a/metals-bench/src/main/scala/bench/PcBenchmark.scala +++ b/metals-bench/src/main/scala/bench/PcBenchmark.scala @@ -8,13 +8,10 @@ import scala.concurrent.ExecutionContextExecutor import scala.meta.internal.jdk.CollectionConverters._ import scala.meta.internal.metals import scala.meta.internal.metals.ClasspathSearch -import scala.meta.internal.metals.ClientConfiguration import scala.meta.internal.metals.Embedded import scala.meta.internal.metals.ExcludedPackagesHandler import scala.meta.internal.metals.MtagsBinaries import scala.meta.internal.metals.MtagsResolver -import scala.meta.internal.metals.StatusBar -import scala.meta.internal.metals.Time import scala.meta.internal.metals.clients.language.NoopLanguageClient import scala.meta.internal.pc.ScalaPresentationCompiler import scala.meta.io.AbsolutePath @@ -33,11 +30,7 @@ abstract class PcBenchmark { protected implicit val ec: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global protected val embedded = new Embedded( - new StatusBar( - NoopLanguageClient, - Time.system, - clientConfig = ClientConfiguration.default, - ) + new metals.SlowTask(NoopLanguageClient) ) protected final val benchmarkedScalaVersions: Array[String] = Array( diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index fce42244f39..cfd495b0f5b 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -17,6 +17,7 @@ import scala.meta.internal.metals.BuildServerConnection import scala.meta.internal.metals.Messages import scala.meta.internal.metals.Messages.BspSwitch import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.Tables import scala.meta.internal.metals.UserConfiguration @@ -36,6 +37,7 @@ class BspConnector( tables: Tables, userConfig: () => UserConfiguration, statusBar: StatusBar, + slowTaskProvider: SlowTask, bspConfigGenerator: BspConfigGenerator, currentConnection: () => Option[BuildServerConnection], restartBspServer: () => Future[Boolean], @@ -129,8 +131,8 @@ class BspConnector( if (shouldReload) connection.workspaceReload() else Future.successful(()) } yield connection - statusBar - .trackFuture("Connecting to sbt", connectionF, showTimer = true) + slowTaskProvider + .trackFuture("Connecting to sbt", connectionF)//TODO: show time .map(Some(_)) case ResolvedBspOne(details) => tables.buildServers.chooseServer(details.getName()) diff --git a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala index c4092f5dc4a..a60c17811f5 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala @@ -13,7 +13,7 @@ import scala.meta.internal.metals.ClientConfiguration import scala.meta.internal.metals.Icons import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.StatusBar +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.clients.language.MetalsInputBoxParams import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.metals.clients.language.MetalsOpenWindowParams @@ -26,11 +26,11 @@ import coursierapi._ class NewProjectProvider( client: MetalsLanguageClient, - statusBar: StatusBar, + slowTaskProvider: SlowTask, config: ClientConfiguration, shell: ShellRunner, icons: Icons, - workspace: AbsolutePath, + workspace: AbsolutePath )(implicit context: ExecutionContext) { private val templatesUrl = @@ -46,7 +46,7 @@ class NewProjectProvider( if (allTemplates.nonEmpty) { allTemplates } else { - statusBar.trackBlockingTask( + slowTaskProvider.trackBlocking( "Fetching template information from Github" ) { // Matches: diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index c389d600d13..99294a957e5 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -15,7 +15,7 @@ import scala.meta.internal.metals.JavaBinary import scala.meta.internal.metals.JdkSources import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable -import scala.meta.internal.metals.StatusBar +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer import scala.meta.internal.metals.clients.language.MetalsLanguageClient @@ -29,7 +29,7 @@ import coursierapi._ class ShellRunner( languageClient: MetalsLanguageClient, time: Time, - statusBar: StatusBar, + slowTaskProvider: SlowTask, )(implicit executionContext: scala.concurrent.ExecutionContext ) extends Cancelable { @@ -133,7 +133,7 @@ class ShellRunner( newCancelables.foreach(cancelables.add) val processFuture = ps.complete - statusBar.trackFuture( + slowTaskProvider.trackFuture( s"Running '$commandRun'", processFuture, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala index b76739130db..a6649a246c7 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala @@ -72,7 +72,7 @@ class Compilers( buffers: Buffers, search: SymbolSearch, embedded: Embedded, - statusBar: StatusBar, + slowTaskProvider: SlowTask, sh: ScheduledExecutorService, initializeParams: InitializeParams, excludedPackages: () => ExcludedPackagesHandler, @@ -938,7 +938,7 @@ class Compilers( } yield { jworksheetsCache.put( path, - statusBar.trackBlockingTask( + slowTaskProvider.trackBlocking( s"${config.icons.sync}Loading worksheet presentation compiler" ) { ScalaLazyCompiler.forWorksheet( @@ -1001,7 +1001,7 @@ class Compilers( val out = jcache.computeIfAbsent( PresentationCompilerKey.ScalaBuildTarget(scalaTarget.info.getId), { _ => - statusBar.trackBlockingTask( + slowTaskProvider.trackBlocking( s"${config.icons.sync}Loading presentation compiler" ) { ScalaLazyCompiler(scalaTarget, mtags, search) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala b/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala index b94b0a0cf83..29a4b3d42a0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala @@ -27,7 +27,7 @@ import mdoc.interfaces.Mdoc * - mdoc */ final class Embedded( - statusBar: StatusBar + slowTaskProvider: SlowTask ) extends Cancelable { private val mdocs: TrieMap[String, URLClassLoader] = @@ -49,7 +49,7 @@ final class Embedded( if (isScala3) Some(scalaVersion) else None val classloader = mdocs.getOrElseUpdate( scalaVersionKey, - statusBar.trackSlowTask("Preparing worksheets") { + slowTaskProvider.trackBlocking("Preparing worksheets") { newMdocClassLoader(scalaBinaryVersion, resolveSpecificVersionCompiler) }, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala index fd95b12c8ec..d19f61f31ac 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala @@ -48,6 +48,7 @@ final class FormattingProvider( client: MetalsLanguageClient, clientConfig: ClientConfiguration, statusBar: StatusBar, + slowTaskProvider: SlowTask, icons: Icons, tables: Tables, buildTargets: BuildTargets, @@ -503,7 +504,7 @@ final class FormattingProvider( def downloadOutputStream(): OutputStream = { downloadingScalafmt.trySuccess(()) downloadingScalafmt = Promise() - statusBar.trackSlowFuture( + slowTaskProvider.trackFuture( "Loading Scalafmt", downloadingScalafmt.future, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index b25b998a08e..67816fd923f 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -5,7 +5,7 @@ import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicReference import scala.collection.concurrent.TrieMap -import scala.concurrent.Promise +import scala.concurrent.Future import scala.util.control.NonFatal import scala.meta.internal.builds.BspErrorHandler @@ -16,6 +16,7 @@ import scala.meta.internal.metals.ConcurrentHashSet import scala.meta.internal.metals.Diagnostics import scala.meta.internal.metals.MetalsBuildClient import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.TaskProgress import scala.meta.internal.metals.Time @@ -55,6 +56,7 @@ final class ForwardingMetalsBuildClient( onBuildTargetDidCompile: b.BuildTargetIdentifier => Unit, onBuildTargetDidChangeFunc: b.DidChangeBuildTarget => Unit, bspErrorHandler: BspErrorHandler, + slowTaskProvider: SlowTask, ) extends MetalsBuildClient with Cancelable { @@ -66,13 +68,21 @@ final class ForwardingMetalsBuildClient( ): List[LogForwarder] = { forwarders.getAndUpdate(_.prepended(logForwarder)) } - private case class Compilation( - timer: Timer, - promise: Promise[b.CompileReport], - isNoOp: Boolean, - progress: TaskProgress = TaskProgress.empty, + private class Compilation( + val timer: Timer, + val isNoOp: Boolean, + token: Option[Future[SlowTask.Token]], + taskProgress: TaskProgress = TaskProgress.empty, ) extends TreeViewCompilation { - def progressPercentage = progress.percentage + + def progressPercentage = taskProgress.percentage + + def end(): Unit = token.foreach(slowTaskProvider.endSlowTask) + + def updateProgress(progress: Long, total: Long = 100): Unit = { + taskProgress.update(progress, total) + token.foreach(slowTaskProvider.notifyProgress(_, progressPercentage)) + } } private val compilations = TrieMap.empty[b.BuildTargetIdentifier, Compilation] @@ -109,7 +119,7 @@ final class ForwardingMetalsBuildClient( key <- compilations.keysIterator compilation <- compilations.remove(key) } { - compilation.promise.cancel() + compilation.end() } } @@ -163,23 +173,24 @@ final class ForwardingMetalsBuildClient( } { diagnostics.onStartCompileBuildTarget(target) // cancel ongoing compilation for the current target, if any. - compilations.remove(target).foreach(_.promise.cancel()) + compilations.remove(target).foreach(_.end()) val name = info.getDisplayName - val promise = Promise[b.CompileReport]() val isNoOp = params.getMessage != null && params.getMessage.startsWith( "Start no-op compilation" ) - val compilation = Compilation(new Timer(time), promise, isNoOp) + val token = + if (isNoOp) None + else + Some( + slowTaskProvider.startSlowTask( + s"Compiling $name", + withProgress = true, + ) + ) + val compilation = new Compilation(new Timer(time), isNoOp, token) compilations(task.getTarget) = compilation - - statusBar.trackFuture( - s"Compiling $name", - promise.future, - showTimer = true, - progress = Some(compilation.progress), - ) } case _ => } @@ -201,7 +212,7 @@ final class ForwardingMetalsBuildClient( scribe.error(s"failed to process compile report", e) } val target = report.getTarget - compilation.promise.trySuccess(report) + compilation.end() val name = buildTargets.info(report.getTarget) match { case Some(i) => i.getDisplayName case None => report.getTarget.getUri @@ -264,7 +275,7 @@ final class ForwardingMetalsBuildClient( buildTarget <- buildTargetFromParams report <- compilations.get(buildTarget) } yield { - report.progress.update(params.getProgress, params.getTotal) + report.updateProgress(params.getProgress, params.getTotal) } case "compile-progress" => // "compile-progress" is from sbt, however its progress field is actually a percentage, @@ -273,7 +284,7 @@ final class ForwardingMetalsBuildClient( buildTarget <- buildTargetFromParams report <- compilations.get(buildTarget) } yield { - report.progress.update(params.getProgress, newTotal = 100) + report.updateProgress(params.getProgress) } case _ => } diff --git a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala index fbfbd0399a8..326b907bf6a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -55,6 +55,7 @@ final case class Indexer( executionContext: ExecutionContextExecutorService, tables: Tables, statusBar: () => StatusBar, + slowTaskProvider: SlowTask, timerProvider: TimerProvider, scalafixProvider: () => ScalafixProvider, indexingPromise: () => Promise[Unit], @@ -165,7 +166,7 @@ final case class Indexer( } def profiledIndexWorkspace(check: () => Unit): Future[Unit] = { - val tracked = statusBar().trackFuture( + val tracked = slowTaskProvider.trackFuture( s"Indexing", Future { timerProvider.timedThunk("indexed workspace", onlyIf = true) { diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index f22f5ea1a2d..d30a59053bc 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -134,6 +134,7 @@ class MetalsLspService( folderVisibleName: Option[String], headDoctor: HeadDoctor, bspStatus: BspStatus, + slowTaskProvider: SlowTask ) extends Folder(folder, folderVisibleName, isKnownMetalsProject = true) with Cancelable with TextDocumentService { @@ -172,9 +173,7 @@ class MetalsLspService( private implicit val executionContext: ExecutionContextExecutorService = ec private val embedded: Embedded = register( - new Embedded( - statusBar - ) + new Embedded(slowTaskProvider) ) val tables: Tables = register(new Tables(folder, time)) @@ -418,6 +417,7 @@ class MetalsLspService( onBuildTargetChanges(params) }, bspErrorHandler, + slowTaskProvider ) private val bloopServers: BloopServers = new BloopServers( @@ -446,6 +446,7 @@ class MetalsLspService( tables, () => userConfig, statusBar, + slowTaskProvider, bspConfigGenerator, () => bspSession.map(_.mainConnection), restartBspServer, @@ -547,6 +548,7 @@ class MetalsLspService( languageClient, clientConfig, statusBar, + slowTaskProvider, clientConfig.icons, tables, buildTargets, @@ -600,7 +602,7 @@ class MetalsLspService( buildTargets, languageClient, () => userConfig, - statusBar, + slowTaskProvider, diagnostics, embedded, worksheetPublisher, @@ -619,7 +621,7 @@ class MetalsLspService( buffers, symbolSearch, embedded, - statusBar, + slowTaskProvider, sh, initializeParams, () => excludedPackageHandler, @@ -719,6 +721,7 @@ class MetalsLspService( semanticdbs, compilers, statusBar, + slowTaskProvider, sourceMapper, () => userConfig, testProvider, @@ -730,7 +733,7 @@ class MetalsLspService( buffers, () => userConfig, folder, - statusBar, + slowTaskProvider, compilations, languageClient, buildTargets, @@ -841,7 +844,7 @@ class MetalsLspService( buffers, compilers, compilations, - statusBar, + slowTaskProvider, diagnostics, tables, languageClient, @@ -2434,7 +2437,7 @@ class MetalsLspService( session.importBuilds() } for { - bspBuilds <- statusBar.trackFuture("Importing build", importedBuilds0) + bspBuilds <- slowTaskProvider.trackFuture("Importing build", importedBuilds0) _ = { val idToConnection = bspBuilds.flatMap { bspBuild => val targets = @@ -2474,7 +2477,7 @@ class MetalsLspService( new ScalaCli( () => compilers, compilations, - () => statusBar, + slowTaskProvider, buffers, () => indexer.profiledIndexWorkspace(() => ()), () => diagnostics, @@ -2496,6 +2499,7 @@ class MetalsLspService( executionContext, tables, () => statusBar, + slowTaskProvider, timerProvider, () => scalafixProvider, () => indexingPromise, diff --git a/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala index 95c8758ebad..58b1f981b75 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala @@ -40,7 +40,7 @@ case class ScalafixProvider( buffers: Buffers, userConfig: () => UserConfiguration, workspace: AbsolutePath, - statusBar: StatusBar, + slowTaskProvider: SlowTask, compilations: Compilations, languageClient: MetalsLanguageClient, buildTargets: BuildTargets, @@ -456,7 +456,7 @@ case class ScalafixProvider( scalafixCache.get(scalaBinaryVersion) match { case Some(value) => Future.successful(value) case None => - statusBar.trackBlockingTask("Downloading scalafix") { + slowTaskProvider.trackBlocking("Downloading scalafix") { val scalafix = if (scalaBinaryVersion == "2.11") Future(scala211Fallback) else @@ -491,7 +491,7 @@ case class ScalafixProvider( rulesClassloaderCache.get(scalfixRulesKey) match { case Some(value) => Future.successful(value) case None => - statusBar.trackBlockingTask( + slowTaskProvider.trackBlocking( "Downloading scalafix rules' dependencies" ) { val rulesDependencies = scalfixRulesKey.usedRulesWithClasspath diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala new file mode 100644 index 00000000000..cad189df63f --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala @@ -0,0 +1,111 @@ +package scala.meta.internal.metals + +import java.util.UUID + +import scala.collection.mutable +import scala.concurrent.ExecutionContext +import scala.concurrent.Future + +import scala.meta.internal.metals.MetalsEnrichments._ + +import org.eclipse.lsp4j.ProgressParams +import org.eclipse.lsp4j.WorkDoneProgressBegin +import org.eclipse.lsp4j.WorkDoneProgressCreateParams +import org.eclipse.lsp4j.WorkDoneProgressEnd +import org.eclipse.lsp4j.WorkDoneProgressNotification +import org.eclipse.lsp4j.WorkDoneProgressReport +import org.eclipse.lsp4j.jsonrpc.messages +import org.eclipse.lsp4j.services.LanguageClient + +class SlowTask(client: LanguageClient)(implicit ec: ExecutionContext) { + type Token = messages.Either[String, Integer] + private def onCancelMap = mutable.Map[Token, () => Unit]() + + def startSlowTask( + message: String, + withProgress: Boolean = false, + onCancel: Option[() => Unit] = None + ): Future[Token] = { + val uuid = UUID.randomUUID().toString() + val token = messages.Either.forLeft[String, Integer](uuid) + + onCancel match { + case Some(onCancel) => + onCancelMap(token) = onCancel + case None => + } + + client + .createProgress(new WorkDoneProgressCreateParams(token)) + .asScala + .map { _ => + val begin = new WorkDoneProgressBegin() + begin.setTitle(message) + if (withProgress) { + begin.setPercentage(0) + } + if (onCancel.isDefined) { + begin.setCancellable(true) + } + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + begin + ) + client.notifyProgress(new ProgressParams(token, notification)) + token + } + } + + def notifyProgress( + token: Future[Token], + percentage: Int, + additionalMessage: Option[String] = None, + ): Future[Unit] = { + token.map{ token => + val report = new WorkDoneProgressReport() + report.setPercentage(percentage) + additionalMessage.foreach(msg => report.setMessage(msg)) + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + report + ) + client.notifyProgress(new ProgressParams(token, notification)) + } + } + + def endSlowTask(token: Future[Token]): Future[Unit] = { + token.map{ token => + val end = messages.Either.forLeft[WorkDoneProgressNotification, Object]( + new WorkDoneProgressEnd() + ) + client.notifyProgress(new ProgressParams(token, end)) + } + } + + //TODO: add possibility to show time + def trackFuture[T]( + message: String, + value: Future[T], + onCancel: Option[() => Unit] = None + )(implicit ec: ExecutionContext): Future[T] = { + val token = startSlowTask(message, onCancel = onCancel) + value.map { result => + //TODO:: probably shouldn't do it if it was already cancelled + endSlowTask(token) + result + } + } + + def trackBlocking[T](message: String)(thunk: => T): T = { + val token = startSlowTask(message) + val result = thunk + endSlowTask(token) + result + } + + def canceled(token: Token): Unit = onCancelMap.get(token).foreach(_()) +} + +object SlowTask { + type Token = messages.Either[String, Integer] +} diff --git a/metals/src/main/scala/scala/meta/internal/metals/StatusBar.scala b/metals/src/main/scala/scala/meta/internal/metals/StatusBar.scala index 6ccc7a59f51..523ee4c9147 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/StatusBar.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/StatusBar.scala @@ -1,33 +1,15 @@ package scala.meta.internal.metals -import java.util.UUID import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ScheduledFuture import java.util.concurrent.TimeUnit -import java.util.concurrent.atomic.AtomicInteger -import scala.concurrent.ExecutionContext -import scala.concurrent.Future -import scala.concurrent.Promise -import scala.util.Failure -import scala.util.Success import scala.util.control.NonFatal import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.clients.language.MetalsLanguageClient -import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams import scala.meta.internal.metals.clients.language.MetalsStatusParams -import scala.meta.internal.metals.config.StatusBarState - -import org.eclipse.lsp4j.MessageParams -import org.eclipse.lsp4j.MessageType -import org.eclipse.lsp4j.ProgressParams -import org.eclipse.lsp4j.WorkDoneProgressBegin -import org.eclipse.lsp4j.WorkDoneProgressCreateParams -import org.eclipse.lsp4j.WorkDoneProgressEnd -import org.eclipse.lsp4j.WorkDoneProgressNotification -import org.eclipse.lsp4j.jsonrpc.messages /** * Manages sending metals/status notifications to the editor client. @@ -45,105 +27,7 @@ import org.eclipse.lsp4j.jsonrpc.messages final class StatusBar( client: MetalsLanguageClient, time: Time, - progressTicks: ProgressTicks = ProgressTicks.braille, - clientConfig: ClientConfiguration, -)(implicit ec: ExecutionContext) - extends Cancelable { - def trackBlockingTask[T](message: String)(thunk: => T): T = { - val promise = Promise[Unit]() - trackFuture(message, promise.future) - try { - thunk - } finally { - promise.trySuccess(()) - } - } - - def trackSlowTask[T](message: String)(thunk: => T): T = { - if (!clientConfig.slowTaskIsOn) - trackBlockingTask(message)(thunk) - else { - val task = client.metalsSlowTask(MetalsSlowTaskParams(message)) - try { - thunk - } catch { - case NonFatal(e) => - slowTaskFailed(message, e) - throw e - } finally { - task.cancel(true) - } - } - } - - def trackSlowFuture[T]( - message: String, - thunk: Future[T], - onCancel: () => Unit = () => (), - ): Future[T] = { - if (!clientConfig.slowTaskIsOn) - trackFuture(message, thunk) - else { - val task = client.metalsSlowTask(MetalsSlowTaskParams(message)) - task.thenApply { response => - if (response.cancel) onCancel() - } - thunk.onComplete { - case Failure(exception) => - slowTaskFailed(message, exception) - task.cancel(true) - case Success(_) => - task.cancel(true) - } - thunk - } - } - - private def slowTaskFailed(message: String, e: Throwable): Unit = { - scribe.error(s"failed: $message", e) - client.logMessage( - new MessageParams( - MessageType.Error, - s"$message failed, see metals.log for more details.", - ) - ) - } - - def trackFuture[T]( - message: String, - value: Future[T], - showTimer: Boolean = false, - progress: Option[TaskProgress] = None, - ): Future[T] = { - - if (clientConfig.statusBarState == StatusBarState.Off) { - val uuid = UUID.randomUUID().toString() - val token = messages.Either.forLeft[String, Integer](uuid) - - val begin = new WorkDoneProgressBegin() - begin.setTitle(message) - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - begin - ) - - client.createProgress(new WorkDoneProgressCreateParams(token)) - client.notifyProgress(new ProgressParams(token, notification)) - - value.map { result => - val end = messages.Either.forLeft[WorkDoneProgressNotification, Object]( - new WorkDoneProgressEnd() - ) - client.notifyProgress(new ProgressParams(token, end)) - result - } - } else { - items.add(Progress(message, value, showTimer, progress)) - tickIfHidden() - - value - } - } +) extends Cancelable { def addMessage(params: MetalsStatusParams): Unit = { items.add(Message(params)) @@ -154,6 +38,7 @@ final class StatusBar( } private var scheduledFuture: ScheduledFuture[_] = _ + def start( sh: ScheduledExecutorService, initialDelay: Long, @@ -176,20 +61,11 @@ final class StatusBar( garbageCollect() mostRelevant() match { case Some(value) => - val isUnchanged = activeItem.exists { - // Don't re-publish static messages. - case m: Message => m eq value - case _ => false - } + val isUnchanged = activeItem.exists(_ eq value) if (!isUnchanged) { activeItem = Some(value) val show: java.lang.Boolean = if (isHidden) true else null - val params = value match { - case Message(p) => - p.copy(show = show) - case _ => - MetalsStatusParams(value.formattedMessage, show = show) - } + val params = value.params.copy(show = show) value.show() client.metalsStatus(params) isHidden = false @@ -203,13 +79,9 @@ final class StatusBar( } } - private var activeItem: Option[Item] = None - private def isActiveMessage: Boolean = - activeItem.exists { - case m: Message => !m.isOutdated - case _ => false - } - private sealed abstract class Item { + private var activeItem: Option[Message] = None + private def isActiveMessage: Boolean = activeItem.exists(!_.isOutdated) + private case class Message(params: MetalsStatusParams) { val timer = new Timer(time) private var firstShow: Option[Timer] = None def show(): Unit = { @@ -219,55 +91,24 @@ final class StatusBar( } def priority: Long = timer.elapsedNanos def isRecent: Boolean = timer.elapsedSeconds < 3 - private val dots = new AtomicInteger() - def formattedMessage: String = - this match { - case Message(value) => value.text - case Progress(message, _, showTimer, maybeProgress) => - if (showTimer) { - val seconds = timer.elapsedSeconds - if (seconds == 0) { - s"$message " - } else { - maybeProgress match { - case Some(TaskProgress(percentage)) if seconds > 3 => - s"$message ${Timer.readableSeconds(seconds)} ($percentage%)" - case _ => - s"$message ${Timer.readableSeconds(seconds)}" - } - } - } else { - message + progressTicks.format(dots.getAndIncrement()) - } - } + def formattedMessage: String = params.text def isOutdated: Boolean = timer.elapsedSeconds > 10 || firstShow.exists(_.elapsedSeconds > 5) - def isStale: Boolean = - this match { - case _: Message => (firstShow.isDefined && !isRecent) || isOutdated - case p: Progress => p.job.isCompleted - } + def isStale: Boolean = (firstShow.isDefined && !isRecent) || isOutdated } - private case class Message(params: MetalsStatusParams) extends Item - private case class Progress( - message: String, - job: Future[_], - showTimer: Boolean, - taskProgress: Option[TaskProgress], - ) extends Item private var isHidden: Boolean = true private def tickIfHidden(): Unit = { if (isHidden) tick() } - private val items = new ConcurrentLinkedQueue[Item]() + private val items = new ConcurrentLinkedQueue[Message]() def pendingItems: Iterable[String] = items.asScala.map(_.formattedMessage) private def garbageCollect(): Unit = { items.removeIf(_.isStale) } - private def mostRelevant(): Option[Item] = { + private def mostRelevant(): Option[Message] = { if (items.isEmpty) None else { Some(items.asScala.maxBy { item => (item.isRecent, item.priority) }) diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index d869ef646ee..2c4fa005163 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -123,18 +123,18 @@ class WorkspaceLspService( languageClient } + private val slowTaskProvider = new SlowTask(languageClient) + private val userConfigSync = new UserConfigurationSync(initializeParams, languageClient, clientConfig) val statusBar: StatusBar = new StatusBar( languageClient, time, - progressTicks, - clientConfig, ) private val shellRunner = register { - new ShellRunner(languageClient, time, statusBar) + new ShellRunner(languageClient, time, slowTaskProvider) } var focusedDocument: Option[AbsolutePath] = None @@ -181,6 +181,7 @@ class WorkspaceLspService( name, doctor, bspStatus, + slowTaskProvider ) } @@ -211,7 +212,7 @@ class WorkspaceLspService( private val newProjectProvider: NewProjectProvider = new NewProjectProvider( languageClient, - statusBar, + slowTaskProvider, clientConfig, shellRunner, clientConfig.icons, @@ -613,6 +614,10 @@ class WorkspaceLspService( ): CompletableFuture[ju.List[Location]] = collectSeq(_.findTextInDependencyJars(params))(_.flatten.asJava).asJava + override def didCancelWorkDoneProgress( + params: lsp4j.WorkDoneProgressCancelParams + ): Unit = slowTaskProvider.canceled(params.getToken()) + def doctorVisibilityDidChange( params: DoctorVisibilityDidChangeParams ): CompletableFuture[Unit] = diff --git a/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala b/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala index c4bf6f77884..cd53241c499 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala @@ -40,7 +40,7 @@ final class Ammonite( buffers: Buffers, compilers: Compilers, compilations: Compilations, - statusBar: StatusBar, + slowTaskProvider: SlowTask, diagnostics: Diagnostics, tables: Tables, languageClient: MetalsLanguageClient, @@ -98,7 +98,7 @@ final class Ammonite( case Some(conn) => compilers.cancel() for { - build0 <- statusBar.trackFuture( + build0 <- slowTaskProvider.trackFuture( "Importing Ammonite scripts", ImportedBuild.fromConnection(conn), ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala index e250633651c..499860f37ec 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala @@ -121,7 +121,7 @@ class DelegatingLanguageClient(var underlying: MetalsLanguageClient) underlying.configuration(configurationParams) override def createProgress( - params: WorkDoneProgressCreateParams + params: WorkDoneProgressCreateParams //TODO:: check that has valid capabilities ): CompletableFuture[Void] = underlying.createProgress(params) override def notifyProgress(params: ProgressParams): Unit = diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala index df75e4a6876..0e4ab492b08 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala @@ -38,6 +38,7 @@ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable import scala.meta.internal.metals.ScalaTestSuites import scala.meta.internal.metals.ScalaTestSuitesDebugRequest +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.SourceMapper import scala.meta.internal.metals.StacktraceAnalyzer import scala.meta.internal.metals.StatusBar @@ -85,6 +86,7 @@ class DebugProvider( semanticdbs: Semanticdbs, compilers: Compilers, statusBar: StatusBar, + slowTaskProvider: SlowTask, sourceMapper: SourceMapper, userConfig: () => UserConfiguration, testProvider: TestSuitesProvider, @@ -142,7 +144,7 @@ class DebugProvider( ) debugServer <- if (isJvm) - statusBar.trackSlowFuture( + slowTaskProvider.trackFuture( "Starting debug server", start( sessionName, @@ -150,7 +152,7 @@ class DebugProvider( buildServer, cancelPromise, ), - () => cancelPromise.trySuccess(()), + Some(() => cancelPromise.trySuccess(())), ) else runLocally( @@ -296,7 +298,7 @@ class DebugProvider( compilers, workspace, clientConfig.disableColorOutput(), - statusBar, + slowTaskProvider, sourceMapper, compilations, targets, diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala index a46d61f01d6..176db6ed3ac 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala @@ -19,9 +19,9 @@ import scala.meta.internal.metals.Compilers import scala.meta.internal.metals.EmptyCancelToken import scala.meta.internal.metals.JsonParser._ import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.SourceMapper import scala.meta.internal.metals.StacktraceAnalyzer -import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.Trace import scala.meta.internal.metals.debug.DebugProtocol.CompletionRequest import scala.meta.internal.metals.debug.DebugProtocol.DisconnectRequest @@ -55,7 +55,7 @@ private[debug] final class DebugProxy( stackTraceAnalyzer: StacktraceAnalyzer, compilers: Compilers, stripColor: Boolean, - statusBar: StatusBar, + slowTaskProvider: SlowTask, sourceMapper: SourceMapper, compilations: Compilations, targets: Seq[BuildTargetIdentifier], @@ -94,7 +94,7 @@ private[debug] final class DebugProxy( case _ if cancelled.get() => () // ignore case request @ InitializeRequest(args) => - statusBar.trackFuture( + slowTaskProvider.trackFuture( "Initializing debugger", initialized.future, ) @@ -313,7 +313,7 @@ private[debug] object DebugProxy { compilers: Compilers, workspace: AbsolutePath, stripColor: Boolean, - status: StatusBar, + slowTaskProvider: SlowTask, sourceMapper: SourceMapper, compilations: Compilations, targets: Seq[BuildTargetIdentifier], @@ -340,7 +340,7 @@ private[debug] object DebugProxy { stackTraceAnalyzer, compilers, stripColor, - status, + slowTaskProvider, sourceMapper, compilations, targets, diff --git a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala index 305086eceda..8af463f12dd 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala @@ -34,8 +34,8 @@ import scala.meta.internal.metals.ImportedBuild import scala.meta.internal.metals.MetalsBuildClient import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MetalsServerConfig +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.SocketConnection -import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.Tables import scala.meta.internal.metals.TargetData import scala.meta.internal.metals.UserConfiguration @@ -50,7 +50,7 @@ import coursier.core.Version class ScalaCli( compilers: () => Compilers, compilations: Compilations, - statusBar: () => StatusBar, + slowTaskProvider: SlowTask, buffers: Buffers, indexWorkspace: () => Future[Unit], diagnostics: () => Diagnostics, @@ -112,7 +112,7 @@ class ScalaCli( ifConnectedOrElse { st => compilers().cancel() - statusBar() + slowTaskProvider .trackFuture( "Importing Scala CLI sources", ImportedBuild.fromConnection(st.connection), diff --git a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala index 7d78efe9109..646298349f5 100644 --- a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala @@ -30,7 +30,7 @@ import scala.meta.internal.metals.Messages import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable import scala.meta.internal.metals.ScalaVersionSelector -import scala.meta.internal.metals.StatusBar +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer import scala.meta.internal.metals.UserConfiguration @@ -66,7 +66,7 @@ class WorksheetProvider( buildTargets: BuildTargets, languageClient: MetalsLanguageClient, userConfig: () => UserConfiguration, - statusBar: StatusBar, + slowTaskProvider: SlowTask, diagnostics: Diagnostics, embedded: Embedded, publisher: WorksheetPublisher, @@ -251,10 +251,10 @@ class WorksheetProvider( ) } cancelables.add(Cancelable(() => completeEmptyResult())) - statusBar.trackFuture( + slowTaskProvider.trackFuture( s"Evaluating ${path.filename}", - result.asScala, - showTimer = true, + result.asScala + // TODO:: showTimer = true, ) token.checkCanceled() // NOTE(olafurpg) Run evaluation in a custom thread so that we can diff --git a/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala b/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala index 490718fe9af..738e5e1cb54 100644 --- a/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala +++ b/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala @@ -221,4 +221,8 @@ class DelegatingScalaService( params: DidChangeWorkspaceFoldersParams ): CompletableFuture[Unit] = underlying.didChangeWorkspaceFolders(params) + override def didCancelWorkDoneProgress( + params: WorkDoneProgressCancelParams + ): Unit = underlying.didCancelWorkDoneProgress(params) + } diff --git a/metals/src/main/scala/scala/meta/metals/lsp/LanguageServer.scala b/metals/src/main/scala/scala/meta/metals/lsp/LanguageServer.scala index a8b40d8a51a..3497181fa6e 100644 --- a/metals/src/main/scala/scala/meta/metals/lsp/LanguageServer.scala +++ b/metals/src/main/scala/scala/meta/metals/lsp/LanguageServer.scala @@ -47,3 +47,4 @@ trait ScalaLspService extends TextDocumentService with WorkspaceService with MetalsService + with WindowService diff --git a/metals/src/main/scala/scala/meta/metals/lsp/WindowService.scala b/metals/src/main/scala/scala/meta/metals/lsp/WindowService.scala new file mode 100644 index 00000000000..2b2c8ca7545 --- /dev/null +++ b/metals/src/main/scala/scala/meta/metals/lsp/WindowService.scala @@ -0,0 +1,9 @@ +package scala.meta.metals.lsp + +import org.eclipse.lsp4j.WorkDoneProgressCancelParams +import org.eclipse.lsp4j.jsonrpc.services.JsonNotification + +trait WindowService { + @JsonNotification("window/workDoneProgress/cancel") + def didCancelWorkDoneProgress(params: WorkDoneProgressCancelParams): Unit +} diff --git a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala index 69980bc636b..370c7a9d3d2 100644 --- a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala +++ b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala @@ -4,16 +4,13 @@ import scala.concurrent.ExecutionContext import scala.meta.internal.io.PathIO import scala.meta.internal.metals.Buffers -import scala.meta.internal.metals.ClientConfiguration import scala.meta.internal.metals.Embedded import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MtagsBinaries -import scala.meta.internal.metals.ProgressTicks -import scala.meta.internal.metals.StatusBar +import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.{BuildInfo => V} import scala.meta.pc.PresentationCompiler -import tests.FakeTime import tests.InputProperties import tests.TestMtagsResolver import tests.TestingClient @@ -25,13 +22,9 @@ object TestScala3Compiler { val resolver = new TestMtagsResolver() resolver.resolve(V.scala3) match { case Some(mtags: MtagsBinaries.Artifacts) => - val time = new FakeTime val client = new TestingClient(PathIO.workingDirectory, Buffers()) - val status = new StatusBar( + val status = new SlowTask( client, - time, - ProgressTicks.dots, - ClientConfiguration.default, )(ec) val embedded = new Embedded(status) val pc = embedded diff --git a/tests/unit/src/test/scala/tests/StatusBarSuite.scala b/tests/unit/src/test/scala/tests/StatusBarSuite.scala index ece2169cf9d..a817982f698 100644 --- a/tests/unit/src/test/scala/tests/StatusBarSuite.scala +++ b/tests/unit/src/test/scala/tests/StatusBarSuite.scala @@ -1,29 +1,13 @@ package tests -import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.Promise - import scala.meta.internal.io.PathIO import scala.meta.internal.metals.Buffers -import scala.meta.internal.metals.ClientConfiguration -import scala.meta.internal.metals.MetalsServerConfig -import scala.meta.internal.metals.ProgressTicks import scala.meta.internal.metals.StatusBar -import scala.meta.internal.metals.StatusBarConfig - -import org.eclipse.lsp4j.WorkDoneProgressBegin class StatusBarSuite extends BaseSuite { val time = new FakeTime val client = new TestingClient(PathIO.workingDirectory, Buffers()) - var status = new StatusBar( - client, - time, - ProgressTicks.dots, - ClientConfiguration( - MetalsServerConfig.base.copy(statusBar = StatusBarConfig.on) - ), - ) + var status = new StatusBar(client, time) override def beforeEach(context: BeforeEach): Unit = { client.statusParams.clear() status.cancel() @@ -50,77 +34,4 @@ class StatusBarSuite extends BaseSuite { |""".stripMargin, ) } - - test("future") { - val promise = Promise[Unit]() - status.trackFuture("tick", promise.future) - 1.to(7).foreach { _ => tickSecond() } - promise.success(()) - status.tick() - assertNoDiff( - client.statusBarHistory, - """| - | - tick - |tick. - |tick.. - |tick... - |tick - |tick. - |tick.. - |tick... - | - |""".stripMargin, - ) - } - - test("progress") { - - val noStatus = new StatusBar( - client, - time, - ProgressTicks.dots, - ClientConfiguration( - MetalsServerConfig.base.copy(statusBar = StatusBarConfig.off) - ), - ) - - val promise = Promise[Unit]() - noStatus.trackFuture("future", promise.future) - assertEquals(client.workDoneProgressCreateParams.size(), 1) - promise.success(()) - assertEquals( - client.progressParams - .peek() - .getValue() - .getLeft() - .asInstanceOf[WorkDoneProgressBegin] - .getTitle(), - "future", - ) - } - - test("race") { - val promise1 = Promise[Unit]() - val promise2 = Promise[Unit]() - status.trackFuture("a", promise1.future, showTimer = true) - tickSecond() - status.trackFuture("b", promise2.future, showTimer = true) - 1.to(2).foreach { _ => tickSecond() } - promise1.success(()) - 1.to(2).foreach { _ => tickSecond() } - promise2.success(()) - status.tick() - assertNoDiff( - client.statusBarHistory, - """| - | - a - |a 1s - |a 2s - |b 2s - |b 3s - |b 4s - | - |""".stripMargin, - ) - } } From b825317a571cf9c1e759c60ce0347a416363c540 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 20 Feb 2024 10:14:40 +0100 Subject: [PATCH 02/12] delete metals slow task --- .../src/main/scala/bench/PcBenchmark.scala | 2 +- .../meta/internal/bsp/BspConnector.scala | 2 +- .../meta/internal/builds/ShellRunner.scala | 37 ++------- .../internal/metals/ClientConfiguration.scala | 13 +-- .../metals/ForwardingMetalsBuildClient.scala | 8 +- .../metals/InitializationOptions.scala | 4 - .../scala/meta/internal/metals/Messages.scala | 4 - .../scala/meta/internal/metals/SlowTask.scala | 81 ++++++++++--------- .../internal/metals/WorkspaceLspService.scala | 6 +- .../language/ConfiguredLanguageClient.scala | 9 --- .../language/DelegatingLanguageClient.scala | 8 +- .../clients/language/MetalsHttpClient.scala | 58 ------------- .../language/MetalsLanguageClient.scala | 18 ----- .../clients/language/NoopLanguageClient.scala | 4 - .../internal/metals/debug/DebugProvider.scala | 7 +- .../worksheets/WorksheetProvider.scala | 25 +++--- .../scala/tests/sbt/SbtBloopLspSuite.scala | 12 --- .../main/scala/tests/BaseImportSuite.scala | 2 +- .../scala/tests/BaseWorksheetLspSuite.scala | 7 -- .../main/scala/tests/TestScala3Compiler.scala | 1 + .../src/main/scala/tests/TestingClient.scala | 18 ----- .../src/main/scala/tests/TestingServer.scala | 3 +- .../debug/DebugProtocolCancelationSuite.scala | 4 - 23 files changed, 82 insertions(+), 251 deletions(-) diff --git a/metals-bench/src/main/scala/bench/PcBenchmark.scala b/metals-bench/src/main/scala/bench/PcBenchmark.scala index b8c76d7c86f..3992f916cfa 100644 --- a/metals-bench/src/main/scala/bench/PcBenchmark.scala +++ b/metals-bench/src/main/scala/bench/PcBenchmark.scala @@ -30,7 +30,7 @@ abstract class PcBenchmark { protected implicit val ec: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global protected val embedded = new Embedded( - new metals.SlowTask(NoopLanguageClient) + new metals.SlowTask(NoopLanguageClient, slowTaskIsOn = false) ) protected final val benchmarkedScalaVersions: Array[String] = Array( diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index cfd495b0f5b..0452731f93f 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -132,7 +132,7 @@ class BspConnector( else Future.successful(()) } yield connection slowTaskProvider - .trackFuture("Connecting to sbt", connectionF)//TODO: show time + .trackFuture("Connecting to sbt", connectionF) //TODO: show time .map(Some(_)) case ResolvedBspOne(details) => tables.buildServers.chooseServer(details.getName()) diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index 99294a957e5..273cfe37685 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -18,19 +18,13 @@ import scala.meta.internal.metals.MutableCancelable import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer -import scala.meta.internal.metals.clients.language.MetalsLanguageClient -import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams import scala.meta.internal.process.ExitCodes import scala.meta.internal.process.SystemProcess import scala.meta.io.AbsolutePath import coursierapi._ -class ShellRunner( - languageClient: MetalsLanguageClient, - time: Time, - slowTaskProvider: SlowTask, -)(implicit +class ShellRunner(time: Time, slowTaskProvider: SlowTask)(implicit executionContext: scala.concurrent.ExecutionContext ) extends Cancelable { @@ -110,40 +104,25 @@ class ShellRunner( Some(processErr), propagateError, ) - // NOTE(olafur): older versions of VS Code don't respect cancellation of - // window/showMessageRequest, meaning the "cancel build import" button - // stays forever in view even after successful build import. In newer - // VS Code versions the message is hidden after a delay. - val taskResponse = - languageClient.metalsSlowTask( - new MetalsSlowTaskParams(commandRun) - ) - val result = Promise[Int] - taskResponse.asScala.foreach { item => - if (item.cancel) { - if (logInfo) - scribe.info(s"user cancelled $commandRun") - result.trySuccess(ExitCodes.Cancel) - ps.cancel - } - } - val newCancelables: List[Cancelable] = - List(() => ps.cancel, () => taskResponse.cancel(false)) - newCancelables.foreach(cancelables.add) + val newCancelable: Cancelable = () => ps.cancel + cancelables.add(newCancelable) val processFuture = ps.complete slowTaskProvider.trackFuture( s"Running '$commandRun'", processFuture, + onCancel = Some(() => { + result.trySuccess(ExitCodes.Cancel) + ps.cancel + }), ) processFuture.map { code => - taskResponse.cancel(false) if (logInfo) scribe.info(s"time: ran '$commandRun' in $elapsed") result.trySuccess(code) } - result.future.onComplete(_ => newCancelables.foreach(cancelables.remove)) + result.future.onComplete(_ => cancelables.remove(newCancelable)) result.future } 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 b89a5c6e857..a29157b9cd3 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala @@ -75,12 +75,13 @@ final class ClientConfiguration( .map(Icons.fromString) .getOrElse(initialConfig.icons) - def slowTaskIsOn(): Boolean = - extract( - initializationOptions.slowTaskProvider, - experimentalCapabilities.slowTaskProvider, - initialConfig.slowTask.isOn, - ) + def slowTaskIsOn(): Boolean = + (for { + params <- initializeParams + capabilities <- Option(params.getCapabilities()) + window <- Option(capabilities.getWindow()) + slowTask <- Option(window.getWorkDoneProgress()) + } yield slowTask.booleanValue()).getOrElse(false) def isExecuteClientCommandProvider(): Boolean = extract( diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index 67816fd923f..41dda1a00b7 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -183,11 +183,9 @@ final class ForwardingMetalsBuildClient( val token = if (isNoOp) None else - Some( - slowTaskProvider.startSlowTask( - s"Compiling $name", - withProgress = true, - ) + slowTaskProvider.startSlowTask( + s"Compiling $name", + withProgress = true, ) val compilation = new Compilation(new Timer(time), isNoOp, token) compilations(task.getTarget) = compilation 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 e850acee4c3..3b6cea590d5 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala @@ -39,7 +39,6 @@ import org.eclipse.{lsp4j => l} * @param quickPickProvider if the client implements `metals/quickPick`. * @param renameFileThreshold amount of files that should be opened during rename if client * is a `openFilesOnRenameProvider`. - * @param slowTaskProvider if the client implements `metals/slowTask`. * @param statusBarProvider if the client implements `metals/status`. * @param treeViewProvider if the client implements the Metals Tree View Protocol. * @param testExplorerProvider if the client implements the Test Explorer UI. @@ -70,7 +69,6 @@ final case class InitializationOptions( openFilesOnRenameProvider: Option[Boolean], quickPickProvider: Option[Boolean], renameFileThreshold: Option[Int], - slowTaskProvider: Option[Boolean], statusBarProvider: Option[String], treeViewProvider: Option[Boolean], testExplorerProvider: Option[Boolean], @@ -120,7 +118,6 @@ object InitializationOptions { None, None, None, - None, ) def from( @@ -168,7 +165,6 @@ object InitializationOptions { jsonObj.getBooleanOption("openFilesOnRenameProvider"), quickPickProvider = jsonObj.getBooleanOption("quickPickProvider"), renameFileThreshold = jsonObj.getIntOption("renameFileThreshold"), - slowTaskProvider = jsonObj.getBooleanOption("slowTaskProvider"), statusBarProvider = jsonObj.getStringOption("statusBarProvider"), treeViewProvider = jsonObj.getBooleanOption("treeViewProvider"), testExplorerProvider = jsonObj.getBooleanOption("testExplorerProvider"), diff --git a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala index fc9813d1a1f..e4de17aea98 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -8,7 +8,6 @@ import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.VersionRecommendation import scala.meta.internal.jdk.CollectionConverters._ import scala.meta.internal.metals.clients.language.MetalsInputBoxParams -import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams import scala.meta.internal.metals.clients.language.MetalsStatusParams import scala.meta.internal.semver.SemVer import scala.meta.io.AbsolutePath @@ -136,9 +135,6 @@ object Messages { "Failed to reset the workspace. See the log for more details.", ) - def bloopInstallProgress(buildToolExecName: String) = - new MetalsSlowTaskParams(s"$buildToolExecName bloopInstall") - def dontShowAgain: MessageActionItem = new MessageActionItem("Don't show again") diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala index cad189df63f..272f7f5a51f 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala @@ -17,51 +17,56 @@ import org.eclipse.lsp4j.WorkDoneProgressReport import org.eclipse.lsp4j.jsonrpc.messages import org.eclipse.lsp4j.services.LanguageClient -class SlowTask(client: LanguageClient)(implicit ec: ExecutionContext) { +class SlowTask( + client: LanguageClient, + slowTaskIsOn: Boolean +)(implicit ec: ExecutionContext) { type Token = messages.Either[String, Integer] private def onCancelMap = mutable.Map[Token, () => Unit]() def startSlowTask( message: String, withProgress: Boolean = false, - onCancel: Option[() => Unit] = None - ): Future[Token] = { - val uuid = UUID.randomUUID().toString() - val token = messages.Either.forLeft[String, Integer](uuid) + onCancel: Option[() => Unit] = None, + ): Option[Future[Token]] = + Option.when(slowTaskIsOn) { + val uuid = UUID.randomUUID().toString() + val token = messages.Either.forLeft[String, Integer](uuid) - onCancel match { - case Some(onCancel) => - onCancelMap(token) = onCancel - case None => - } + onCancel match { + case Some(onCancel) => + onCancelMap(token) = onCancel + case None => + } + + client + .createProgress(new WorkDoneProgressCreateParams(token)) + .asScala + .map { _ => + val begin = new WorkDoneProgressBegin() + begin.setTitle(message) + if (withProgress) { + begin.setPercentage(0) + } + if (onCancel.isDefined) { + begin.setCancellable(true) + } + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + begin + ) + client.notifyProgress(new ProgressParams(token, notification)) + token - client - .createProgress(new WorkDoneProgressCreateParams(token)) - .asScala - .map { _ => - val begin = new WorkDoneProgressBegin() - begin.setTitle(message) - if (withProgress) { - begin.setPercentage(0) - } - if (onCancel.isDefined) { - begin.setCancellable(true) } - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - begin - ) - client.notifyProgress(new ProgressParams(token, notification)) - token - } - } + } def notifyProgress( token: Future[Token], percentage: Int, additionalMessage: Option[String] = None, ): Future[Unit] = { - token.map{ token => + token.map { token => val report = new WorkDoneProgressReport() report.setPercentage(percentage) additionalMessage.foreach(msg => report.setMessage(msg)) @@ -74,7 +79,7 @@ class SlowTask(client: LanguageClient)(implicit ec: ExecutionContext) { } def endSlowTask(token: Future[Token]): Future[Unit] = { - token.map{ token => + token.map { token => val end = messages.Either.forLeft[WorkDoneProgressNotification, Object]( new WorkDoneProgressEnd() ) @@ -82,24 +87,24 @@ class SlowTask(client: LanguageClient)(implicit ec: ExecutionContext) { } } - //TODO: add possibility to show time + // TODO: add possibility to show time def trackFuture[T]( message: String, value: Future[T], - onCancel: Option[() => Unit] = None + onCancel: Option[() => Unit] = None, )(implicit ec: ExecutionContext): Future[T] = { - val token = startSlowTask(message, onCancel = onCancel) + val optToken = startSlowTask(message, onCancel = onCancel) value.map { result => - //TODO:: probably shouldn't do it if it was already cancelled - endSlowTask(token) + // TODO:: probably shouldn't do it if it was already cancelled + optToken.foreach(endSlowTask) result } } def trackBlocking[T](message: String)(thunk: => T): T = { - val token = startSlowTask(message) + val optToken = startSlowTask(message) val result = thunk - endSlowTask(token) + optToken.foreach(endSlowTask) result } diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index 2c4fa005163..3e2e20e642a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -123,7 +123,7 @@ class WorkspaceLspService( languageClient } - private val slowTaskProvider = new SlowTask(languageClient) + private val slowTaskProvider = new SlowTask(languageClient, slowTaskIsOn = false) private val userConfigSync = new UserConfigurationSync(initializeParams, languageClient, clientConfig) @@ -134,7 +134,7 @@ class WorkspaceLspService( ) private val shellRunner = register { - new ShellRunner(languageClient, time, slowTaskProvider) + new ShellRunner(time, slowTaskProvider) } var focusedDocument: Option[AbsolutePath] = None @@ -1194,8 +1194,6 @@ class WorkspaceLspService( languageClient.underlying, () => server.reload(), clientConfig.icons, - time, - sh, clientConfig, ) render = () => newClient.renderHtml diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala index f0b717e3196..81f01e9d7e1 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala @@ -86,15 +86,6 @@ final class ConfiguredLanguageClient( case _ => } } - override def metalsSlowTask( - params: MetalsSlowTaskParams - ): CompletableFuture[MetalsSlowTaskResult] = { - if (clientConfig.slowTaskIsOn) { - underlying.metalsSlowTask(params) - } else { - new CompletableFuture[MetalsSlowTaskResult]() - } - } override def showMessage(params: MessageParams): Unit = { underlying.showMessage(params) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala index 499860f37ec..98ccc6b19bd 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/DelegatingLanguageClient.scala @@ -48,12 +48,6 @@ class DelegatingLanguageClient(var underlying: MetalsLanguageClient) underlying.metalsStatus(params) } - override def metalsSlowTask( - params: MetalsSlowTaskParams - ): CompletableFuture[MetalsSlowTaskResult] = { - underlying.metalsSlowTask(params) - } - override def telemetryEvent(value: Any): Unit = { underlying.telemetryEvent(value) } @@ -121,7 +115,7 @@ class DelegatingLanguageClient(var underlying: MetalsLanguageClient) underlying.configuration(configurationParams) override def createProgress( - params: WorkDoneProgressCreateParams //TODO:: check that has valid capabilities + params: WorkDoneProgressCreateParams ): CompletableFuture[Void] = underlying.createProgress(params) override def notifyProgress(params: ProgressParams): Unit = diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsHttpClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsHttpClient.scala index d4b88568dae..c950cefad99 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsHttpClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsHttpClient.scala @@ -2,12 +2,9 @@ package scala.meta.internal.metals import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentLinkedDeque -import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference -import scala.concurrent.CancellationException import scala.concurrent.ExecutionContext import scala.util.Try @@ -16,8 +13,6 @@ import scala.meta.internal.io.PathIO import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.clients.language.DelegatingLanguageClient import scala.meta.internal.metals.clients.language.MetalsLanguageClient -import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams -import scala.meta.internal.metals.clients.language.MetalsSlowTaskResult import scala.meta.internal.metals.clients.language.MetalsStatusParams import scala.meta.io.AbsolutePath @@ -53,8 +48,6 @@ final class MetalsHttpClient( initial: MetalsLanguageClient, triggerReload: () => Unit, icons: Icons, - time: Time, - sh: ScheduledExecutorService, clientConfig: ClientConfiguration, )(implicit ec: ExecutionContext) extends DelegatingLanguageClient(initial) { @@ -78,50 +71,6 @@ final class MetalsHttpClient( underlying.metalsStatus(params) } - // =============== - // metals/slowTask - // =============== - private case class SlowTask( - id: String, - value: MetalsSlowTaskParams, - promise: CompletableFuture[MetalsSlowTaskResult], - timer: Timer, - ) - private val slowTasks = new ConcurrentLinkedDeque[SlowTask]() - private def slowTasksFormatted(html: HtmlBuilder): HtmlBuilder = { - slowTasks.removeIf(_.promise.isDone) - html.unorderedList(slowTasks.asScala) { task => - html - .text(task.value.message) - .text(" ") - .text(task.timer.toString) - .submitButton(s"id=${task.id}", "Cancel") - } - } - override def metalsSlowTask( - params: MetalsSlowTaskParams - ): CompletableFuture[MetalsSlowTaskResult] = { - val fromEditorCompletable = underlying.metalsSlowTask(params) - slowTasks.add( - SlowTask(nextId(), params, fromEditorCompletable, new Timer(time)) - ) - sh.scheduleAtFixedRate( - new Runnable { - override def run(): Unit = { - triggerReload() - if (fromEditorCompletable.isDone) { - throw new CancellationException - } - } - }, - 0, - 1, - TimeUnit.SECONDS, - ) - fromEditorCompletable.asScala.onComplete { _ => triggerReload() } - fromEditorCompletable - } - // ================== // window/showMessage // ================== @@ -246,7 +195,6 @@ final class MetalsHttpClient( "metals/status", _.element("p")(_.text("Status: ").raw(statusFormatted)), ) - .section("metals/slowTask", slowTasksFormatted) .section("workspace/executeCommand", serverCommands) .section("window/showMessageRequests", showMessageRequestsFormatted) .section("window/showMessage", showMessagesFormatted) @@ -291,12 +239,6 @@ final class MetalsHttpClient( def completeCommand(exchange: HttpServerExchange): Unit = { val id = exchange.getQuery("id").getOrElse("") showMessages.removeIf(_.id == id) - slowTasks.forEach { task => - if (task.id == id) { - task.promise.complete(MetalsSlowTaskResult(cancel = true)) - triggerReload() - } - } for { messageRequest <- showMessageRequests.asScala if id == messageRequest.id diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsLanguageClient.scala index 4e44b203118..6d7483b9bd8 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/MetalsLanguageClient.scala @@ -33,17 +33,6 @@ trait MetalsLanguageClient @JsonNotification("metals/status") def metalsStatus(params: MetalsStatusParams): Unit - /** - * Starts a long running task with no estimate for how long it will take to complete. - * - * - request cancellation from the server indicates that the task has completed - * - response with cancel=true indicates the client wishes to cancel the slow task - */ - @JsonRequest("metals/slowTask") - def metalsSlowTask( - params: MetalsSlowTaskParams - ): CompletableFuture[MetalsSlowTaskResult] - @JsonNotification("metals/executeClientCommand") def metalsExecuteClientCommand(params: ExecuteCommandParams): Unit @@ -175,13 +164,6 @@ object StatusType extends Enumeration { val metals, bsp = Value } -case class MetalsSlowTaskParams( - message: String, - quietLogs: java.lang.Boolean = null, - secondsElapsed: java.lang.Integer = null, -) -case class MetalsSlowTaskResult(cancel: Boolean) - case class MetalsInputBoxParams( // The value to prefill in the input box @Nullable value: String = null, diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/NoopLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/NoopLanguageClient.scala index 562da7db814..f48a7bba409 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/NoopLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/NoopLanguageClient.scala @@ -21,10 +21,6 @@ import org.eclipse.lsp4j.WorkDoneProgressCreateParams */ abstract class NoopLanguageClient extends MetalsLanguageClient { override def metalsStatus(params: MetalsStatusParams): Unit = () - override def metalsSlowTask( - params: MetalsSlowTaskParams - ): CompletableFuture[MetalsSlowTaskResult] = - new CompletableFuture[MetalsSlowTaskResult]() override def telemetryEvent(`object`: Any): Unit = () override def publishDiagnostics(diagnostics: PublishDiagnosticsParams): Unit = () diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala index 0e4ab492b08..4356e1ca1e7 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala @@ -258,12 +258,7 @@ class DebugProvider( socket } - val connWithTimeout = - // if slow task is supported users can stop it themselves - if (clientConfig.slowTaskIsOn()) conn - else conn.withTimeout(60, TimeUnit.SECONDS) - - connWithTimeout + conn.withTimeout(60, TimeUnit.SECONDS) .recover { case exception => connectedToServer.tryFailure(exception) cancelPromise.trySuccess(()) diff --git a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala index 646298349f5..c458ee5cecb 100644 --- a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala @@ -35,7 +35,6 @@ import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.clients.language.MetalsLanguageClient -import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams import scala.meta.internal.mtags.MD5 import scala.meta.internal.pc.CompilerJobQueue import scala.meta.internal.pc.InterruptException @@ -253,8 +252,8 @@ class WorksheetProvider( cancelables.add(Cancelable(() => completeEmptyResult())) slowTaskProvider.trackFuture( s"Evaluating ${path.filename}", - result.asScala - // TODO:: showTimer = true, + result.asScala, + // TODO:: showTimer = true, ) token.checkCanceled() // NOTE(olafurpg) Run evaluation in a custom thread so that we can @@ -340,17 +339,17 @@ class WorksheetProvider( val interruptThread = new Runnable { def run(): Unit = { if (!result.isDone()) { - val cancel = languageClient.metalsSlowTask( - new MetalsSlowTaskParams( - s"Evaluating worksheet '${path.filename}'", - quietLogs = true, - secondsElapsed = userConfig().worksheetCancelTimeout, - ) + //TODO: what about worksheet timeout ??? + slowTaskProvider.startSlowTask( + s"Evaluating worksheet '${path.filename}'" ) - cancel.asScala.foreach { c => - if (c.cancel) cancellable.cancel() - } - result.asScala.onComplete(_ => cancel.cancel(true)) + + val optToken = slowTaskProvider.startSlowTask( + s"Evaluating worksheet '${path.filename}'", + onCancel = Some(cancellable.cancel), + ) + + result.asScala.onComplete(_ => optToken.foreach(slowTaskProvider.endSlowTask)) } } } diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index 29e10b457bf..a7e1a65d732 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -1,7 +1,5 @@ package tests.sbt -import java.util.concurrent.TimeUnit - import scala.concurrent.Future import scala.meta.internal.builds.SbtBuildTool @@ -13,7 +11,6 @@ import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ServerCommands import scala.meta.internal.metals.UserConfiguration -import scala.meta.internal.metals.clients.language.MetalsSlowTaskResult import scala.meta.internal.metals.{BuildInfo => V} import scala.meta.io.AbsolutePath @@ -279,14 +276,6 @@ class SbtBloopLspSuite } test("cancel") { - client.slowTaskHandler = params => { - if (params == bloopInstallProgress("sbt")) { - Thread.sleep(TimeUnit.SECONDS.toMillis(2)) - Some(MetalsSlowTaskResult(cancel = true)) - } else { - None - } - } cleanWorkspace() for { _ <- initialize( @@ -300,7 +289,6 @@ class SbtBloopLspSuite expectError = true, ) _ = assertStatus(!_.isInstalled) - _ = client.slowTaskHandler = _ => None _ <- server.didSave("build.sbt")(_ + "\n// comment") _ = assertNoDiff(client.workspaceShowMessages, "") _ = assertStatus(!_.isInstalled) diff --git a/tests/unit/src/main/scala/tests/BaseImportSuite.scala b/tests/unit/src/main/scala/tests/BaseImportSuite.scala index 2bfe3ff479c..e60cfb04afd 100644 --- a/tests/unit/src/main/scala/tests/BaseImportSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseImportSuite.scala @@ -21,7 +21,7 @@ abstract class BaseImportSuite( ImportBuildChanges.params(buildTool.toString()).getMessage def progressMessage: String = - bloopInstallProgress(buildTool.executableName).message + s"${buildTool.executableName} bloopInstall" def bazelNavigationMessage: String = CheckDoctor.bazelNavigation diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index 14ba943c520..51cc766697d 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -6,7 +6,6 @@ import scala.meta.internal.metals.InitializationOptions import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ScalaVersions import scala.meta.internal.metals.UserConfiguration -import scala.meta.internal.metals.clients.language.MetalsSlowTaskResult import scala.meta.internal.metals.codeactions.CreateNewSymbol import scala.meta.internal.metals.codeactions.ImportMissingSymbol import scala.meta.internal.metals.{BuildInfo => V} @@ -261,10 +260,6 @@ abstract class BaseWorksheetLspSuite( test("cancel") { cleanWorkspace() val cancelled = Promise[Unit]() - client.slowTaskHandler = { _ => - cancelled.trySuccess(()) - Some(MetalsSlowTaskResult(cancel = true)) - } for { _ <- initialize( s""" @@ -277,7 +272,6 @@ abstract class BaseWorksheetLspSuite( ) _ <- server.didOpen("a/src/main/scala/Main.worksheet.sc") _ <- cancelled.future - _ = client.slowTaskHandler = (_ => None) _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")( _.replace("Stream", "// Stream") ) @@ -400,7 +394,6 @@ abstract class BaseWorksheetLspSuite( test("update-classpath") { cleanWorkspace() - client.slowTaskHandler = _ => None for { _ <- initialize( s""" diff --git a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala index 370c7a9d3d2..c0dd8db6c32 100644 --- a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala +++ b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala @@ -25,6 +25,7 @@ object TestScala3Compiler { val client = new TestingClient(PathIO.workingDirectory, Buffers()) val status = new SlowTask( client, + slowTaskIsOn = false )(ec) val embedded = new Embedded(status) val pc = embedded diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 8d4f3c53ed1..a4d37f0f6d2 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -27,8 +27,6 @@ import scala.meta.internal.metals.TextEdits import scala.meta.internal.metals.WorkspaceChoicePopup import scala.meta.internal.metals.clients.language.MetalsInputBoxParams import scala.meta.internal.metals.clients.language.MetalsQuickPickParams -import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams -import scala.meta.internal.metals.clients.language.MetalsSlowTaskResult import scala.meta.internal.metals.clients.language.MetalsStatusParams import scala.meta.internal.metals.clients.language.NoopLanguageClient import scala.meta.internal.metals.clients.language.RawMetalsInputBoxResult @@ -113,9 +111,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) val clientCommands = new ConcurrentLinkedDeque[ExecuteCommandParams]() val decorations = new ConcurrentHashMap[AbsolutePath, Set[PublishDecorationsParams]]() - var slowTaskHandler: MetalsSlowTaskParams => Option[MetalsSlowTaskResult] = { - _: MetalsSlowTaskParams => None - } var showMessageHandler: MessageParams => Unit = { _: MessageParams => () } @@ -403,19 +398,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) override def logMessage(params: MessageParams): Unit = { logMessages.add(params) } - override def metalsSlowTask( - params: MetalsSlowTaskParams - ): CompletableFuture[MetalsSlowTaskResult] = { - CompletableFuture.completedFuture { - messageRequests.addLast(params.message) - slowTaskHandler(params) match { - case Some(result) => - result - case None => - MetalsSlowTaskResult(cancel = false) - } - } - } override def metalsStatus(params: MetalsStatusParams): Unit = { statusParams.add(params) diff --git a/tests/unit/src/main/scala/tests/TestingServer.scala b/tests/unit/src/main/scala/tests/TestingServer.scala index 29fadfd13f3..8baf840d82e 100644 --- a/tests/unit/src/main/scala/tests/TestingServer.scala +++ b/tests/unit/src/main/scala/tests/TestingServer.scala @@ -2088,8 +2088,7 @@ object TestingServer { InitializationOptions.Default.copy( debuggingProvider = Some(true), runProvider = Some(true), - treeViewProvider = Some(true), - slowTaskProvider = Some(true), + treeViewProvider = Some(true) ) // Caching is done using a key: dependency jars + excludedPackages setting + bucket size diff --git a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala index 4bc58f2a72d..54485ba8459 100644 --- a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala +++ b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala @@ -1,7 +1,6 @@ package tests.debug import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.clients.language.MetalsSlowTaskResult import ch.epfl.scala.bsp4j.DebugSessionParamsDataKind import ch.epfl.scala.bsp4j.ScalaMainClass @@ -20,9 +19,6 @@ class DebugProtocolCancelationSuite ) { test("start") { - client.slowTaskHandler = { _ => - Some(MetalsSlowTaskResult(cancel = true)) - } val mainClass = new ScalaMainClass( "a.Main", List("Bar").asJava, From 0050e3916aad72067498f2e0dbf153da166de62a Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 20 Feb 2024 14:42:16 +0100 Subject: [PATCH 03/12] fix cancel tests --- .../src/main/scala/bench/PcBenchmark.scala | 4 +- .../meta/internal/bsp/BspConnector.scala | 2 +- .../meta/internal/builds/ShellRunner.scala | 2 +- .../metals/ForwardingMetalsBuildClient.scala | 4 +- .../scala/meta/internal/metals/SlowTask.scala | 122 +++++++++--------- .../internal/metals/WorkspaceLspService.scala | 2 +- .../language/ConfiguredLanguageClient.scala | 14 ++ .../worksheets/WorksheetProvider.scala | 14 +- .../scala/tests/sbt/SbtBloopLspSuite.scala | 8 ++ .../scala/tests/BaseWorksheetLspSuite.scala | 7 + .../main/scala/tests/TestScala3Compiler.scala | 5 +- .../src/main/scala/tests/TestingClient.scala | 13 +- .../src/main/scala/tests/TestingServer.scala | 3 + .../debug/DebugProtocolCancelationSuite.scala | 6 + 14 files changed, 124 insertions(+), 82 deletions(-) diff --git a/metals-bench/src/main/scala/bench/PcBenchmark.scala b/metals-bench/src/main/scala/bench/PcBenchmark.scala index 3992f916cfa..40b5823a1b7 100644 --- a/metals-bench/src/main/scala/bench/PcBenchmark.scala +++ b/metals-bench/src/main/scala/bench/PcBenchmark.scala @@ -29,9 +29,7 @@ abstract class PcBenchmark { TrieMap.empty[String, PresentationCompiler] protected implicit val ec: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global - protected val embedded = new Embedded( - new metals.SlowTask(NoopLanguageClient, slowTaskIsOn = false) - ) + protected val embedded = new Embedded(new metals.SlowTask(NoopLanguageClient)) protected final val benchmarkedScalaVersions: Array[String] = Array( metals.BuildInfo.scala3, diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index 0452731f93f..b39604881c9 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -132,7 +132,7 @@ class BspConnector( else Future.successful(()) } yield connection slowTaskProvider - .trackFuture("Connecting to sbt", connectionF) //TODO: show time + .trackFuture("Connecting to sbt", connectionF) .map(Some(_)) case ResolvedBspOne(details) => tables.buildServers.chooseServer(details.getName()) diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index 273cfe37685..9d3540a885a 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -110,7 +110,7 @@ class ShellRunner(time: Time, slowTaskProvider: SlowTask)(implicit val processFuture = ps.complete slowTaskProvider.trackFuture( - s"Running '$commandRun'", + commandRun, processFuture, onCancel = Some(() => { result.trySuccess(ExitCodes.Cancel) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index 41dda1a00b7..fb65ebbbefa 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -181,12 +181,12 @@ final class ForwardingMetalsBuildClient( "Start no-op compilation" ) val token = - if (isNoOp) None - else + Option.when(isNoOp) { slowTaskProvider.startSlowTask( s"Compiling $name", withProgress = true, ) + } val compilation = new Compilation(new Timer(time), isNoOp, token) compilations(task.getTarget) = compilation } diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala index 272f7f5a51f..cd2d6fd50e9 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala @@ -1,8 +1,8 @@ package scala.meta.internal.metals import java.util.UUID +import java.util.concurrent.ConcurrentHashMap -import scala.collection.mutable import scala.concurrent.ExecutionContext import scala.concurrent.Future @@ -18,97 +18,103 @@ import org.eclipse.lsp4j.jsonrpc.messages import org.eclipse.lsp4j.services.LanguageClient class SlowTask( - client: LanguageClient, - slowTaskIsOn: Boolean + client: LanguageClient )(implicit ec: ExecutionContext) { type Token = messages.Either[String, Integer] - private def onCancelMap = mutable.Map[Token, () => Unit]() + case class Task(onCancel: Option[() => Unit]) + private val taskMap = new ConcurrentHashMap[Token, Task]() def startSlowTask( message: String, withProgress: Boolean = false, onCancel: Option[() => Unit] = None, - ): Option[Future[Token]] = - Option.when(slowTaskIsOn) { - val uuid = UUID.randomUUID().toString() - val token = messages.Either.forLeft[String, Integer](uuid) + ): Future[Token] = { + val uuid = UUID.randomUUID().toString() + val token = messages.Either.forLeft[String, Integer](uuid) - onCancel match { - case Some(onCancel) => - onCancelMap(token) = onCancel - case None => - } - - client - .createProgress(new WorkDoneProgressCreateParams(token)) - .asScala - .map { _ => - val begin = new WorkDoneProgressBegin() - begin.setTitle(message) - if (withProgress) { - begin.setPercentage(0) - } - if (onCancel.isDefined) { - begin.setCancellable(true) - } - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - begin - ) - client.notifyProgress(new ProgressParams(token, notification)) - token + taskMap.put(token, Task(onCancel)) + client + .createProgress(new WorkDoneProgressCreateParams(token)) + .asScala + .map { _ => + val begin = new WorkDoneProgressBegin() + begin.setTitle(message) + if (withProgress) { + begin.setPercentage(0) } - } + if (onCancel.isDefined) { + begin.setCancellable(true) + } + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + begin + ) + client.notifyProgress(new ProgressParams(token, notification)) + token + } + } def notifyProgress( token: Future[Token], percentage: Int, - additionalMessage: Option[String] = None, ): Future[Unit] = { - token.map { token => - val report = new WorkDoneProgressReport() - report.setPercentage(percentage) - additionalMessage.foreach(msg => report.setMessage(msg)) - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - report - ) - client.notifyProgress(new ProgressParams(token, notification)) - } + if (taskMap.contains(token)) { + token.map { token => + val report = new WorkDoneProgressReport() + report.setPercentage(percentage) + report.setMessage(s"$percentage%") + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + report + ) + client.notifyProgress(new ProgressParams(token, notification)) + } + } else Future.successful(()) } - def endSlowTask(token: Future[Token]): Future[Unit] = { - token.map { token => - val end = messages.Either.forLeft[WorkDoneProgressNotification, Object]( - new WorkDoneProgressEnd() - ) - client.notifyProgress(new ProgressParams(token, end)) + def endSlowTask(token: Future[Token]): Future[Unit] = + try { + taskMap.remove(token) + token.map { token => + val end = messages.Either.forLeft[WorkDoneProgressNotification, Object]( + new WorkDoneProgressEnd() + ) + client.notifyProgress(new ProgressParams(token, end)) + } + } catch { + case _: NullPointerException => + // no such value in map + Future.unit } - } - // TODO: add possibility to show time def trackFuture[T]( message: String, value: Future[T], onCancel: Option[() => Unit] = None, )(implicit ec: ExecutionContext): Future[T] = { - val optToken = startSlowTask(message, onCancel = onCancel) + val token = startSlowTask(message, onCancel = onCancel) value.map { result => - // TODO:: probably shouldn't do it if it was already cancelled - optToken.foreach(endSlowTask) + endSlowTask(token) result } } def trackBlocking[T](message: String)(thunk: => T): T = { - val optToken = startSlowTask(message) + val token = startSlowTask(message) val result = thunk - optToken.foreach(endSlowTask) + endSlowTask(token) result } - def canceled(token: Token): Unit = onCancelMap.get(token).foreach(_()) + def canceled(token: Token): Unit = + try { + val task = taskMap.remove(token) + task.onCancel.foreach(_()) + } catch { + case _: NullPointerException => + // no such value in map + } } object SlowTask { diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index 3e2e20e642a..fefcbafae41 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -123,7 +123,7 @@ class WorkspaceLspService( languageClient } - private val slowTaskProvider = new SlowTask(languageClient, slowTaskIsOn = false) + private val slowTaskProvider = new SlowTask(languageClient) private val userConfigSync = new UserConfigurationSync(initializeParams, languageClient, clientConfig) diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala index 81f01e9d7e1..369635285e0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala @@ -20,7 +20,9 @@ import org.eclipse.lsp4j.ExecuteCommandParams import org.eclipse.lsp4j.MessageActionItem import org.eclipse.lsp4j.MessageParams import org.eclipse.lsp4j.MessageType +import org.eclipse.lsp4j.ProgressParams import org.eclipse.lsp4j.ShowMessageRequestParams +import org.eclipse.lsp4j.WorkDoneProgressCreateParams /** * Delegates requests/notifications to the underlying language client according to the user configuration. @@ -200,4 +202,16 @@ final class ConfiguredLanguageClient( result } + override def createProgress( + params: WorkDoneProgressCreateParams + ): CompletableFuture[Void] = + if(clientConfig.slowTaskIsOn()) { + underlying.createProgress(params) + } else CompletableFuture.completedFuture(null) + + override def notifyProgress(params: ProgressParams): Unit = + if(clientConfig.slowTaskIsOn()) { + underlying.notifyProgress(params) + } + } diff --git a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala index c458ee5cecb..ff596d0e1ca 100644 --- a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala @@ -250,11 +250,7 @@ class WorksheetProvider( ) } cancelables.add(Cancelable(() => completeEmptyResult())) - slowTaskProvider.trackFuture( - s"Evaluating ${path.filename}", - result.asScala, - // TODO:: showTimer = true, - ) + slowTaskProvider.trackFuture(s"Evaluating ${path.filename}", result.asScala) token.checkCanceled() // NOTE(olafurpg) Run evaluation in a custom thread so that we can // `Thread.stop()` it in case of infinite loop. I'm not aware of any @@ -340,16 +336,12 @@ class WorksheetProvider( def run(): Unit = { if (!result.isDone()) { //TODO: what about worksheet timeout ??? - slowTaskProvider.startSlowTask( - s"Evaluating worksheet '${path.filename}'" - ) - - val optToken = slowTaskProvider.startSlowTask( + val token = slowTaskProvider.startSlowTask( s"Evaluating worksheet '${path.filename}'", onCancel = Some(cancellable.cancel), ) - result.asScala.onComplete(_ => optToken.foreach(slowTaskProvider.endSlowTask)) + result.asScala.onComplete(_ => slowTaskProvider.endSlowTask(token)) } } } diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index a7e1a65d732..c02d2470c1c 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -20,6 +20,7 @@ import tests.BaseImportSuite import tests.JavaHomeChangeTest import tests.ScriptsAssertions import tests.TestSemanticTokens +import java.util.concurrent.TimeUnit class SbtBloopLspSuite extends BaseImportSuite("sbt-bloop-import") @@ -277,6 +278,12 @@ class SbtBloopLspSuite test("cancel") { cleanWorkspace() + client.onBeginSlowTask = (name, cancelParams) => { + if (name == progressMessage) { + Thread.sleep(TimeUnit.SECONDS.toMillis(2)) + server.fullServer.didCancelWorkDoneProgress(cancelParams) + } + } for { _ <- initialize( s""" @@ -289,6 +296,7 @@ class SbtBloopLspSuite expectError = true, ) _ = assertStatus(!_.isInstalled) + _ = client.onBeginSlowTask = (_, _) => { } _ <- server.didSave("build.sbt")(_ + "\n// comment") _ = assertNoDiff(client.workspaceShowMessages, "") _ = assertStatus(!_.isInstalled) diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index 51cc766697d..12a322e0d2d 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -260,6 +260,13 @@ abstract class BaseWorksheetLspSuite( test("cancel") { cleanWorkspace() val cancelled = Promise[Unit]() + client.onBeginSlowTask = { (message, cancelParams) => + if(message.startsWith("Evaluating worksheet")) { + cancelled.trySuccess(()) + server.fullServer.didCancelWorkDoneProgress(cancelParams) + } + } + for { _ <- initialize( s""" diff --git a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala index c0dd8db6c32..53a57c03576 100644 --- a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala +++ b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala @@ -23,10 +23,7 @@ object TestScala3Compiler { resolver.resolve(V.scala3) match { case Some(mtags: MtagsBinaries.Artifacts) => val client = new TestingClient(PathIO.workingDirectory, Buffers()) - val status = new SlowTask( - client, - slowTaskIsOn = false - )(ec) + val status = new SlowTask(client)(ec) val embedded = new Embedded(status) val pc = embedded .presentationCompiler(mtags, mtags.jars) diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index a4d37f0f6d2..8f20a545bdc 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -56,6 +56,8 @@ import org.eclipse.lsp4j.WorkspaceEdit import org.eclipse.lsp4j.jsonrpc.CompletableFutures import tests.MetalsTestEnrichments._ import tests.TestOrderings._ +import org.eclipse.lsp4j.WorkDoneProgressBegin +import org.eclipse.lsp4j.WorkDoneProgressCancelParams /** * Fake LSP client that responds to notifications/requests initiated by the server. @@ -92,7 +94,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) var resetWorkspace = new MessageActionItem(ResetWorkspace.cancel) var regenerateAndRestartScalaCliBuildSever = FileOutOfScalaCliBspScope.ignore var shouldReloadAfterJavaHomeUpdate = ProjectJavaHomeUpdate.notNow - + var onBeginSlowTask: (String, WorkDoneProgressCancelParams) => Unit = (_, _) => { } val resources = new ResourceOperations(buffers) val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] = TrieMap.empty[AbsolutePath, Seq[Diagnostic]] @@ -414,6 +416,15 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) } override def notifyProgress(params: ProgressParams): Unit = { + if(params.getValue().isLeft()) { + params.getValue().getLeft() match { + case begin: WorkDoneProgressBegin => + val cancelParams = new WorkDoneProgressCancelParams(params.getToken()) + onBeginSlowTask(begin.getTitle(), cancelParams) + messageRequests.addLast(begin.getTitle()) + case _ => + } + } progressParams.add(params) } diff --git a/tests/unit/src/main/scala/tests/TestingServer.scala b/tests/unit/src/main/scala/tests/TestingServer.scala index 8baf840d82e..b5d0e7f8496 100644 --- a/tests/unit/src/main/scala/tests/TestingServer.scala +++ b/tests/unit/src/main/scala/tests/TestingServer.scala @@ -547,6 +547,8 @@ final case class TestingServer( val params = new InitializeParams val workspaceCapabilities = new WorkspaceClientCapabilities() val textDocumentCapabilities = new TextDocumentClientCapabilities + val windowCapabilities = new l.WindowClientCapabilities() + windowCapabilities.setWorkDoneProgress(true) textDocumentCapabilities.setFoldingRange(new FoldingRangeCapabilities) val completionItemCapabilities = new l.CompletionItemCapabilities(true) textDocumentCapabilities.setCompletion( @@ -579,6 +581,7 @@ final case class TestingServer( new ClientCapabilities( workspaceCapabilities, textDocumentCapabilities, + windowCapabilities, Map.empty.asJava.toJson, ) ) diff --git a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala index 54485ba8459..13123c0cf5a 100644 --- a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala +++ b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala @@ -19,6 +19,12 @@ class DebugProtocolCancelationSuite ) { test("start") { + cleanWorkspace() + client.onBeginSlowTask = (message, cancelParams) => { + if(message == "Starting debug server") { + server.fullServer.didCancelWorkDoneProgress(cancelParams) + } + } val mainClass = new ScalaMainClass( "a.Main", List("Bar").asJava, From d80bb07bf4e35ddbbdf108f4702da775c889bff0 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 20 Feb 2024 16:59:13 +0100 Subject: [PATCH 04/12] add timer --- .../src/main/scala/bench/PcBenchmark.scala | 2 +- .../metals/ForwardingMetalsBuildClient.scala | 9 +- .../scala/meta/internal/metals/SlowTask.scala | 125 ++++++++++++++---- .../meta/internal/metals/TaskProgress.scala | 2 +- .../internal/metals/WorkspaceLspService.scala | 17 ++- .../scala/tests/sbt/SbtBloopLspSuite.scala | 3 +- .../main/scala/tests/TestScala3Compiler.scala | 3 +- .../src/main/scala/tests/TestingClient.scala | 4 +- 8 files changed, 125 insertions(+), 40 deletions(-) diff --git a/metals-bench/src/main/scala/bench/PcBenchmark.scala b/metals-bench/src/main/scala/bench/PcBenchmark.scala index 40b5823a1b7..631b6e7982e 100644 --- a/metals-bench/src/main/scala/bench/PcBenchmark.scala +++ b/metals-bench/src/main/scala/bench/PcBenchmark.scala @@ -29,7 +29,7 @@ abstract class PcBenchmark { TrieMap.empty[String, PresentationCompiler] protected implicit val ec: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global - protected val embedded = new Embedded(new metals.SlowTask(NoopLanguageClient)) + protected val embedded = new Embedded(new metals.SlowTask(NoopLanguageClient, metals.Time.system)) protected final val benchmarkedScalaVersions: Array[String] = Array( metals.BuildInfo.scala3, diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index fb65ebbbefa..99138f5f67c 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -77,11 +77,14 @@ final class ForwardingMetalsBuildClient( def progressPercentage = taskProgress.percentage - def end(): Unit = token.foreach(slowTaskProvider.endSlowTask) + def end(): Unit = token.foreach(slowTaskProvider.endSlowTask(_)) def updateProgress(progress: Long, total: Long = 100): Unit = { + val prev = taskProgress.percentage taskProgress.update(progress, total) - token.foreach(slowTaskProvider.notifyProgress(_, progressPercentage)) + if(prev != taskProgress.percentage) { + token.foreach(slowTaskProvider.notifyProgress(_, progressPercentage)) + } } } @@ -181,7 +184,7 @@ final class ForwardingMetalsBuildClient( "Start no-op compilation" ) val token = - Option.when(isNoOp) { + Option.when(!isNoOp) { slowTaskProvider.startSlowTask( s"Compiling $name", withProgress = true, diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala index cd2d6fd50e9..efe0b0d585b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala @@ -2,6 +2,9 @@ package scala.meta.internal.metals import java.util.UUID import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.ScheduledFuture +import java.util.concurrent.TimeUnit import scala.concurrent.ExecutionContext import scala.concurrent.Future @@ -18,21 +21,69 @@ import org.eclipse.lsp4j.jsonrpc.messages import org.eclipse.lsp4j.services.LanguageClient class SlowTask( - client: LanguageClient -)(implicit ec: ExecutionContext) { + client: LanguageClient, + time: Time, +)(implicit ec: ExecutionContext) extends Cancelable { type Token = messages.Either[String, Integer] - case class Task(onCancel: Option[() => Unit]) + case class Task( + onCancel: Option[() => Unit], + showTimer: Boolean, + maybeProgress: Option[TaskProgress], + ) { + val timer = new Timer(time) + def additionalMessage: Option[String] = + if (showTimer) { + val seconds = timer.elapsedSeconds + if (seconds == 0) None + else { + maybeProgress match { + case Some(TaskProgress(percentage)) if seconds > 3 => + Some(s"${Timer.readableSeconds(seconds)} ($percentage%)") + case _ => + Some(s"${Timer.readableSeconds(seconds)}") + } + } + } else + maybeProgress match { + case Some(TaskProgress(0)) => None + case Some(TaskProgress(percentage)) => Some(s"($percentage%)") + case _ => None + } + } + + object Task { + def empty: Task = Task(onCancel = None, showTimer = false, maybeProgress = None) + } + private val taskMap = new ConcurrentHashMap[Token, Task]() + private var scheduledFuture: ScheduledFuture[_] = _ + + def start( + sh: ScheduledExecutorService, + initialDelay: Long, + period: Long, + unit: TimeUnit, + ): Unit = { + cancel() + scheduledFuture = + sh.scheduleAtFixedRate(() => updateTimers(), initialDelay, period, unit) + } + + private def updateTimers() = taskMap.keys.asScala.foreach(notifyProgress(_)) + def startSlowTask( message: String, withProgress: Boolean = false, + showTimer: Boolean = true, onCancel: Option[() => Unit] = None, ): Future[Token] = { val uuid = UUID.randomUUID().toString() val token = messages.Either.forLeft[String, Integer](uuid) - taskMap.put(token, Task(onCancel)) + val optProgress = Option.when(withProgress)(TaskProgress.empty) + val task = Task(onCancel, showTimer, optProgress) + taskMap.put(token, task) client .createProgress(new WorkDoneProgressCreateParams(token)) @@ -40,6 +91,7 @@ class SlowTask( .map { _ => val begin = new WorkDoneProgressBegin() begin.setTitle(message) + task.additionalMessage.foreach(begin.setMessage) if (withProgress) { begin.setPercentage(0) } @@ -58,42 +110,59 @@ class SlowTask( def notifyProgress( token: Future[Token], percentage: Int, - ): Future[Unit] = { - if (taskMap.contains(token)) { - token.map { token => - val report = new WorkDoneProgressReport() - report.setPercentage(percentage) - report.setMessage(s"$percentage%") - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - report - ) - client.notifyProgress(new ProgressParams(token, notification)) + ): Future[Unit] = + token.map { token => + val task = taskMap.getOrDefault(token, Task.empty) + task.maybeProgress match { + case Some(progress) => + progress.update(percentage) + val report = new WorkDoneProgressReport() + report.setPercentage(progress.percentage) + task.additionalMessage.foreach(report.setMessage) + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + report + ) + client.notifyProgress(new ProgressParams(token, notification)) + case None => + } + } + + def notifyProgress(token: Token): Unit = { + val task = taskMap.getOrDefault(token, Task.empty) + if (task.showTimer) { + val report = new WorkDoneProgressReport() + task.maybeProgress.foreach { progress => + report.setPercentage(progress.percentage) } + task.additionalMessage.foreach(report.setMessage) + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + report + ) + client.notifyProgress(new ProgressParams(token, notification)) } else Future.successful(()) } def endSlowTask(token: Future[Token]): Future[Unit] = - try { + token.map { token => taskMap.remove(token) - token.map { token => - val end = messages.Either.forLeft[WorkDoneProgressNotification, Object]( - new WorkDoneProgressEnd() - ) - client.notifyProgress(new ProgressParams(token, end)) - } - } catch { + val end = new WorkDoneProgressEnd() + val params = messages.Either.forLeft[WorkDoneProgressNotification, Object](end) + client.notifyProgress(new ProgressParams(token, params)) + }.recover{ case _: NullPointerException => // no such value in map - Future.unit } def trackFuture[T]( message: String, value: Future[T], onCancel: Option[() => Unit] = None, + showTimer: Boolean = true, )(implicit ec: ExecutionContext): Future[T] = { - val token = startSlowTask(message, onCancel = onCancel) + val token = + startSlowTask(message, onCancel = onCancel, showTimer = showTimer) value.map { result => endSlowTask(token) result @@ -115,6 +184,12 @@ class SlowTask( case _: NullPointerException => // no such value in map } + + override def cancel(): Unit = { + if (scheduledFuture != null) { + scheduledFuture.cancel(false) + } + } } object SlowTask { diff --git a/metals/src/main/scala/scala/meta/internal/metals/TaskProgress.scala b/metals/src/main/scala/scala/meta/internal/metals/TaskProgress.scala index d7266cd87b6..71324aada33 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/TaskProgress.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/TaskProgress.scala @@ -10,7 +10,7 @@ class TaskProgress( var progress: Long, var total: Long, ) { - def update(newProgress: Long, newTotal: Long): Unit = { + def update(newProgress: Long, newTotal: Long = 100): Unit = { progress = newProgress total = newTotal } diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index fefcbafae41..40f57a352ff 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -123,15 +123,19 @@ class WorkspaceLspService( languageClient } - private val slowTaskProvider = new SlowTask(languageClient) + private val slowTaskProvider = register { + new SlowTask(languageClient, time) + } private val userConfigSync = new UserConfigurationSync(initializeParams, languageClient, clientConfig) - val statusBar: StatusBar = new StatusBar( - languageClient, - time, - ) + val statusBar: StatusBar = register { + new StatusBar( + languageClient, + time, + ) + } private val shellRunner = register { new ShellRunner(time, slowTaskProvider) @@ -181,7 +185,7 @@ class WorkspaceLspService( name, doctor, bspStatus, - slowTaskProvider + slowTaskProvider, ) } @@ -1160,6 +1164,7 @@ class WorkspaceLspService( def initialized(): Future[Unit] = { statusBar.start(sh, 0, 1, ju.concurrent.TimeUnit.SECONDS) + slowTaskProvider.start(sh, 0, 1, ju.concurrent.TimeUnit.SECONDS) for { _ <- userConfigSync.initSyncUserConfiguration(folderServices) _ <- Future.sequence(folderServices.map(_.initialized())) diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index c02d2470c1c..d3cdebcb5f6 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -1,5 +1,7 @@ package tests.sbt +import java.util.concurrent.TimeUnit + import scala.concurrent.Future import scala.meta.internal.builds.SbtBuildTool @@ -20,7 +22,6 @@ import tests.BaseImportSuite import tests.JavaHomeChangeTest import tests.ScriptsAssertions import tests.TestSemanticTokens -import java.util.concurrent.TimeUnit class SbtBloopLspSuite extends BaseImportSuite("sbt-bloop-import") diff --git a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala index 53a57c03576..c9f3d5fe5ba 100644 --- a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala +++ b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala @@ -22,8 +22,9 @@ object TestScala3Compiler { val resolver = new TestMtagsResolver() resolver.resolve(V.scala3) match { case Some(mtags: MtagsBinaries.Artifacts) => + val time = new FakeTime val client = new TestingClient(PathIO.workingDirectory, Buffers()) - val status = new SlowTask(client)(ec) + val status = new SlowTask(client, time)(ec) val embedded = new Embedded(status) val pc = embedded .presentationCompiler(mtags, mtags.jars) diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 8f20a545bdc..033fd8bb0a9 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -51,13 +51,13 @@ import org.eclipse.lsp4j.ResourceOperation import org.eclipse.lsp4j.ShowMessageRequestParams import org.eclipse.lsp4j.TextDocumentEdit import org.eclipse.lsp4j.TextEdit +import org.eclipse.lsp4j.WorkDoneProgressBegin +import org.eclipse.lsp4j.WorkDoneProgressCancelParams import org.eclipse.lsp4j.WorkDoneProgressCreateParams import org.eclipse.lsp4j.WorkspaceEdit import org.eclipse.lsp4j.jsonrpc.CompletableFutures import tests.MetalsTestEnrichments._ import tests.TestOrderings._ -import org.eclipse.lsp4j.WorkDoneProgressBegin -import org.eclipse.lsp4j.WorkDoneProgressCancelParams /** * Fake LSP client that responds to notifications/requests initiated by the server. From c7907b70153039a34a65a75bcd884d8d4998c15a Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 20 Feb 2024 16:59:34 +0100 Subject: [PATCH 05/12] format --- .../src/main/scala/bench/PcBenchmark.scala | 4 ++- .../internal/builds/NewProjectProvider.scala | 2 +- .../internal/metals/ClientConfiguration.scala | 2 +- .../metals/ForwardingMetalsBuildClient.scala | 2 +- .../internal/metals/MetalsLspService.scala | 9 ++++--- .../scala/meta/internal/metals/SlowTask.scala | 26 +++++++++++-------- .../language/ConfiguredLanguageClient.scala | 6 ++--- .../internal/metals/debug/DebugProvider.scala | 3 ++- .../worksheets/WorksheetProvider.scala | 5 +++- .../metals/lsp/DelegatingScalaService.scala | 2 +- .../scala/tests/sbt/SbtBloopLspSuite.scala | 2 +- .../scala/tests/BaseWorksheetLspSuite.scala | 2 +- .../src/main/scala/tests/TestingClient.scala | 5 ++-- .../src/main/scala/tests/TestingServer.scala | 2 +- .../debug/DebugProtocolCancelationSuite.scala | 2 +- 15 files changed, 44 insertions(+), 30 deletions(-) diff --git a/metals-bench/src/main/scala/bench/PcBenchmark.scala b/metals-bench/src/main/scala/bench/PcBenchmark.scala index 631b6e7982e..d6df461501f 100644 --- a/metals-bench/src/main/scala/bench/PcBenchmark.scala +++ b/metals-bench/src/main/scala/bench/PcBenchmark.scala @@ -29,7 +29,9 @@ abstract class PcBenchmark { TrieMap.empty[String, PresentationCompiler] protected implicit val ec: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global - protected val embedded = new Embedded(new metals.SlowTask(NoopLanguageClient, metals.Time.system)) + protected val embedded = new Embedded( + new metals.SlowTask(NoopLanguageClient, metals.Time.system) + ) protected final val benchmarkedScalaVersions: Array[String] = Array( metals.BuildInfo.scala3, diff --git a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala index a60c17811f5..9a7f2511c91 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala @@ -30,7 +30,7 @@ class NewProjectProvider( config: ClientConfiguration, shell: ShellRunner, icons: Icons, - workspace: AbsolutePath + workspace: AbsolutePath, )(implicit context: ExecutionContext) { private val templatesUrl = 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 a29157b9cd3..17a0aac9792 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala @@ -75,7 +75,7 @@ final class ClientConfiguration( .map(Icons.fromString) .getOrElse(initialConfig.icons) - def slowTaskIsOn(): Boolean = + def slowTaskIsOn(): Boolean = (for { params <- initializeParams capabilities <- Option(params.getCapabilities()) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index 99138f5f67c..99f35b37de3 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -82,7 +82,7 @@ final class ForwardingMetalsBuildClient( def updateProgress(progress: Long, total: Long = 100): Unit = { val prev = taskProgress.percentage taskProgress.update(progress, total) - if(prev != taskProgress.percentage) { + if (prev != taskProgress.percentage) { token.foreach(slowTaskProvider.notifyProgress(_, progressPercentage)) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index d30a59053bc..8b3df0f2574 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -134,7 +134,7 @@ class MetalsLspService( folderVisibleName: Option[String], headDoctor: HeadDoctor, bspStatus: BspStatus, - slowTaskProvider: SlowTask + slowTaskProvider: SlowTask, ) extends Folder(folder, folderVisibleName, isKnownMetalsProject = true) with Cancelable with TextDocumentService { @@ -417,7 +417,7 @@ class MetalsLspService( onBuildTargetChanges(params) }, bspErrorHandler, - slowTaskProvider + slowTaskProvider, ) private val bloopServers: BloopServers = new BloopServers( @@ -2437,7 +2437,10 @@ class MetalsLspService( session.importBuilds() } for { - bspBuilds <- slowTaskProvider.trackFuture("Importing build", importedBuilds0) + bspBuilds <- slowTaskProvider.trackFuture( + "Importing build", + importedBuilds0, + ) _ = { val idToConnection = bspBuilds.flatMap { bspBuild => val targets = diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala index efe0b0d585b..a5510740a40 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala @@ -23,7 +23,8 @@ import org.eclipse.lsp4j.services.LanguageClient class SlowTask( client: LanguageClient, time: Time, -)(implicit ec: ExecutionContext) extends Cancelable { +)(implicit ec: ExecutionContext) + extends Cancelable { type Token = messages.Either[String, Integer] case class Task( onCancel: Option[() => Unit], @@ -52,7 +53,8 @@ class SlowTask( } object Task { - def empty: Task = Task(onCancel = None, showTimer = false, maybeProgress = None) + def empty: Task = + Task(onCancel = None, showTimer = false, maybeProgress = None) } private val taskMap = new ConcurrentHashMap[Token, Task]() @@ -145,15 +147,17 @@ class SlowTask( } def endSlowTask(token: Future[Token]): Future[Unit] = - token.map { token => - taskMap.remove(token) - val end = new WorkDoneProgressEnd() - val params = messages.Either.forLeft[WorkDoneProgressNotification, Object](end) - client.notifyProgress(new ProgressParams(token, params)) - }.recover{ - case _: NullPointerException => - // no such value in map - } + token + .map { token => + taskMap.remove(token) + val end = new WorkDoneProgressEnd() + val params = + messages.Either.forLeft[WorkDoneProgressNotification, Object](end) + client.notifyProgress(new ProgressParams(token, params)) + } + .recover { case _: NullPointerException => + // no such value in map + } def trackFuture[T]( message: String, diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala index 369635285e0..e2f8132ab5d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala @@ -204,13 +204,13 @@ final class ConfiguredLanguageClient( override def createProgress( params: WorkDoneProgressCreateParams - ): CompletableFuture[Void] = - if(clientConfig.slowTaskIsOn()) { + ): CompletableFuture[Void] = + if (clientConfig.slowTaskIsOn()) { underlying.createProgress(params) } else CompletableFuture.completedFuture(null) override def notifyProgress(params: ProgressParams): Unit = - if(clientConfig.slowTaskIsOn()) { + if (clientConfig.slowTaskIsOn()) { underlying.notifyProgress(params) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala index 4356e1ca1e7..747a486ce79 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala @@ -258,7 +258,8 @@ class DebugProvider( socket } - conn.withTimeout(60, TimeUnit.SECONDS) + conn + .withTimeout(60, TimeUnit.SECONDS) .recover { case exception => connectedToServer.tryFailure(exception) cancelPromise.trySuccess(()) diff --git a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala index ff596d0e1ca..e123413b9e5 100644 --- a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala @@ -250,7 +250,10 @@ class WorksheetProvider( ) } cancelables.add(Cancelable(() => completeEmptyResult())) - slowTaskProvider.trackFuture(s"Evaluating ${path.filename}", result.asScala) + slowTaskProvider.trackFuture( + s"Evaluating ${path.filename}", + result.asScala, + ) token.checkCanceled() // NOTE(olafurpg) Run evaluation in a custom thread so that we can // `Thread.stop()` it in case of infinite loop. I'm not aware of any diff --git a/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala b/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala index 738e5e1cb54..c16ae96f685 100644 --- a/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala +++ b/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala @@ -221,7 +221,7 @@ class DelegatingScalaService( params: DidChangeWorkspaceFoldersParams ): CompletableFuture[Unit] = underlying.didChangeWorkspaceFolders(params) - override def didCancelWorkDoneProgress( + override def didCancelWorkDoneProgress( params: WorkDoneProgressCancelParams ): Unit = underlying.didCancelWorkDoneProgress(params) diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index d3cdebcb5f6..62e05a98798 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -297,7 +297,7 @@ class SbtBloopLspSuite expectError = true, ) _ = assertStatus(!_.isInstalled) - _ = client.onBeginSlowTask = (_, _) => { } + _ = client.onBeginSlowTask = (_, _) => {} _ <- server.didSave("build.sbt")(_ + "\n// comment") _ = assertNoDiff(client.workspaceShowMessages, "") _ = assertStatus(!_.isInstalled) diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index 12a322e0d2d..d88a8794eb2 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -261,7 +261,7 @@ abstract class BaseWorksheetLspSuite( cleanWorkspace() val cancelled = Promise[Unit]() client.onBeginSlowTask = { (message, cancelParams) => - if(message.startsWith("Evaluating worksheet")) { + if (message.startsWith("Evaluating worksheet")) { cancelled.trySuccess(()) server.fullServer.didCancelWorkDoneProgress(cancelParams) } diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 033fd8bb0a9..be8ada45856 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -94,7 +94,8 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) var resetWorkspace = new MessageActionItem(ResetWorkspace.cancel) var regenerateAndRestartScalaCliBuildSever = FileOutOfScalaCliBspScope.ignore var shouldReloadAfterJavaHomeUpdate = ProjectJavaHomeUpdate.notNow - var onBeginSlowTask: (String, WorkDoneProgressCancelParams) => Unit = (_, _) => { } + var onBeginSlowTask: (String, WorkDoneProgressCancelParams) => Unit = + (_, _) => {} val resources = new ResourceOperations(buffers) val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] = TrieMap.empty[AbsolutePath, Seq[Diagnostic]] @@ -416,7 +417,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) } override def notifyProgress(params: ProgressParams): Unit = { - if(params.getValue().isLeft()) { + if (params.getValue().isLeft()) { params.getValue().getLeft() match { case begin: WorkDoneProgressBegin => val cancelParams = new WorkDoneProgressCancelParams(params.getToken()) diff --git a/tests/unit/src/main/scala/tests/TestingServer.scala b/tests/unit/src/main/scala/tests/TestingServer.scala index b5d0e7f8496..45c99e4032e 100644 --- a/tests/unit/src/main/scala/tests/TestingServer.scala +++ b/tests/unit/src/main/scala/tests/TestingServer.scala @@ -2091,7 +2091,7 @@ object TestingServer { InitializationOptions.Default.copy( debuggingProvider = Some(true), runProvider = Some(true), - treeViewProvider = Some(true) + treeViewProvider = Some(true), ) // Caching is done using a key: dependency jars + excludedPackages setting + bucket size diff --git a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala index 13123c0cf5a..79cdbb49950 100644 --- a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala +++ b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala @@ -21,7 +21,7 @@ class DebugProtocolCancelationSuite test("start") { cleanWorkspace() client.onBeginSlowTask = (message, cancelParams) => { - if(message == "Starting debug server") { + if (message == "Starting debug server") { server.fullServer.didCancelWorkDoneProgress(cancelParams) } } From b2f456226a467a5c7f18df785a97abafc6748d0d Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 21 Feb 2024 15:24:33 +0100 Subject: [PATCH 06/12] test fixes --- .../scala/meta/internal/metals/Indexer.scala | 2 +- .../scala/meta/internal/metals/Messages.scala | 3 + .../internal/metals/MetalsLspService.scala | 2 +- .../tests/MultipleBuildFilesLspSuite.scala | 10 +-- .../scala/tests/bazel/BazelLspSuite.scala | 8 +-- .../scala/tests/gradle/GradleLspSuite.scala | 55 +++++----------- .../scala/tests/maven/MavenLspSuite.scala | 40 ++++-------- .../test/scala/tests/mill/MillLspSuite.scala | 22 ++----- .../scala/tests/sbt/SbtBloopLspSuite.scala | 62 ++++++------------- .../src/main/scala/tests/TestingClient.scala | 14 ++++- .../test/scala/tests/FormattingLspSuite.scala | 3 +- 11 files changed, 72 insertions(+), 149 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala index 326b907bf6a..6ac1081f839 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -167,7 +167,7 @@ final case class Indexer( def profiledIndexWorkspace(check: () => Unit): Future[Unit] = { val tracked = slowTaskProvider.trackFuture( - s"Indexing", + Messages.indexing, Future { timerProvider.timedThunk("indexed workspace", onlyIf = true) { try indexWorkspace(check) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala index e4de17aea98..58e6a7331ff 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -1034,6 +1034,9 @@ object Messages { |See: https://scalameta.org/metals/docs/troubleshooting/faq#i-see-spurious-errors-or-worksheet-fails-to-evaluate |""".stripMargin + val indexing = "Indexing" + val importingBuild = "Importing build" + } object FileOutOfScalaCliBspScope { diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index 8b3df0f2574..61699c52293 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -2438,7 +2438,7 @@ class MetalsLspService( } for { bspBuilds <- slowTaskProvider.trackFuture( - "Importing build", + Messages.importingBuild, importedBuilds0, ) _ = { diff --git a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala index a306458ef4d..64f14501815 100644 --- a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala +++ b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala @@ -43,7 +43,6 @@ class MultipleBuildFilesLspSuite // Project has no .bloop directory so user is asked to "import via bloop" chooseBuildToolMessage, importBuildMessage, - progressMessage, ).mkString("\n"), ) _ = client.messageRequests.clear() // restart @@ -56,12 +55,9 @@ class MultipleBuildFilesLspSuite } yield { assertNoDiff( client.workspaceMessageRequests, - List( - // Ensure that after a choice was made, the user doesn't get re-prompted - // to choose their build tool again - importBuildChangesMessage, - progressMessage, - ).mkString("\n"), + // Ensure that after a choice was made, the user doesn't get re-prompted + // to choose their build tool again + importBuildChangesMessage, ) } } diff --git a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala index 9b0d8bbce46..e5cf9cc8438 100644 --- a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala +++ b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala @@ -42,7 +42,6 @@ class BazelLspSuite client.workspaceMessageRequests, List( importMessage, - "bazelbsp bspConfig", bazelNavigationMessage, ).mkString("\n"), ) @@ -125,10 +124,7 @@ class BazelLspSuite } yield { assertNoDiff( client.workspaceMessageRequests, - List( - "bazelbsp bspConfig", - bazelNavigationMessage, - ).mkString("\n"), + bazelNavigationMessage, ) assert(bazelBspConfig.exists) server.assertBuildServerConnection() @@ -195,10 +191,8 @@ class BazelLspSuite assertNoDiff( client.workspaceMessageRequests, List( - "bazelbsp bspConfig", bazelNavigationMessage, Messages.ResetWorkspace.message, - "bazelbsp bspConfig", ).mkString("\n"), ) assert(bazelBspConfig.exists) diff --git a/tests/slow/src/test/scala/tests/gradle/GradleLspSuite.scala b/tests/slow/src/test/scala/tests/gradle/GradleLspSuite.scala index 79ac9d9499d..9bb50693f17 100644 --- a/tests/slow/src/test/scala/tests/gradle/GradleLspSuite.scala +++ b/tests/slow/src/test/scala/tests/gradle/GradleLspSuite.scala @@ -5,6 +5,7 @@ import scala.concurrent.Future import scala.meta.internal.builds.GradleBuildTool import scala.meta.internal.builds.GradleDigest import scala.meta.internal.metals.ClientCommands +import scala.meta.internal.metals.Messages import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ServerCommands @@ -41,11 +42,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() // restart _ = assertStatus(_.isInstalled) @@ -63,11 +60,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { } yield { assertNoDiff( client.workspaceMessageRequests, - List( - // Project has .bloop directory so user is asked to "re-import project" - importBuildChangesMessage, - progressMessage, - ).mkString("\n"), + importBuildChangesMessage, ) } } @@ -145,11 +138,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() // restart _ = assertStatus(_.isInstalled) @@ -198,11 +187,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() _ = assertStatus(_.isInstalled) @@ -242,11 +227,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() _ = assertStatus(_.isInstalled) @@ -271,19 +252,17 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ <- server.server.buildServerPromise.future - _ = client.messageRequests.clear() // restart + _ = client.progressParams.clear() // restart _ <- server.executeCommand(ServerCommands.ImportBuild) _ = assertNoDiff( - client.workspaceMessageRequests, + client.beginProgressMessages, List( - progressMessage + progressMessage, + Messages.importingBuild, + Messages.indexing, ).mkString("\n"), ) } yield () @@ -341,10 +320,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertNoDiff( client.workspaceShowMessages, @@ -366,10 +342,7 @@ class GradleLspSuite extends BaseImportSuite("gradle-import") { } _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertStatus(_.isInstalled) } yield () diff --git a/tests/slow/src/test/scala/tests/maven/MavenLspSuite.scala b/tests/slow/src/test/scala/tests/maven/MavenLspSuite.scala index 80f025c01f7..6bf89773cd9 100644 --- a/tests/slow/src/test/scala/tests/maven/MavenLspSuite.scala +++ b/tests/slow/src/test/scala/tests/maven/MavenLspSuite.scala @@ -5,6 +5,7 @@ import java.nio.charset.StandardCharsets import scala.meta.internal.builds.MavenBuildTool import scala.meta.internal.builds.MavenDigest import scala.meta.internal.io.InputStreamIO +import scala.meta.internal.metals.Messages import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ServerCommands @@ -40,10 +41,7 @@ class MavenLspSuite extends BaseImportSuite("maven-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() // restart _ = assertStatus(_.isInstalled) @@ -64,11 +62,7 @@ class MavenLspSuite extends BaseImportSuite("maven-import") { } yield { assertNoDiff( client.workspaceMessageRequests, - List( - // Project has .bloop directory so user is asked to "re-import project" - importBuildChangesMessage, - progressMessage, - ).mkString("\n"), + importBuildChangesMessage, ) } } @@ -84,17 +78,16 @@ class MavenLspSuite extends BaseImportSuite("maven-import") { _ <- server.server.buildServerPromise.future _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) - _ = client.messageRequests.clear() // restart + _ = client.progressParams.clear() // restart _ <- server.executeCommand(ServerCommands.ImportBuild) _ = assertNoDiff( - client.workspaceMessageRequests, + client.beginProgressMessages, List( - progressMessage + progressMessage, + Messages.importingBuild, + Messages.indexing, ).mkString("\n"), ) } yield () @@ -120,10 +113,7 @@ class MavenLspSuite extends BaseImportSuite("maven-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) debugger <- server.startDebugging( "maven-test-repo", @@ -201,10 +191,7 @@ class MavenLspSuite extends BaseImportSuite("maven-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertNoDiff( client.workspaceShowMessages, @@ -215,10 +202,7 @@ class MavenLspSuite extends BaseImportSuite("maven-import") { _ <- server.didSave("pom.xml")(_ => defaultPom) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertStatus(_.isInstalled) } yield () diff --git a/tests/slow/src/test/scala/tests/mill/MillLspSuite.scala b/tests/slow/src/test/scala/tests/mill/MillLspSuite.scala index 01b87b31b47..7a0f49a67b7 100644 --- a/tests/slow/src/test/scala/tests/mill/MillLspSuite.scala +++ b/tests/slow/src/test/scala/tests/mill/MillLspSuite.scala @@ -30,11 +30,7 @@ class MillLspSuite extends BaseImportSuite("mill-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() // restart _ = assertStatus(_.isInstalled) @@ -57,11 +53,7 @@ class MillLspSuite extends BaseImportSuite("mill-import") { } yield { assertNoDiff( client.workspaceMessageRequests, - List( - // Project has .bloop directory so user is asked to "re-import project" - importBuildChangesMessage, - progressMessage, - ).mkString("\n"), + importBuildChangesMessage, ) } } @@ -114,10 +106,7 @@ class MillLspSuite extends BaseImportSuite("mill-import") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertNoDiff( client.workspaceShowMessages, @@ -135,10 +124,7 @@ class MillLspSuite extends BaseImportSuite("mill-import") { } _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertStatus(_.isInstalled) } yield () diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index 62e05a98798..14493b0a061 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -9,6 +9,7 @@ import scala.meta.internal.builds.SbtDigest import scala.meta.internal.io.FileIO import scala.meta.internal.metals.ClientCommands import scala.meta.internal.metals.InitializationOptions +import scala.meta.internal.metals.Messages import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ServerCommands @@ -56,11 +57,7 @@ class SbtBloopLspSuite ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() // restart _ = assertStatus(_.isInstalled) @@ -78,11 +75,7 @@ class SbtBloopLspSuite } yield { assertNoDiff( client.workspaceMessageRequests, - List( - // Project has .bloop directory so user is asked to "re-import project" - importBuildChangesMessage, - progressMessage, - ).mkString("\n"), + importBuildChangesMessage, ) } } @@ -151,18 +144,16 @@ class SbtBloopLspSuite _ <- server.server.buildServerPromise.future _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) - _ = client.messageRequests.clear() // restart + _ = client.progressParams.clear() // restart _ <- server.executeCommand(ServerCommands.ImportBuild) _ = assertNoDiff( - client.workspaceMessageRequests, + client.beginProgressMessages, List( - progressMessage + progressMessage, + Messages.importingBuild, + Messages.indexing, ).mkString("\n"), ) } yield () @@ -181,11 +172,7 @@ class SbtBloopLspSuite _ <- server.server.buildServerPromise.future _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = client.messageRequests.clear() // restart _ <- server.didChangeConfiguration( @@ -197,10 +184,7 @@ class SbtBloopLspSuite _ <- server.executeCommand(ServerCommands.ImportBuild) _ = assertNoDiff( client.workspaceMessageRequests, - List( - BloopVersionChange.msg, - progressMessage, - ).mkString("\n"), + BloopVersionChange.msg, ) _ = assertStatus(_.isInstalled) } yield () @@ -218,20 +202,18 @@ class SbtBloopLspSuite ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - // Project has no .bloop directory so user is asked to "import via bloop" - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) - _ = client.messageRequests.clear() // restart + _ = client.progressParams.clear() // restart _ <- server .executeCommand(ServerCommands.ImportBuild) .zip(server.executeCommand(ServerCommands.ImportBuild)) _ = assertNoDiff( - client.workspaceMessageRequests, + client.beginProgressMessages, List( - progressMessage + progressMessage, + Messages.importingBuild, + Messages.indexing, ).mkString("\n"), ) _ = assertNoDiff( @@ -320,10 +302,7 @@ class SbtBloopLspSuite ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertNoDiff( client.workspaceShowMessages, @@ -336,10 +315,7 @@ class SbtBloopLspSuite } _ = assertNoDiff( client.workspaceMessageRequests, - List( - importBuildMessage, - progressMessage, - ).mkString("\n"), + importBuildMessage, ) _ = assertStatus(_.isInstalled) } yield () diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index be8ada45856..dc5d522528f 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -134,6 +134,19 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) val testExplorerUpdates: Promise[List[JsonObject]] = Promise[List[JsonObject]]() + def beginProgressMessages: String = + progressParams.asScala + .flatMap { params => + if (params.getValue().isLeft()) { + params.getValue().getLeft() match { + case begin: WorkDoneProgressBegin => + Some(begin.getTitle()) + case _ => None + } + } else None + } + .mkString("\n") + override def metalsExecuteClientCommand( params: ExecuteCommandParams ): Unit = { @@ -422,7 +435,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) case begin: WorkDoneProgressBegin => val cancelParams = new WorkDoneProgressCancelParams(params.getToken()) onBeginSlowTask(begin.getTitle(), cancelParams) - messageRequests.addLast(begin.getTitle()) case _ => } } diff --git a/tests/unit/src/test/scala/tests/FormattingLspSuite.scala b/tests/unit/src/test/scala/tests/FormattingLspSuite.scala index 28fce6e9ba5..10c21667cd8 100644 --- a/tests/unit/src/test/scala/tests/FormattingLspSuite.scala +++ b/tests/unit/src/test/scala/tests/FormattingLspSuite.scala @@ -345,8 +345,7 @@ class FormattingLspSuite extends BaseLspSuite("formatting") { _ = { assertNoDiff( client.workspaceMessageRequests, - s"""|${MissingScalafmtVersion.messageRequestMessage} - |Loading Scalafmt""".stripMargin, + MissingScalafmtVersion.messageRequestMessage, ) assertNoDiff( client.workspaceShowMessages, From e379913a0d3c7e401f1411581807ebad602a6a2c Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 5 Mar 2024 13:21:45 +0100 Subject: [PATCH 07/12] rename `slowTask` to `workDoneProgress` --- docs/editors/overview.md | 2 - docs/integrations/new-editor.md | 63 +------------------ .../src/main/scala/bench/PcBenchmark.scala | 2 +- .../meta/internal/bsp/BspConnector.scala | 6 +- .../internal/builds/NewProjectProvider.scala | 6 +- .../meta/internal/builds/ShellRunner.scala | 6 +- .../internal/metals/ClientConfiguration.scala | 6 +- .../ClientExperimentalCapabilities.scala | 3 - .../meta/internal/metals/Compilers.scala | 6 +- .../scala/meta/internal/metals/Embedded.scala | 4 +- .../internal/metals/FormattingProvider.scala | 4 +- .../metals/ForwardingMetalsBuildClient.scala | 12 ++-- .../scala/meta/internal/metals/Indexer.scala | 4 +- .../internal/metals/MetalsLspService.scala | 28 ++++----- .../internal/metals/MetalsServerConfig.scala | 3 - .../internal/metals/ScalafixProvider.scala | 6 +- .../meta/internal/metals/SlowTaskConfig.scala | 15 ----- ...{SlowTask.scala => WorkDoneProgress.scala} | 16 ++--- .../internal/metals/WorkspaceLspService.scala | 14 ++--- .../internal/metals/ammonite/Ammonite.scala | 4 +- .../language/ConfiguredLanguageClient.scala | 4 +- .../internal/metals/debug/DebugProvider.scala | 8 +-- .../internal/metals/debug/DebugProxy.scala | 10 +-- .../internal/metals/scalacli/ScalaCli.scala | 6 +- .../worksheets/WorksheetProvider.scala | 11 ++-- .../scala/tests/sbt/CancelCompileSuite.scala | 6 +- .../scala/tests/sbt/SbtBloopLspSuite.scala | 4 +- .../scala/tests/scalacli/ScalaCliSuite.scala | 4 -- .../src/main/scala/tests/BaseLspSuite.scala | 4 +- .../scala/tests/BaseWorksheetLspSuite.scala | 2 +- .../main/scala/tests/TestScala3Compiler.scala | 4 +- .../src/main/scala/tests/TestingClient.scala | 4 +- .../debug/DebugProtocolCancelationSuite.scala | 2 +- 33 files changed, 92 insertions(+), 187 deletions(-) delete mode 100644 metals/src/main/scala/scala/meta/internal/metals/SlowTaskConfig.scala rename metals/src/main/scala/scala/meta/internal/metals/{SlowTask.scala => WorkDoneProgress.scala} (95%) diff --git a/docs/editors/overview.md b/docs/editors/overview.md index a3e122c9e17..7b376643b9f 100644 --- a/docs/editors/overview.md +++ b/docs/editors/overview.md @@ -455,8 +455,6 @@ website. **Did focus**: Editor client implements the `metals/didFocusTextDocument` notification. -**Slow task**: Editor client implements the `metals/slowTask` request. - **Input box**: Editor client implements the `metals/inputBox` request. **Quick pick**: Editor client implements the `metals/quickPick` request. diff --git a/docs/integrations/new-editor.md b/docs/integrations/new-editor.md index 22fa30cfe03..317d404ff11 100644 --- a/docs/integrations/new-editor.md +++ b/docs/integrations/new-editor.md @@ -115,7 +115,6 @@ The currently available settings for `InitializationOptions` are listed below. openFilesOnRenameProvider?: boolean; quickPickProvider?: boolean; renameFileThreshold?: number; - slowTaskProvider?: boolean; statusBarProvider?: "on" | "off" | "log-message" | "show-message"; treeViewProvider?: boolean; testExplorerProvider?: boolean; @@ -366,13 +365,6 @@ a `openFilesOnRenameProvider`. Default value: `300` -##### `slowTaskProvider` - -Possible values: - -- `off` (default): the `metals/slowTask` request is not supported. -- `on`: the `metals/slowTask` request is fully supported. - ##### `statusBarProvider` Possible values: @@ -426,10 +418,6 @@ Metals to clean up open resources. Kills the process using `System.exit`. -### `$/cancelRequest` - -Used by `metals/slowTask` to notify when a long-running process has finished. - ### `client/registerCapability` If the client declares the `workspace.didChangeWatchedFiles` capability during @@ -561,8 +549,7 @@ non-critical notifications, see `metals/status`. ### `window/showMessageRequest` Used to send critical and actionable notifications to the user. To notify the -user about long running tasks that can be cancelled, the extension -`metals/slowTask` is used instead. +user about long running tasks that can be cancelled. ## Metals server properties @@ -714,54 +701,6 @@ views in the editor client, the Metals implements an LSP extension to display non-editable text in the editor, see the [Decoration Protocol](../integrations/decoration-protocol.md). -### `metals/slowTask` - -The Metals slow task request is sent from the server to the client to notify the -start of a long running process with unknown estimated total time. A -`cancel: true` response from the client cancels the task. A `$/cancelRequest` -request from the server indicates that the task has completed. - -![Metals slow task](https://i.imgur.com/nsjWHWR.gif) - -The difference between `metals/slowTask` and `window/showMessageRequest` is that -`slowTask` is time-sensitive and the interface should display a timer for how -long the task has been running while `showMessageRequest` is static. - -_Request_: - -- method: `metals/slowTask` -- params: `MetalsSlowTaskParams` defined as follows: - -```ts -interface MetalsSlowTaskParams { - /** The name of this slow task */ - message: string; - /** - * If true, the log output from this task does not need to be displayed to the user. - * - * In VS Code, the Metals "Output channel" is not toggled when this flag is true. - */ - quietLogs?: boolean; - /** Time that has elapsed since the begging of the task. */ - secondsElapsed?: number; -} -``` - -_Response_: - -- result: `MetalsSlowTaskResponse` defined as follows - -```ts -interface MetalsSlowTaskResult { - /** - * If true, cancel the running task. - * If false, the user dismissed the dialogue but want to - * continue running the task. - */ - cancel: boolean; -} -``` - ### `metals/status` The Metals status notification is sent from the server to the client to notify diff --git a/metals-bench/src/main/scala/bench/PcBenchmark.scala b/metals-bench/src/main/scala/bench/PcBenchmark.scala index d6df461501f..f3afc1d21a1 100644 --- a/metals-bench/src/main/scala/bench/PcBenchmark.scala +++ b/metals-bench/src/main/scala/bench/PcBenchmark.scala @@ -30,7 +30,7 @@ abstract class PcBenchmark { protected implicit val ec: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global protected val embedded = new Embedded( - new metals.SlowTask(NoopLanguageClient, metals.Time.system) + new metals.WorkDoneProgress(NoopLanguageClient, metals.Time.system) ) protected final val benchmarkedScalaVersions: Array[String] = Array( diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index b39604881c9..328d02ba7a6 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -17,10 +17,10 @@ import scala.meta.internal.metals.BuildServerConnection import scala.meta.internal.metals.Messages import scala.meta.internal.metals.Messages.BspSwitch import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.Tables import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.semver.SemVer import scala.meta.io.AbsolutePath @@ -37,7 +37,7 @@ class BspConnector( tables: Tables, userConfig: () => UserConfiguration, statusBar: StatusBar, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, bspConfigGenerator: BspConfigGenerator, currentConnection: () => Option[BuildServerConnection], restartBspServer: () => Future[Boolean], @@ -131,7 +131,7 @@ class BspConnector( if (shouldReload) connection.workspaceReload() else Future.successful(()) } yield connection - slowTaskProvider + workDoneProgress .trackFuture("Connecting to sbt", connectionF) .map(Some(_)) case ResolvedBspOne(details) => diff --git a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala index 9a7f2511c91..ee9d1131d40 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala @@ -13,7 +13,7 @@ import scala.meta.internal.metals.ClientConfiguration import scala.meta.internal.metals.Icons import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.SlowTask +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.clients.language.MetalsInputBoxParams import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.metals.clients.language.MetalsOpenWindowParams @@ -26,7 +26,7 @@ import coursierapi._ class NewProjectProvider( client: MetalsLanguageClient, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, config: ClientConfiguration, shell: ShellRunner, icons: Icons, @@ -46,7 +46,7 @@ class NewProjectProvider( if (allTemplates.nonEmpty) { allTemplates } else { - slowTaskProvider.trackBlocking( + workDoneProgress.trackBlocking( "Fetching template information from Github" ) { // Matches: diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index 9d3540a885a..612244fb180 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -15,16 +15,16 @@ import scala.meta.internal.metals.JavaBinary import scala.meta.internal.metals.JdkSources import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.process.ExitCodes import scala.meta.internal.process.SystemProcess import scala.meta.io.AbsolutePath import coursierapi._ -class ShellRunner(time: Time, slowTaskProvider: SlowTask)(implicit +class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit executionContext: scala.concurrent.ExecutionContext ) extends Cancelable { @@ -109,7 +109,7 @@ class ShellRunner(time: Time, slowTaskProvider: SlowTask)(implicit cancelables.add(newCancelable) val processFuture = ps.complete - slowTaskProvider.trackFuture( + workDoneProvider.trackFuture( commandRun, processFuture, onCancel = Some(() => { 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 17a0aac9792..537f41843b2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala @@ -75,13 +75,13 @@ final class ClientConfiguration( .map(Icons.fromString) .getOrElse(initialConfig.icons) - def slowTaskIsOn(): Boolean = + def hasWorkDoneProgressCapability(): Boolean = (for { params <- initializeParams capabilities <- Option(params.getCapabilities()) window <- Option(capabilities.getWindow()) - slowTask <- Option(window.getWorkDoneProgress()) - } yield slowTask.booleanValue()).getOrElse(false) + workDoneProgress <- Option(window.getWorkDoneProgress()) + } yield workDoneProgress.booleanValue()).getOrElse(false) def isExecuteClientCommandProvider(): Boolean = extract( diff --git a/metals/src/main/scala/scala/meta/internal/metals/ClientExperimentalCapabilities.scala b/metals/src/main/scala/scala/meta/internal/metals/ClientExperimentalCapabilities.scala index c23adefe421..d7533bb9cc0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientExperimentalCapabilities.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientExperimentalCapabilities.scala @@ -22,7 +22,6 @@ final case class ClientExperimentalCapabilities( inputBoxProvider: Option[Boolean], openFilesOnRenameProvider: Option[Boolean], quickPickProvider: Option[Boolean], - slowTaskProvider: Option[Boolean], statusBarProvider: Option[String], treeViewProvider: Option[Boolean], ) { @@ -46,7 +45,6 @@ object ClientExperimentalCapabilities { None, None, None, - None, ) def from( @@ -80,7 +78,6 @@ object ClientExperimentalCapabilities { openFilesOnRenameProvider = jsonObj.getBooleanOption("openFilesOnRenameProvider"), quickPickProvider = jsonObj.getBooleanOption("quickPickProvider"), - slowTaskProvider = jsonObj.getBooleanOption("slowTaskProvider"), statusBarProvider = jsonObj.getStringOption("statusBarProvider"), treeViewProvider = jsonObj.getBooleanOption("treeViewProvider"), ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala index a6649a246c7..c5af1155490 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala @@ -72,7 +72,7 @@ class Compilers( buffers: Buffers, search: SymbolSearch, embedded: Embedded, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, sh: ScheduledExecutorService, initializeParams: InitializeParams, excludedPackages: () => ExcludedPackagesHandler, @@ -938,7 +938,7 @@ class Compilers( } yield { jworksheetsCache.put( path, - slowTaskProvider.trackBlocking( + workDoneProgress.trackBlocking( s"${config.icons.sync}Loading worksheet presentation compiler" ) { ScalaLazyCompiler.forWorksheet( @@ -1001,7 +1001,7 @@ class Compilers( val out = jcache.computeIfAbsent( PresentationCompilerKey.ScalaBuildTarget(scalaTarget.info.getId), { _ => - slowTaskProvider.trackBlocking( + workDoneProgress.trackBlocking( s"${config.icons.sync}Loading presentation compiler" ) { ScalaLazyCompiler(scalaTarget, mtags, search) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala b/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala index 29a4b3d42a0..3253788050d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Embedded.scala @@ -27,7 +27,7 @@ import mdoc.interfaces.Mdoc * - mdoc */ final class Embedded( - slowTaskProvider: SlowTask + workDoneProgress: WorkDoneProgress ) extends Cancelable { private val mdocs: TrieMap[String, URLClassLoader] = @@ -49,7 +49,7 @@ final class Embedded( if (isScala3) Some(scalaVersion) else None val classloader = mdocs.getOrElseUpdate( scalaVersionKey, - slowTaskProvider.trackBlocking("Preparing worksheets") { + workDoneProgress.trackBlocking("Preparing worksheets") { newMdocClassLoader(scalaBinaryVersion, resolveSpecificVersionCompiler) }, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala index d19f61f31ac..3f4bcdfd806 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala @@ -48,7 +48,7 @@ final class FormattingProvider( client: MetalsLanguageClient, clientConfig: ClientConfiguration, statusBar: StatusBar, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, icons: Icons, tables: Tables, buildTargets: BuildTargets, @@ -504,7 +504,7 @@ final class FormattingProvider( def downloadOutputStream(): OutputStream = { downloadingScalafmt.trySuccess(()) downloadingScalafmt = Promise() - slowTaskProvider.trackFuture( + workDoneProgress.trackFuture( "Loading Scalafmt", downloadingScalafmt.future, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index 99f35b37de3..9ac28d0a70a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -16,11 +16,11 @@ import scala.meta.internal.metals.ConcurrentHashSet import scala.meta.internal.metals.Diagnostics import scala.meta.internal.metals.MetalsBuildClient import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.TaskProgress import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.tvp._ import scala.meta.io.AbsolutePath @@ -56,7 +56,7 @@ final class ForwardingMetalsBuildClient( onBuildTargetDidCompile: b.BuildTargetIdentifier => Unit, onBuildTargetDidChangeFunc: b.DidChangeBuildTarget => Unit, bspErrorHandler: BspErrorHandler, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, ) extends MetalsBuildClient with Cancelable { @@ -71,19 +71,19 @@ final class ForwardingMetalsBuildClient( private class Compilation( val timer: Timer, val isNoOp: Boolean, - token: Option[Future[SlowTask.Token]], + token: Option[Future[WorkDoneProgress.Token]], taskProgress: TaskProgress = TaskProgress.empty, ) extends TreeViewCompilation { def progressPercentage = taskProgress.percentage - def end(): Unit = token.foreach(slowTaskProvider.endSlowTask(_)) + def end(): Unit = token.foreach(workDoneProgress.endProgress(_)) def updateProgress(progress: Long, total: Long = 100): Unit = { val prev = taskProgress.percentage taskProgress.update(progress, total) if (prev != taskProgress.percentage) { - token.foreach(slowTaskProvider.notifyProgress(_, progressPercentage)) + token.foreach(workDoneProgress.notifyProgress(_, progressPercentage)) } } } @@ -185,7 +185,7 @@ final class ForwardingMetalsBuildClient( ) val token = Option.when(!isNoOp) { - slowTaskProvider.startSlowTask( + workDoneProgress.startProgress( s"Compiling $name", withProgress = true, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala index 6ac1081f839..23ba6a948fd 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -55,7 +55,7 @@ final case class Indexer( executionContext: ExecutionContextExecutorService, tables: Tables, statusBar: () => StatusBar, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, timerProvider: TimerProvider, scalafixProvider: () => ScalafixProvider, indexingPromise: () => Promise[Unit], @@ -166,7 +166,7 @@ final case class Indexer( } def profiledIndexWorkspace(check: () => Unit): Future[Unit] = { - val tracked = slowTaskProvider.trackFuture( + val tracked = workDoneProgress.trackFuture( Messages.indexing, Future { timerProvider.timedThunk("indexed workspace", onlyIf = true) { diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index 61699c52293..5f27319d79d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -134,7 +134,7 @@ class MetalsLspService( folderVisibleName: Option[String], headDoctor: HeadDoctor, bspStatus: BspStatus, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, ) extends Folder(folder, folderVisibleName, isKnownMetalsProject = true) with Cancelable with TextDocumentService { @@ -173,7 +173,7 @@ class MetalsLspService( private implicit val executionContext: ExecutionContextExecutorService = ec private val embedded: Embedded = register( - new Embedded(slowTaskProvider) + new Embedded(workDoneProgress) ) val tables: Tables = register(new Tables(folder, time)) @@ -417,7 +417,7 @@ class MetalsLspService( onBuildTargetChanges(params) }, bspErrorHandler, - slowTaskProvider, + workDoneProgress, ) private val bloopServers: BloopServers = new BloopServers( @@ -446,7 +446,7 @@ class MetalsLspService( tables, () => userConfig, statusBar, - slowTaskProvider, + workDoneProgress, bspConfigGenerator, () => bspSession.map(_.mainConnection), restartBspServer, @@ -548,7 +548,7 @@ class MetalsLspService( languageClient, clientConfig, statusBar, - slowTaskProvider, + workDoneProgress, clientConfig.icons, tables, buildTargets, @@ -602,7 +602,7 @@ class MetalsLspService( buildTargets, languageClient, () => userConfig, - slowTaskProvider, + workDoneProgress, diagnostics, embedded, worksheetPublisher, @@ -621,7 +621,7 @@ class MetalsLspService( buffers, symbolSearch, embedded, - slowTaskProvider, + workDoneProgress, sh, initializeParams, () => excludedPackageHandler, @@ -721,7 +721,7 @@ class MetalsLspService( semanticdbs, compilers, statusBar, - slowTaskProvider, + workDoneProgress, sourceMapper, () => userConfig, testProvider, @@ -733,7 +733,7 @@ class MetalsLspService( buffers, () => userConfig, folder, - slowTaskProvider, + workDoneProgress, compilations, languageClient, buildTargets, @@ -844,7 +844,7 @@ class MetalsLspService( buffers, compilers, compilations, - slowTaskProvider, + workDoneProgress, diagnostics, tables, languageClient, @@ -1862,7 +1862,7 @@ class MetalsLspService( def cleanCompile(): Future[Unit] = compilations.recompileAll() def cancelCompile(): Future[Unit] = Future { - // We keep this in here to provide a way for clients that aren't slowTask providers + // We keep this in here to provide a way for clients that aren't work done progress cancel providers // to be able to cancel a long-running worksheet evaluation by canceling compilation. if (focusedDocument().exists(_.isWorksheet)) worksheetProvider.cancel() @@ -2437,7 +2437,7 @@ class MetalsLspService( session.importBuilds() } for { - bspBuilds <- slowTaskProvider.trackFuture( + bspBuilds <- workDoneProgress.trackFuture( Messages.importingBuild, importedBuilds0, ) @@ -2480,7 +2480,7 @@ class MetalsLspService( new ScalaCli( () => compilers, compilations, - slowTaskProvider, + workDoneProgress, buffers, () => indexer.profiledIndexWorkspace(() => ()), () => diagnostics, @@ -2502,7 +2502,7 @@ class MetalsLspService( executionContext, tables, () => statusBar, - slowTaskProvider, + workDoneProgress, timerProvider, () => scalafixProvider, () => indexingPromise, 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 5da255d443a..c1667bc6120 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala @@ -15,7 +15,6 @@ import scala.meta.pc.PresentationCompilerConfig.OverrideDefFormat * @param globSyntax pattern used for `DidChangeWatchedFilesRegistrationOptions`. * @param statusBar how to handle metals/status notifications with {"statusType": "metals"}. * @param bspStatusBar how to handle metals/status notifications with {"statusType": "bsp"}. - * @param slowTask how to handle metals/slowTask requests. * @param executeClientCommand whether client provides the ability to support the * `metals/executeClientCommand` command. * @param snippetAutoIndent if the client defaults to adding the identation of @@ -50,7 +49,6 @@ final case class MetalsServerConfig( globSyntax: GlobSyntaxConfig = GlobSyntaxConfig.default, statusBar: StatusBarConfig = StatusBarConfig.default, bspStatusBar: StatusBarConfig = StatusBarConfig.bspDefault, - slowTask: SlowTaskConfig = SlowTaskConfig.default, executeClientCommand: ExecuteClientCommandConfig = ExecuteClientCommandConfig.default, snippetAutoIndent: Boolean = MetalsServerConfig.binaryOption( @@ -119,7 +117,6 @@ final case class MetalsServerConfig( s"status-bar=$statusBar", s"open-files-on-rename=$openFilesOnRenames", s"rename-file-threshold=$renameFileThreshold", - s"slow-task=$slowTask", s"execute-client-command=$executeClientCommand", s"compilers=$compilers", s"http=$isHttpEnabled", diff --git a/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala index 58b1f981b75..284cde5a132 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala @@ -40,7 +40,7 @@ case class ScalafixProvider( buffers: Buffers, userConfig: () => UserConfiguration, workspace: AbsolutePath, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, compilations: Compilations, languageClient: MetalsLanguageClient, buildTargets: BuildTargets, @@ -456,7 +456,7 @@ case class ScalafixProvider( scalafixCache.get(scalaBinaryVersion) match { case Some(value) => Future.successful(value) case None => - slowTaskProvider.trackBlocking("Downloading scalafix") { + workDoneProgress.trackBlocking("Downloading scalafix") { val scalafix = if (scalaBinaryVersion == "2.11") Future(scala211Fallback) else @@ -491,7 +491,7 @@ case class ScalafixProvider( rulesClassloaderCache.get(scalfixRulesKey) match { case Some(value) => Future.successful(value) case None => - slowTaskProvider.trackBlocking( + workDoneProgress.trackBlocking( "Downloading scalafix rules' dependencies" ) { val rulesDependencies = scalfixRulesKey.usedRulesWithClasspath diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTaskConfig.scala b/metals/src/main/scala/scala/meta/internal/metals/SlowTaskConfig.scala deleted file mode 100644 index 1f711722ef2..00000000000 --- a/metals/src/main/scala/scala/meta/internal/metals/SlowTaskConfig.scala +++ /dev/null @@ -1,15 +0,0 @@ -package scala.meta.internal.metals - -final case class SlowTaskConfig(value: String) { - def isOff: Boolean = value == "off" - def isOn: Boolean = value == "on" -} - -object SlowTaskConfig { - def off = new SlowTaskConfig("off") - def on = new SlowTaskConfig("on") - def default = - new SlowTaskConfig( - System.getProperty("metals.slow-task", "off") - ) -} diff --git a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala similarity index 95% rename from metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala rename to metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala index a5510740a40..910dab204eb 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/SlowTask.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala @@ -20,7 +20,7 @@ import org.eclipse.lsp4j.WorkDoneProgressReport import org.eclipse.lsp4j.jsonrpc.messages import org.eclipse.lsp4j.services.LanguageClient -class SlowTask( +class WorkDoneProgress( client: LanguageClient, time: Time, )(implicit ec: ExecutionContext) @@ -74,7 +74,7 @@ class SlowTask( private def updateTimers() = taskMap.keys.asScala.foreach(notifyProgress(_)) - def startSlowTask( + def startProgress( message: String, withProgress: Boolean = false, showTimer: Boolean = true, @@ -146,7 +146,7 @@ class SlowTask( } else Future.successful(()) } - def endSlowTask(token: Future[Token]): Future[Unit] = + def endProgress(token: Future[Token]): Future[Unit] = token .map { token => taskMap.remove(token) @@ -166,17 +166,17 @@ class SlowTask( showTimer: Boolean = true, )(implicit ec: ExecutionContext): Future[T] = { val token = - startSlowTask(message, onCancel = onCancel, showTimer = showTimer) + startProgress(message, onCancel = onCancel, showTimer = showTimer) value.map { result => - endSlowTask(token) + endProgress(token) result } } def trackBlocking[T](message: String)(thunk: => T): T = { - val token = startSlowTask(message) + val token = startProgress(message) val result = thunk - endSlowTask(token) + endProgress(token) result } @@ -196,6 +196,6 @@ class SlowTask( } } -object SlowTask { +object WorkDoneProgress { type Token = messages.Either[String, Integer] } diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index 40f57a352ff..31455f905f4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -123,8 +123,8 @@ class WorkspaceLspService( languageClient } - private val slowTaskProvider = register { - new SlowTask(languageClient, time) + private val workDoneProgress = register { + new WorkDoneProgress(languageClient, time) } private val userConfigSync = @@ -138,7 +138,7 @@ class WorkspaceLspService( } private val shellRunner = register { - new ShellRunner(time, slowTaskProvider) + new ShellRunner(time, workDoneProgress) } var focusedDocument: Option[AbsolutePath] = None @@ -185,7 +185,7 @@ class WorkspaceLspService( name, doctor, bspStatus, - slowTaskProvider, + workDoneProgress, ) } @@ -216,7 +216,7 @@ class WorkspaceLspService( private val newProjectProvider: NewProjectProvider = new NewProjectProvider( languageClient, - slowTaskProvider, + workDoneProgress, clientConfig, shellRunner, clientConfig.icons, @@ -620,7 +620,7 @@ class WorkspaceLspService( override def didCancelWorkDoneProgress( params: lsp4j.WorkDoneProgressCancelParams - ): Unit = slowTaskProvider.canceled(params.getToken()) + ): Unit = workDoneProgress.canceled(params.getToken()) def doctorVisibilityDidChange( params: DoctorVisibilityDidChangeParams @@ -1164,7 +1164,7 @@ class WorkspaceLspService( def initialized(): Future[Unit] = { statusBar.start(sh, 0, 1, ju.concurrent.TimeUnit.SECONDS) - slowTaskProvider.start(sh, 0, 1, ju.concurrent.TimeUnit.SECONDS) + workDoneProgress.start(sh, 0, 1, ju.concurrent.TimeUnit.SECONDS) for { _ <- userConfigSync.initSyncUserConfiguration(folderServices) _ <- Future.sequence(folderServices.map(_.initialized())) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala b/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala index cd53241c499..37713da7064 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ammonite/Ammonite.scala @@ -40,7 +40,7 @@ final class Ammonite( buffers: Buffers, compilers: Compilers, compilations: Compilations, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, diagnostics: Diagnostics, tables: Tables, languageClient: MetalsLanguageClient, @@ -98,7 +98,7 @@ final class Ammonite( case Some(conn) => compilers.cancel() for { - build0 <- slowTaskProvider.trackFuture( + build0 <- workDoneProgress.trackFuture( "Importing Ammonite scripts", ImportedBuild.fromConnection(conn), ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala index e2f8132ab5d..ab8e8c8ef28 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/clients/language/ConfiguredLanguageClient.scala @@ -205,12 +205,12 @@ final class ConfiguredLanguageClient( override def createProgress( params: WorkDoneProgressCreateParams ): CompletableFuture[Void] = - if (clientConfig.slowTaskIsOn()) { + if (clientConfig.hasWorkDoneProgressCapability()) { underlying.createProgress(params) } else CompletableFuture.completedFuture(null) override def notifyProgress(params: ProgressParams): Unit = - if (clientConfig.slowTaskIsOn()) { + if (clientConfig.hasWorkDoneProgressCapability()) { underlying.notifyProgress(params) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala index 747a486ce79..15ec657c05e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala @@ -38,11 +38,11 @@ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable import scala.meta.internal.metals.ScalaTestSuites import scala.meta.internal.metals.ScalaTestSuitesDebugRequest -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.SourceMapper import scala.meta.internal.metals.StacktraceAnalyzer import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.clients.language.LogForwarder import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.metals.clients.language.MetalsQuickPickItem @@ -86,7 +86,7 @@ class DebugProvider( semanticdbs: Semanticdbs, compilers: Compilers, statusBar: StatusBar, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, sourceMapper: SourceMapper, userConfig: () => UserConfiguration, testProvider: TestSuitesProvider, @@ -144,7 +144,7 @@ class DebugProvider( ) debugServer <- if (isJvm) - slowTaskProvider.trackFuture( + workDoneProgress.trackFuture( "Starting debug server", start( sessionName, @@ -294,7 +294,7 @@ class DebugProvider( compilers, workspace, clientConfig.disableColorOutput(), - slowTaskProvider, + workDoneProgress, sourceMapper, compilations, targets, diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala index 176db6ed3ac..914ad5ad8c3 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala @@ -19,10 +19,10 @@ import scala.meta.internal.metals.Compilers import scala.meta.internal.metals.EmptyCancelToken import scala.meta.internal.metals.JsonParser._ import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.SourceMapper import scala.meta.internal.metals.StacktraceAnalyzer import scala.meta.internal.metals.Trace +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.debug.DebugProtocol.CompletionRequest import scala.meta.internal.metals.debug.DebugProtocol.DisconnectRequest import scala.meta.internal.metals.debug.DebugProtocol.ErrorOutputNotification @@ -55,7 +55,7 @@ private[debug] final class DebugProxy( stackTraceAnalyzer: StacktraceAnalyzer, compilers: Compilers, stripColor: Boolean, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, sourceMapper: SourceMapper, compilations: Compilations, targets: Seq[BuildTargetIdentifier], @@ -94,7 +94,7 @@ private[debug] final class DebugProxy( case _ if cancelled.get() => () // ignore case request @ InitializeRequest(args) => - slowTaskProvider.trackFuture( + workDoneProgress.trackFuture( "Initializing debugger", initialized.future, ) @@ -313,7 +313,7 @@ private[debug] object DebugProxy { compilers: Compilers, workspace: AbsolutePath, stripColor: Boolean, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, sourceMapper: SourceMapper, compilations: Compilations, targets: Seq[BuildTargetIdentifier], @@ -340,7 +340,7 @@ private[debug] object DebugProxy { stackTraceAnalyzer, compilers, stripColor, - slowTaskProvider, + workDoneProgress, sourceMapper, compilations, targets, diff --git a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala index 8af463f12dd..c2cbab4ccd8 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala @@ -34,11 +34,11 @@ import scala.meta.internal.metals.ImportedBuild import scala.meta.internal.metals.MetalsBuildClient import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MetalsServerConfig -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.SocketConnection import scala.meta.internal.metals.Tables import scala.meta.internal.metals.TargetData import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.process.SystemProcess import scala.meta.io.AbsolutePath @@ -50,7 +50,7 @@ import coursier.core.Version class ScalaCli( compilers: () => Compilers, compilations: Compilations, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, buffers: Buffers, indexWorkspace: () => Future[Unit], diagnostics: () => Diagnostics, @@ -112,7 +112,7 @@ class ScalaCli( ifConnectedOrElse { st => compilers().cancel() - slowTaskProvider + workDoneProgress .trackFuture( "Importing Scala CLI sources", ImportedBuild.fromConnection(st.connection), diff --git a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala index e123413b9e5..97aa29620ef 100644 --- a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala @@ -30,10 +30,10 @@ import scala.meta.internal.metals.Messages import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable import scala.meta.internal.metals.ScalaVersionSelector -import scala.meta.internal.metals.SlowTask import scala.meta.internal.metals.Time import scala.meta.internal.metals.Timer import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.mtags.MD5 import scala.meta.internal.pc.CompilerJobQueue @@ -65,7 +65,7 @@ class WorksheetProvider( buildTargets: BuildTargets, languageClient: MetalsLanguageClient, userConfig: () => UserConfiguration, - slowTaskProvider: SlowTask, + workDoneProgress: WorkDoneProgress, diagnostics: Diagnostics, embedded: Embedded, publisher: WorksheetPublisher, @@ -250,7 +250,7 @@ class WorksheetProvider( ) } cancelables.add(Cancelable(() => completeEmptyResult())) - slowTaskProvider.trackFuture( + workDoneProgress.trackFuture( s"Evaluating ${path.filename}", result.asScala, ) @@ -338,13 +338,12 @@ class WorksheetProvider( val interruptThread = new Runnable { def run(): Unit = { if (!result.isDone()) { - //TODO: what about worksheet timeout ??? - val token = slowTaskProvider.startSlowTask( + val token = workDoneProgress.startProgress( s"Evaluating worksheet '${path.filename}'", onCancel = Some(cancellable.cancel), ) - result.asScala.onComplete(_ => slowTaskProvider.endSlowTask(token)) + result.asScala.onComplete(_ => workDoneProgress.endProgress(token)) } } } diff --git a/tests/slow/src/test/scala/tests/sbt/CancelCompileSuite.scala b/tests/slow/src/test/scala/tests/sbt/CancelCompileSuite.scala index c8763cdb5a9..932994d0d21 100644 --- a/tests/slow/src/test/scala/tests/sbt/CancelCompileSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/CancelCompileSuite.scala @@ -2,7 +2,6 @@ package tests import scala.meta.internal.metals.MetalsServerConfig import scala.meta.internal.metals.ServerCommands -import scala.meta.internal.metals.SlowTaskConfig import scala.meta.internal.metals.{BuildInfo => V} import ch.epfl.scala.bsp4j.StatusCode @@ -11,10 +10,7 @@ class CancelCompileSuite extends BaseLspSuite("compile-cancel", SbtServerInitializer) { override def serverConfig: MetalsServerConfig = - MetalsServerConfig.default.copy( - slowTask = SlowTaskConfig.on, - loglevel = "debug", - ) + MetalsServerConfig.default.copy(loglevel = "debug") test("basic") { cleanWorkspace() diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index 14493b0a061..26424a7ff06 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -261,7 +261,7 @@ class SbtBloopLspSuite test("cancel") { cleanWorkspace() - client.onBeginSlowTask = (name, cancelParams) => { + client.onWorkDoneProgressStart = (name, cancelParams) => { if (name == progressMessage) { Thread.sleep(TimeUnit.SECONDS.toMillis(2)) server.fullServer.didCancelWorkDoneProgress(cancelParams) @@ -279,7 +279,7 @@ class SbtBloopLspSuite expectError = true, ) _ = assertStatus(!_.isInstalled) - _ = client.onBeginSlowTask = (_, _) => {} + _ = client.onWorkDoneProgressStart = (_, _) => {} _ <- server.didSave("build.sbt")(_ + "\n// comment") _ = assertNoDiff(client.workspaceShowMessages, "") _ = assertStatus(!_.isInstalled) diff --git a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala index 0c076b27170..2a40cefda0f 100644 --- a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala +++ b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala @@ -8,9 +8,7 @@ import scala.meta.internal.metals.InitializationOptions import scala.meta.internal.metals.JsonParser._ import scala.meta.internal.metals.Messages import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.MetalsServerConfig import scala.meta.internal.metals.ServerCommands -import scala.meta.internal.metals.SlowTaskConfig import scala.meta.internal.metals.debug.TestDebugger import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.metals.{BuildInfo => V} @@ -19,8 +17,6 @@ import org.eclipse.{lsp4j => l} import tests.FileLayout class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) { - override def serverConfig: MetalsServerConfig = - MetalsServerConfig.default.copy(slowTask = SlowTaskConfig.on) override protected def initializationOptions: Option[InitializationOptions] = Some( diff --git a/tests/unit/src/main/scala/tests/BaseLspSuite.scala b/tests/unit/src/main/scala/tests/BaseLspSuite.scala index 7d0ebf591dd..33596d34f2a 100644 --- a/tests/unit/src/main/scala/tests/BaseLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseLspSuite.scala @@ -17,7 +17,6 @@ import scala.meta.internal.metals.InitializationOptions import scala.meta.internal.metals.MetalsServerConfig import scala.meta.internal.metals.MtagsResolver import scala.meta.internal.metals.RecursivelyDelete -import scala.meta.internal.metals.SlowTaskConfig import scala.meta.internal.metals.Time import scala.meta.internal.metals.UserConfiguration import scala.meta.internal.metals.logging.MetalsLogger @@ -38,8 +37,7 @@ abstract class BaseLspSuite( MetalsLogger.updateDefaultFormat() def icons: Icons = Icons.default def userConfig: UserConfiguration = UserConfiguration() - def serverConfig: MetalsServerConfig = - MetalsServerConfig.default.copy(slowTask = SlowTaskConfig.on) + def serverConfig: MetalsServerConfig = MetalsServerConfig.default def time: Time = Time.system implicit val ex: ExecutionContextExecutorService = ExecutionContext.fromExecutorService(Executors.newCachedThreadPool()) diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index d88a8794eb2..3425e1e756d 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -260,7 +260,7 @@ abstract class BaseWorksheetLspSuite( test("cancel") { cleanWorkspace() val cancelled = Promise[Unit]() - client.onBeginSlowTask = { (message, cancelParams) => + client.onWorkDoneProgressStart = { (message, cancelParams) => if (message.startsWith("Evaluating worksheet")) { cancelled.trySuccess(()) server.fullServer.didCancelWorkDoneProgress(cancelParams) diff --git a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala index c9f3d5fe5ba..c930cd19b5c 100644 --- a/tests/unit/src/main/scala/tests/TestScala3Compiler.scala +++ b/tests/unit/src/main/scala/tests/TestScala3Compiler.scala @@ -7,7 +7,7 @@ import scala.meta.internal.metals.Buffers import scala.meta.internal.metals.Embedded import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MtagsBinaries -import scala.meta.internal.metals.SlowTask +import scala.meta.internal.metals.WorkDoneProgress import scala.meta.internal.metals.{BuildInfo => V} import scala.meta.pc.PresentationCompiler @@ -24,7 +24,7 @@ object TestScala3Compiler { case Some(mtags: MtagsBinaries.Artifacts) => val time = new FakeTime val client = new TestingClient(PathIO.workingDirectory, Buffers()) - val status = new SlowTask(client, time)(ec) + val status = new WorkDoneProgress(client, time)(ec) val embedded = new Embedded(status) val pc = embedded .presentationCompiler(mtags, mtags.jars) diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index dc5d522528f..307ef6e7b27 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -94,7 +94,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) var resetWorkspace = new MessageActionItem(ResetWorkspace.cancel) var regenerateAndRestartScalaCliBuildSever = FileOutOfScalaCliBspScope.ignore var shouldReloadAfterJavaHomeUpdate = ProjectJavaHomeUpdate.notNow - var onBeginSlowTask: (String, WorkDoneProgressCancelParams) => Unit = + var onWorkDoneProgressStart: (String, WorkDoneProgressCancelParams) => Unit = (_, _) => {} val resources = new ResourceOperations(buffers) val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] = @@ -434,7 +434,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) params.getValue().getLeft() match { case begin: WorkDoneProgressBegin => val cancelParams = new WorkDoneProgressCancelParams(params.getToken()) - onBeginSlowTask(begin.getTitle(), cancelParams) + onWorkDoneProgressStart(begin.getTitle(), cancelParams) case _ => } } diff --git a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala index 79cdbb49950..897ccbf7385 100644 --- a/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala +++ b/tests/unit/src/test/scala/tests/debug/DebugProtocolCancelationSuite.scala @@ -20,7 +20,7 @@ class DebugProtocolCancelationSuite test("start") { cleanWorkspace() - client.onBeginSlowTask = (message, cancelParams) => { + client.onWorkDoneProgressStart = (message, cancelParams) => { if (message == "Starting debug server") { server.fullServer.didCancelWorkDoneProgress(cancelParams) } From f7d0206f4d3e1453a2523ecc58c3dbb9ea7db770 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 15 Mar 2024 09:58:58 +0100 Subject: [PATCH 08/12] review fixes --- .../src/main/scala/scala/meta/internal/builds/ShellRunner.scala | 2 ++ .../scala/scala/meta/internal/metals/WorkDoneProgress.scala | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index 612244fb180..da87b120603 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -113,6 +113,8 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit commandRun, processFuture, onCancel = Some(() => { + if (logInfo) + scribe.info(s"user cancelled $commandRun") result.trySuccess(ExitCodes.Cancel) ps.cancel }), diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala index 910dab204eb..03510d7813b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala @@ -10,6 +10,7 @@ import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.WorkDoneProgress.Token import org.eclipse.lsp4j.ProgressParams import org.eclipse.lsp4j.WorkDoneProgressBegin @@ -25,7 +26,6 @@ class WorkDoneProgress( time: Time, )(implicit ec: ExecutionContext) extends Cancelable { - type Token = messages.Either[String, Integer] case class Task( onCancel: Option[() => Unit], showTimer: Boolean, From 5c03559bb767a57492e2150c01721c56f129705d Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 2 Apr 2024 17:40:55 +0200 Subject: [PATCH 09/12] review fixes [skip ci] --- .../internal/metals/WorkDoneProgress.scala | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala index 03510d7813b..0fd7392a6d8 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala @@ -118,32 +118,28 @@ class WorkDoneProgress( task.maybeProgress match { case Some(progress) => progress.update(percentage) - val report = new WorkDoneProgressReport() - report.setPercentage(progress.percentage) - task.additionalMessage.foreach(report.setMessage) - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - report - ) - client.notifyProgress(new ProgressParams(token, notification)) + notifyProgress(token, task) case None => } } def notifyProgress(token: Token): Unit = { val task = taskMap.getOrDefault(token, Task.empty) - if (task.showTimer) { - val report = new WorkDoneProgressReport() - task.maybeProgress.foreach { progress => - report.setPercentage(progress.percentage) - } - task.additionalMessage.foreach(report.setMessage) - val notification = - messages.Either.forLeft[WorkDoneProgressNotification, Object]( - report - ) - client.notifyProgress(new ProgressParams(token, notification)) - } else Future.successful(()) + if (task.showTimer) notifyProgress(token, task) + else Future.successful(()) + } + + private def notifyProgress(token: Token, task: Task): Unit = { + val report = new WorkDoneProgressReport() + task.maybeProgress.foreach { progress => + report.setPercentage(progress.percentage) + } + task.additionalMessage.foreach(report.setMessage) + val notification = + messages.Either.forLeft[WorkDoneProgressNotification, Object]( + report + ) + client.notifyProgress(new ProgressParams(token, notification)) } def endProgress(token: Future[Token]): Future[Unit] = From cc718417a85a4f08402ea62bc432545583f62c97 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 17 Apr 2024 12:06:49 +0200 Subject: [PATCH 10/12] delete `noop` logic for `build/taskStart` and similar --- .../metals/ForwardingMetalsBuildClient.scala | 38 ++++++------------- .../internal/metals/WorkDoneProgress.scala | 4 +- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala index 9ac28d0a70a..e79e2ec2d92 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ForwardingMetalsBuildClient.scala @@ -70,20 +70,19 @@ final class ForwardingMetalsBuildClient( } private class Compilation( val timer: Timer, - val isNoOp: Boolean, - token: Option[Future[WorkDoneProgress.Token]], + token: Future[WorkDoneProgress.Token], taskProgress: TaskProgress = TaskProgress.empty, ) extends TreeViewCompilation { def progressPercentage = taskProgress.percentage - def end(): Unit = token.foreach(workDoneProgress.endProgress(_)) + def end(): Unit = workDoneProgress.endProgress(token) def updateProgress(progress: Long, total: Long = 100): Unit = { val prev = taskProgress.percentage taskProgress.update(progress, total) if (prev != taskProgress.percentage) { - token.foreach(workDoneProgress.notifyProgress(_, progressPercentage)) + workDoneProgress.notifyProgress(token, progressPercentage) } } } @@ -179,18 +178,12 @@ final class ForwardingMetalsBuildClient( compilations.remove(target).foreach(_.end()) val name = info.getDisplayName - val isNoOp = - params.getMessage != null && params.getMessage.startsWith( - "Start no-op compilation" - ) val token = - Option.when(!isNoOp) { - workDoneProgress.startProgress( - s"Compiling $name", - withProgress = true, - ) - } - val compilation = new Compilation(new Timer(time), isNoOp, token) + workDoneProgress.startProgress( + s"Compiling $name", + withProgress = true, + ) + val compilation = new Compilation(new Timer(time), token) compilations(task.getTarget) = compilation } case _ => @@ -223,23 +216,14 @@ final class ForwardingMetalsBuildClient( if (isSuccess) clientConfig.icons.check else clientConfig.icons.alert val message = s"${icon}Compiled $name (${compilation.timer})" - if (!compilation.isNoOp) { - scribe.info(s"time: compiled $name in ${compilation.timer}") - } + scribe.info(s"time: compiled $name in ${compilation.timer}") if (isSuccess) { if (hasReportedError.contains(target)) { // Only report success compilation if it fixes a previous compile error. statusBar.addMessage(message) } - if (!compilation.isNoOp || !updatedTreeViews.contains(target)) { - // By default, skip `onBuildTargetDidCompile` notifications on no-op - // compilations to reduce noisy traffic to the client. However, we - // send the notification if it's the first successful compilation of - // that target to fix - // https://github.com/scalameta/metals/issues/846. - updatedTreeViews.add(target) - onBuildTargetDidCompile(target) - } + updatedTreeViews.add(target) + onBuildTargetDidCompile(target) hasReportedError.remove(target) } else { hasReportedError.add(target) diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala index 0fd7392a6d8..4a5ba307bdd 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkDoneProgress.scala @@ -152,7 +152,7 @@ class WorkDoneProgress( client.notifyProgress(new ProgressParams(token, params)) } .recover { case _: NullPointerException => - // no such value in map + // no such value in the task map, task already ended or cancelled } def trackFuture[T]( @@ -182,7 +182,7 @@ class WorkDoneProgress( task.onCancel.foreach(_()) } catch { case _: NullPointerException => - // no such value in map + // no such value in the task map, task already ended or cancelled } override def cancel(): Unit = { From 41e1e05ddae3ab209264cab0249918d1e617bda2 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 19 Apr 2024 13:10:04 +0200 Subject: [PATCH 11/12] test: `WorksheetLspSuite.cancel` fix --- tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index 3425e1e756d..d145bb16e99 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -279,6 +279,7 @@ abstract class BaseWorksheetLspSuite( ) _ <- server.didOpen("a/src/main/scala/Main.worksheet.sc") _ <- cancelled.future + _ = client.onWorkDoneProgressStart = (_, _) => {} _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")( _.replace("Stream", "// Stream") ) From 66c6d05e07b432c7932d738f0bb473c172747a27 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 19 Apr 2024 13:49:53 +0200 Subject: [PATCH 12/12] fix after rebase --- .../src/main/scala/scala/meta/internal/metals/Compilers.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala index c5af1155490..4a25ff48c77 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala @@ -975,7 +975,7 @@ class Compilers( .computeIfAbsent( PresentationCompilerKey.JavaBuildTarget(targetId), { _ => - statusBar.trackBlockingTask( + workDoneProgress.trackBlocking( s"${config.icons.sync}Loading presentation compiler" ) { JavaLazyCompiler(targetId, search)