From 53af3a8528146638a29b53d9123ee8d5368bdf82 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 1 Sep 2023 19:24:08 +0200 Subject: [PATCH] bugfix: Correctly choose the folder to use for a command This was actually my bad, since I noticed it while working on the migration to Scala 3, where this was actually reported as a compilation error. Anyway, I added a test and did a small refactor so that it's easier to keep things consistent (such as expected title etc.) Fixes https://github.com/scalameta/metals/issues/5611 --- .../meta/internal/metals/ServerCommands.scala | 2 +- .../metals/WorkspaceChoicePopup.scala | 46 +++++++++++++------ .../internal/metals/WorkspaceLspService.scala | 31 ++++++++----- .../src/main/scala/tests/TestingClient.scala | 11 +++++ .../scala/tests/WorkspaceFoldersSuite.scala | 10 ++++ 5 files changed, 73 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala index 241681401ac..9991dca16c7 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala @@ -38,7 +38,7 @@ object ServerCommands { val DisconnectBuildServer = new Command( "build-disconnect", - "Disconnect to build server", + "Disconnect from old build server", """Unconditionally cancel existing build server connection without reconnecting""", ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceChoicePopup.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceChoicePopup.scala index 741f04ccbdb..4a6d5ccdd96 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceChoicePopup.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceChoicePopup.scala @@ -17,27 +17,43 @@ class WorkspaceChoicePopup( def interactiveChooseFolder( actionName: String )(implicit ec: ExecutionContext): Future[Option[MetalsLspService]] = { - def choicesParams(): ShowMessageRequestParams = { - val params = new ShowMessageRequestParams() - params.setMessage( - s"For which folder would you like to $actionName?" - ) - params.setType(MessageType.Info) - params.setActions( - folders() - .map(folder => new MessageActionItem(folder.getVisibleName)) - .asJava - ) - params - } val currentFolders = folders() if (currentFolders.length == 1) Future.successful(currentFolders.headOption) else { languageClient - .showMessageRequest(choicesParams()) + .showMessageRequest( + WorkspaceChoicePopup + .choicesParams(actionName, currentFolders.map(_.getVisibleName)) + ) .asScala - .map { item => currentFolders.find(_.getVisibleName == item) } + .map { item => + currentFolders.find(_.getVisibleName == item.getTitle()) + } } } } + +object WorkspaceChoicePopup { + def choicesParams( + actionName: String, + folders: List[String], + ): ShowMessageRequestParams = { + val params = new ShowMessageRequestParams() + + val lowerCaseActionName = + if (actionName.nonEmpty) actionName.head.toLower + actionName.tail + else actionName + + params.setMessage( + s"For which folder would you like to $lowerCaseActionName?" + ) + params.setType(MessageType.Info) + params.setActions( + folders + .map(folder => new MessageActionItem(folder)) + .asJava + ) + params + } +} 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 01ea1b04100..1492575b202 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -642,29 +642,29 @@ class WorkspaceLspService( case ServerCommands.RestartBuildServer() => onCurrentFolder( _.restartBspServer().ignoreValue, - "restart BSP server", + ServerCommands.RestartBuildServer.title, ).asJavaObject case ServerCommands.GenerateBspConfig() => onCurrentFolder( _.generateBspConfig(), - "generate BSP config", + ServerCommands.GenerateBspConfig.title, ).asJavaObject case ServerCommands.ImportBuild() => onCurrentFolder( _.slowConnectToBuildServer(forceImport = true), - "import build", + ServerCommands.ImportBuild.title, default = () => BuildChange.None, ).asJavaObject case ServerCommands.ConnectBuildServer() => onCurrentFolder( _.quickConnectToBuildServer(), - "connect build server", + ServerCommands.ConnectBuildServer.title, default = () => BuildChange.None, ).asJavaObject case ServerCommands.DisconnectBuildServer() => onCurrentFolder( _.disconnectOldBuildServer(), - "disconnect old build server", + ServerCommands.DisconnectBuildServer.title, ).asJavaObject case ServerCommands.DecodeFile(uri) => getServiceFor(uri).decodeFile(uri).asJavaObject @@ -734,7 +734,10 @@ class WorkspaceLspService( .asJava }.asJavaObject case ServerCommands.BspSwitch() => - onCurrentFolder(_.switchBspServer(), "switch BSP server").asJavaObject + onCurrentFolder( + _.switchBspServer(), + ServerCommands.BspSwitch.title, + ).asJavaObject case ServerCommands.OpenIssue() => Future .successful(Urls.openBrowser(githubNewIssueUrlCreator.buildUrl())) @@ -742,9 +745,15 @@ class WorkspaceLspService( case OpenBrowserCommand(url) => Future.successful(Urls.openBrowser(url)).asJavaObject case ServerCommands.CascadeCompile() => - onCurrentFolder(_.cascadeCompile(), "cascade compile").asJavaObject + onCurrentFolder( + _.cascadeCompile(), + ServerCommands.CascadeCompile.title, + ).asJavaObject case ServerCommands.CleanCompile() => - onCurrentFolder(_.cleanCompile(), "clean compile").asJavaObject + onCurrentFolder( + _.cleanCompile(), + ServerCommands.CleanCompile.title, + ).asJavaObject case ServerCommands.CancelCompile() => foreachSeq(_.cancelCompile(), ignoreValue = true) case ServerCommands.PresentationCompilerRestart() => @@ -799,7 +808,7 @@ class WorkspaceLspService( ) ) }, - "go to log", + ServerCommands.GotoLog.title, ).asJavaObject case ServerCommands.StartDebugAdapter(params) if params.getData != null => @@ -901,14 +910,14 @@ class WorkspaceLspService( ) onCurrentFolder( _.interactivePopupChoiceReset(), - "interactive reset choice", + ServerCommands.ResetChoicePopup.title, ) }).asJavaObject case ServerCommands.ResetNotifications() => onCurrentFolder( _.resetNotifications(), - "reset dismissed notifications", + ServerCommands.ResetNotifications.title, ).asJavaObject case ServerCommands.NewScalaFile(args) => diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 41b8c8eeb6f..6bcd2a72bf1 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -20,8 +20,10 @@ import scala.meta.internal.metals.ClientCommands import scala.meta.internal.metals.FileOutOfScalaCliBspScope import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.ServerCommands import scala.meta.internal.metals.ServerLivenessMonitor 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 @@ -322,6 +324,13 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) ) } + def choicesMessage = WorkspaceChoicePopup + .choicesParams( + ServerCommands.ConnectBuildServer.title.toLowerCase(), + Nil, + ) + .getMessage() + CompletableFuture.completedFuture { messageRequests.addLast(params.getMessage) showMessageRequestHandler(params).getOrElse { @@ -360,6 +369,8 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) ) ) { regenerateAndRestartScalaCliBuildSever + } else if (params.getMessage() == choicesMessage) { + params.getActions().asScala.head } else { throw new IllegalArgumentException(params.toString) } diff --git a/tests/unit/src/test/scala/tests/WorkspaceFoldersSuite.scala b/tests/unit/src/test/scala/tests/WorkspaceFoldersSuite.scala index 9d9f9c564b2..e0fa1a5aa09 100644 --- a/tests/unit/src/test/scala/tests/WorkspaceFoldersSuite.scala +++ b/tests/unit/src/test/scala/tests/WorkspaceFoldersSuite.scala @@ -1,5 +1,6 @@ package tests +import scala.meta.internal.metals.ServerCommands import scala.meta.internal.metals.{BuildInfo => V} class WorkspaceFoldersSuite @@ -32,6 +33,15 @@ class WorkspaceFoldersSuite expectError = false, ) _ = assert(server.fullServer.folderServices.length == 2) + _ <- server.executeCommand(ServerCommands.ConnectBuildServer) + _ = assertNoDiff( + server.client.workspaceMessageRequests, + "For which folder would you like to connect to build server?", + ) + _ = assertNoDiff( + server.client.workspaceShowMessages, + "", + ) _ = assertNoDiff( server.workspaceSymbol("MyObject"), s"""|a.MyObjectA