From 65101dca42033eb786e4fb28627509170168e8ec Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 17 Dec 2024 17:15:04 +0100 Subject: [PATCH] chore: Fixes after code review --- .../main/scala/docs/WorksheetModifier.scala | 132 ++++++++---------- .../metals/InlayHintResolveProvider.scala | 2 +- .../internal/metals/MetalsLspService.scala | 7 +- .../worksheets/WorksheetProvider.scala | 18 ++- .../scala/tests/BaseWorksheetLspSuite.scala | 5 +- 5 files changed, 79 insertions(+), 85 deletions(-) diff --git a/metals-docs/src/main/scala/docs/WorksheetModifier.scala b/metals-docs/src/main/scala/docs/WorksheetModifier.scala index 62651257787..dc718ec069c 100644 --- a/metals-docs/src/main/scala/docs/WorksheetModifier.scala +++ b/metals-docs/src/main/scala/docs/WorksheetModifier.scala @@ -14,76 +14,68 @@ class WorksheetModifier extends StringModifier { reporter: Reporter, ): String = { - val (howYouSeeEvaluations, howToHover) = info match { - case "vscode" => - "as a inlay hint at the end of the line." -> - "hover on the inlay hint to expand it." - case _ => - "as a comment as the end of the line." -> "hover on the comment to expand." - } - - s"""|## Worksheets - | - |Worksheets are a great way to explore an api, try out an idea, or code - |up an example and quickly see the evaluated expression or result. Behind - |the scenes worksheets are powered by the great work done in - |[mdoc](https://scalameta.org/mdoc/). - | - |### Getting started with Worksheets - | - |To get started with a worksheet you can either use the `metals.new-scala-file` - |command and select *Worksheet* or create a file called `*.worksheet.sc`. - |This format is important since this is what tells Metals that it's meant to be - |treated as a worksheet and not just a Scala script. Where you create the - |script also matters. If you'd like to use classes and values from your - |project, you need to make sure the worksheet is created inside of your sources next to any existing Scala files. - |directory. You can still create a worksheet in other places, but you will - |only have access to the standard library and your dependencies. - | - |### Evaluations - | - |After saving you'll see the result of the expression ${howYouSeeEvaluations} - |You may not see the full result for example if it's too long, so you are also - |able to ${howToHover} - | - |Keep in mind that you don't need to wrap your code in an `object`. In worksheets - |everything can be evaluated at the top level. - | - |### Using dependencies in worksheets - | - |You are able to include an external dependency in your worksheet by including - |it in one of the following two ways. - | - |```scala - |// $$dep.`organisation`::artifact:version` style - |import $$dep.`com.lihaoyi::scalatags:0.7.0` - | - |// $$ivy.`organisation::artifact:version` style - |import $$ivy.`com.lihaoyi::scalatags:0.7.0` - |``` - | - |`::` is the same as `%%` in sbt, which will append the current Scala binary version - |to the artifact name. - | - |You can also import `scalac` options in a special `$$scalac` import like below: - | - |```scala - |import $$scalac.`-Ywarn-unused` - |``` - | - |### Troubleshooting - | - |Since worksheets are not standard Scala files, you may run into issues with some constructs. - |For example, you may see an error like this: - | - |``` - |value classes may not be a member of another class - mdoc - |``` - | - |This means that one of the classes defined in the worksheet extends AnyVal, which is - |not currently supported. You can work around this by moving the class to a separate file or removing - |the AnyVal parent. - |""".stripMargin + """|## Worksheets + | + |Worksheets are a great way to explore an api, try out an idea, or code + |up an example and quickly see the evaluated expression or result. Behind + |the scenes worksheets are powered by the great work done in + |[mdoc](https://scalameta.org/mdoc/). + | + |### Getting started with Worksheets + | + |To get started with a worksheet you can either use the `metals.new-scala-file` + |command and select *Worksheet* or create a file called `*.worksheet.sc`. + |This format is important since this is what tells Metals that it's meant to be + |treated as a worksheet and not just a Scala script. Where you create the + |script also matters. If you'd like to use classes and values from your + |project, you need to make sure the worksheet is created inside of your sources next to any existing Scala files. + |directory. You can still create a worksheet in other places, but you will + |only have access to the standard library and your dependencies. + | + |### Evaluations + | + |After saving you'll see the result of the expression as a inlay hint at the end of the line. + |You may not see the full result for example if it's too long, so you are also + |able to hover on the inlay hint to expand it. + | + |Keep in mind that you don't need to wrap your code in an `object`. In worksheets + |everything can be evaluated at the top level. + | + |### Using dependencies in worksheets + | + |You are able to include an external dependency in your worksheet by including + |it in one of the following two ways. + | + |```scala + |// $$dep.`organisation`::artifact:version` style + |import $$dep.`com.lihaoyi::scalatags:0.7.0` + | + |// $$ivy.`organisation::artifact:version` style + |import $$ivy.`com.lihaoyi::scalatags:0.7.0` + |``` + | + |`::` is the same as `%%` in sbt, which will append the current Scala binary version + |to the artifact name. + | + |You can also import `scalac` options in a special `$$scalac` import like below: + | + |```scala + |import $$scalac.`-Ywarn-unused` + |``` + | + |### Troubleshooting + | + |Since worksheets are not standard Scala files, you may run into issues with some constructs. + |For example, you may see an error like this: + | + |``` + |value classes may not be a member of another class - mdoc + |``` + | + |This means that one of the classes defined in the worksheet extends AnyVal, which is + |not currently supported. You can work around this by moving the class to a separate file or removing + |the AnyVal parent. + |""".stripMargin } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/InlayHintResolveProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/InlayHintResolveProvider.scala index a51521ba136..a388afbff6c 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/InlayHintResolveProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/InlayHintResolveProvider.scala @@ -18,7 +18,7 @@ final class InlayHintResolveProvider( definitionProvider: DefinitionProvider, compilers: Compilers, )(implicit ec: ExecutionContextExecutorService, rc: ReportContext) { - + def resolve( inlayHint: InlayHint, token: CancelToken, 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 8cd4adffed0..6278d01ee96 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -981,10 +981,7 @@ abstract class MetalsLspService( CancelTokens.future { token => compilers .hover(params, token) - .map(_.map(_.toLsp())) - .map( - _.orNull - ) + .map(_.map(_.toLsp()).orNull) } } @@ -999,7 +996,7 @@ abstract class MetalsLspService( compilers.inlayHints(params, token) else Future.successful(List.empty[l.InlayHint].asJava) worksheet <- worksheetProvider.inlayHints( - params.getTextDocument().getUri().toAbsolutePath, + params.getTextDocument().getUri().toAbsolutePathSafe, token, ) } yield (hints.asScala ++ worksheet).asJava 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 f9b8cced0fc..505e4bf7ab4 100644 --- a/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/worksheets/WorksheetProvider.scala @@ -174,6 +174,16 @@ class WorksheetProvider( ) } + def inlayHints( + path: Option[AbsolutePath], + token: CancelToken, + ): Future[List[InlayHint]] = { + path match { + case Some(path) => inlayHints(path, token) + case None => Future.successful(Nil) + } + } + def inlayHints( path: AbsolutePath, token: CancelToken, @@ -212,11 +222,9 @@ class WorksheetProvider( .map { stat => val statEnd = stat.position().toLsp.getEnd() // Update positions so that they don't break - distance.toRevised(statEnd) match { - case Left(_) => - case Right(right) => - statEnd.setLine(right.startLine) - statEnd.setCharacter(right.startColumn) + distance.toRevised(statEnd).foreach { right => + statEnd.setLine(right.startLine) + statEnd.setCharacter(right.startColumn) } val hint = new InlayHint( statEnd, diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index 40aeb98c73f..36e09d3d015 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -208,7 +208,6 @@ abstract class BaseWorksheetLspSuite( cleanWorkspace() val cancelled = Promise[Unit]() client.onWorkDoneProgressStart = { (message, cancelParams) => - scribe.info(message) if (message.startsWith("Evaluating worksheet")) { cancelled.trySuccess(()) server.fullServer.didCancelWorkDoneProgress(cancelParams) @@ -227,9 +226,7 @@ abstract class BaseWorksheetLspSuite( ) _ <- server.didOpen("a/src/main/scala/Main.worksheet.sc") _ <- cancelled.future.withTimeout(10.seconds) - _ = client.onWorkDoneProgressStart = (message, _) => { - scribe.info(message) - } + _ = client.onWorkDoneProgressStart = (_, _) => {} _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")( _.replace("Stream", "// Stream") )