Skip to content

Commit

Permalink
Replace slowTask with LSP progress (#6144)
Browse files Browse the repository at this point in the history
* use slow task instead of status bar

* delete metals slow task

* fix cancel tests

* add timer

* format

* test fixes

* rename `slowTask` to `workDoneProgress`

* review fixes

* review fixes [skip ci]

* delete `noop` logic for `build/taskStart` and similar

* test: `WorksheetLspSuite.cancel` fix

* fix after rebase
  • Loading branch information
kasiaMarek authored Apr 19, 2024
1 parent 4507e7a commit dfa2bfe
Show file tree
Hide file tree
Showing 53 changed files with 479 additions and 807 deletions.
2 changes: 0 additions & 2 deletions docs/editors/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
63 changes: 1 addition & 62 deletions docs/integrations/new-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
9 changes: 1 addition & 8 deletions metals-bench/src/main/scala/bench/PcBenchmark.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.WorkDoneProgress(NoopLanguageClient, metals.Time.system)
)

protected final val benchmarkedScalaVersions: Array[String] = Array(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import scala.meta.internal.metals.MetalsEnrichments._
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
Expand All @@ -36,6 +37,7 @@ class BspConnector(
tables: Tables,
userConfig: () => UserConfiguration,
statusBar: StatusBar,
workDoneProgress: WorkDoneProgress,
bspConfigGenerator: BspConfigGenerator,
currentConnection: () => Option[BuildServerConnection],
restartBspServer: () => Future[Boolean],
Expand Down Expand Up @@ -129,8 +131,8 @@ class BspConnector(
if (shouldReload) connection.workspaceReload()
else Future.successful(())
} yield connection
statusBar
.trackFuture("Connecting to sbt", connectionF, showTimer = true)
workDoneProgress
.trackFuture("Connecting to sbt", connectionF)
.map(Some(_))
case ResolvedBspOne(details) =>
tables.buildServers.chooseServer(details.getName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.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
Expand All @@ -26,7 +26,7 @@ import coursierapi._

class NewProjectProvider(
client: MetalsLanguageClient,
statusBar: StatusBar,
workDoneProgress: WorkDoneProgress,
config: ClientConfiguration,
shell: ShellRunner,
icons: Icons,
Expand All @@ -46,7 +46,7 @@ class NewProjectProvider(
if (allTemplates.nonEmpty) {
allTemplates
} else {
statusBar.trackBlockingTask(
workDoneProgress.trackBlocking(
"Fetching template information from Github"
) {
// Matches:
Expand Down
43 changes: 12 additions & 31 deletions metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +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.StatusBar
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.metals.WorkDoneProgress
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,
statusBar: StatusBar,
)(implicit
class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit
executionContext: scala.concurrent.ExecutionContext
) extends Cancelable {

Expand Down Expand Up @@ -110,40 +104,27 @@ 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) {
val newCancelable: Cancelable = () => ps.cancel
cancelables.add(newCancelable)

val processFuture = ps.complete
workDoneProvider.trackFuture(
commandRun,
processFuture,
onCancel = Some(() => {
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 processFuture = ps.complete
statusBar.trackFuture(
s"Running '$commandRun'",
processFuture,
}),
)
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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 hasWorkDoneProgressCapability(): Boolean =
(for {
params <- initializeParams
capabilities <- Option(params.getCapabilities())
window <- Option(capabilities.getWindow())
workDoneProgress <- Option(window.getWorkDoneProgress())
} yield workDoneProgress.booleanValue()).getOrElse(false)

def isExecuteClientCommandProvider(): Boolean =
extract(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
) {
Expand All @@ -46,7 +45,6 @@ object ClientExperimentalCapabilities {
None,
None,
None,
None,
)

def from(
Expand Down Expand Up @@ -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"),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class Compilers(
buffers: Buffers,
search: SymbolSearch,
embedded: Embedded,
statusBar: StatusBar,
workDoneProgress: WorkDoneProgress,
sh: ScheduledExecutorService,
initializeParams: InitializeParams,
excludedPackages: () => ExcludedPackagesHandler,
Expand Down Expand Up @@ -938,7 +938,7 @@ class Compilers(
} yield {
jworksheetsCache.put(
path,
statusBar.trackBlockingTask(
workDoneProgress.trackBlocking(
s"${config.icons.sync}Loading worksheet presentation compiler"
) {
ScalaLazyCompiler.forWorksheet(
Expand Down Expand Up @@ -975,7 +975,7 @@ class Compilers(
.computeIfAbsent(
PresentationCompilerKey.JavaBuildTarget(targetId),
{ _ =>
statusBar.trackBlockingTask(
workDoneProgress.trackBlocking(
s"${config.icons.sync}Loading presentation compiler"
) {
JavaLazyCompiler(targetId, search)
Expand All @@ -1001,7 +1001,7 @@ class Compilers(
val out = jcache.computeIfAbsent(
PresentationCompilerKey.ScalaBuildTarget(scalaTarget.info.getId),
{ _ =>
statusBar.trackBlockingTask(
workDoneProgress.trackBlocking(
s"${config.icons.sync}Loading presentation compiler"
) {
ScalaLazyCompiler(scalaTarget, mtags, search)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import mdoc.interfaces.Mdoc
* - mdoc
*/
final class Embedded(
statusBar: StatusBar
workDoneProgress: WorkDoneProgress
) extends Cancelable {

private val mdocs: TrieMap[String, URLClassLoader] =
Expand All @@ -49,7 +49,7 @@ final class Embedded(
if (isScala3) Some(scalaVersion) else None
val classloader = mdocs.getOrElseUpdate(
scalaVersionKey,
statusBar.trackSlowTask("Preparing worksheets") {
workDoneProgress.trackBlocking("Preparing worksheets") {
newMdocClassLoader(scalaBinaryVersion, resolveSpecificVersionCompiler)
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ final class FormattingProvider(
client: MetalsLanguageClient,
clientConfig: ClientConfiguration,
statusBar: StatusBar,
workDoneProgress: WorkDoneProgress,
icons: Icons,
tables: Tables,
buildTargets: BuildTargets,
Expand Down Expand Up @@ -503,7 +504,7 @@ final class FormattingProvider(
def downloadOutputStream(): OutputStream = {
downloadingScalafmt.trySuccess(())
downloadingScalafmt = Promise()
statusBar.trackSlowFuture(
workDoneProgress.trackFuture(
"Loading Scalafmt",
downloadingScalafmt.future,
)
Expand Down
Loading

0 comments on commit dfa2bfe

Please sign in to comment.