improvement: add do not show bsp errors option (#5678)

scalameta · Oct 4, 2023 · e3b108a · e3b108a
1 parent 5bad90e
commit e3b108a
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 18 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala b/metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala
@@ -12,6 +12,7 @@ import scala.meta.internal.metals.ClientCommands
 import scala.meta.internal.metals.ConcurrentHashSet
 import scala.meta.internal.metals.Directories
 import scala.meta.internal.metals.MetalsEnrichments._
+import scala.meta.internal.metals.Tables
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.io.AbsolutePath
 
@@ -20,8 +21,8 @@ import org.eclipse.{lsp4j => l}
 class BspErrorHandler(
     languageClient: MetalsLanguageClient,
     workspaceFolder: AbsolutePath,
-    restartBspServer: () => Future[Boolean],
     currentSession: () => Option[BspSession],
+    tables: Tables,
 )(implicit context: ExecutionContext) {
 
   protected def logsPath: AbsolutePath =
@@ -30,7 +31,11 @@ class BspErrorHandler(
   private val dismissedErrors = ConcurrentHashSet.empty[String]
 
   def onError(message: String): Future[Unit] = {
-    if (shouldShowBspError && !dismissedErrors.contains(message)) {
+    if (
+      !tables.dismissedNotifications.BspErrors.isDismissed &&
+      shouldShowBspError &&
+      !dismissedErrors.contains(message)
+    ) {
       val previousError = lastError.getAndSet(message)
       if (message != previousError) {
         showError(message)
@@ -58,10 +63,12 @@ class BspErrorHandler(
             .flatMap(findLine(_))
             .getOrElse(0)
         Future.successful(gotoLogs(errorMsgStartLine))
-      case BspErrorHandler.restartBuildServer =>
-        restartBspServer().ignoreValue
       case BspErrorHandler.dismiss =>
         Future.successful(dismissedErrors.add(message)).ignoreValue
+      case BspErrorHandler.doNotShowErrors =>
+        Future.successful {
+          tables.dismissedNotifications.BspErrors.dismissForever
+        }.ignoreValue
       case _ => Future.successful(())
     }
   }
@@ -103,12 +110,12 @@ object BspErrorHandler {
       if (message.length() <= MESSAGE_MAX_LENGTH) {
         (
           makeShortMessage(message),
-          List(restartBuildServer, dismiss),
+          List(dismiss, doNotShowErrors),
         )
       } else {
         (
           makeLongMessage(message),
-          List(goToLogs, restartBuildServer, dismiss),
+          List(goToLogs, dismiss, doNotShowErrors),
         )
       }
     val params = new l.ShowMessageRequestParams()
@@ -128,8 +135,7 @@ object BspErrorHandler {
 
   val goToLogs = new l.MessageActionItem("Go to logs.")
   val dismiss = new l.MessageActionItem("Dismiss.")
-  val restartBuildServer =
-    new l.MessageActionItem("Restart build server.")
+  val doNotShowErrors = new l.MessageActionItem("Stop showing bsp errors.")
 
   val errorHeader = "Encountered an error in the build server:"
   private val gotoLogsToSeeFull = "Go to logs to see the full error"

diff --git a/metals/src/main/scala/scala/meta/internal/metals/DismissedNotifications.scala b/metals/src/main/scala/scala/meta/internal/metals/DismissedNotifications.scala
@@ -22,6 +22,7 @@ final class DismissedNotifications(conn: () => Connection, time: Time) {
   val UpdateBloopJson = new Notification(12)
   val ReconnectScalaCli = new Notification(13)
   val ScalaCliImportAuto = new Notification(14)
+  val BspErrors = new Notification(15)
 
   val all: List[Notification] = List(
     Only212Navigation,
@@ -38,6 +39,7 @@ final class DismissedNotifications(conn: () => Connection, time: Time) {
     UpdateBloopJson,
     ReconnectScalaCli,
     ScalaCliImportAuto,
+    BspErrors,
   )
 
   def resetAll(): Unit = {

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -369,8 +369,8 @@ class MetalsLspService(
     new BspErrorHandler(
       languageClient,
       folder,
-      restartBspServer,
       () => bspSession,
+      tables,
     )
 
   private val buildClient: ForwardingMetalsBuildClient =

diff --git a/tests/unit/src/test/scala/tests/BspErrorHandlerSuite.scala b/tests/unit/src/test/scala/tests/BspErrorHandlerSuite.scala
@@ -1,28 +1,24 @@
 package tests
 
-import java.nio.file.Paths
 import java.util.concurrent.Executors
 
 import scala.concurrent.ExecutionContext
-import scala.concurrent.Future
 
 import scala.meta.internal.builds.BspErrorHandler
 import scala.meta.internal.metals.Buffers
 import scala.meta.internal.metals.Directories
 import scala.meta.internal.metals.MetalsEnrichments._
+import scala.meta.internal.metals.Tables
 import scala.meta.io.AbsolutePath
 
 import com.google.gson.JsonObject
+import org.eclipse.lsp4j.MessageActionItem
 
-class BspErrorHandlerSuite extends BaseSuite {
+class BspErrorHandlerSuite extends BaseTablesSuite {
   implicit val ec: ExecutionContext =
     ExecutionContext.fromExecutorService(Executors.newCachedThreadPool())
 
-  val workspace: AbsolutePath = AbsolutePath(Paths.get("."))
-    .resolve("target/bsp-error-suite/")
-    .createDirectories()
   val client = new TestingClient(workspace, Buffers())
-  val errorHandler = new TestBspErrorHandler(client, workspace)
   val exampleError1 = "an error"
   val exampleError2 = "a different error"
   val longError: String =
@@ -34,14 +30,16 @@ class BspErrorHandlerSuite extends BaseSuite {
        |""".stripMargin
 
   test("handle-bsp-error") {
+    val errorHandler = new TestBspErrorHandler(client, workspace, tables)
+
     FileLayout.fromString(
       s"""|/.metals/metals.log
           |
           |""".stripMargin,
       workspace,
     )
 
-    client.bspError = BspErrorHandler.restartBuildServer
+    client.bspError = new MessageActionItem("OK")
 
     for {
       _ <- errorHandler.onError(exampleError1)
@@ -52,13 +50,18 @@ class BspErrorHandlerSuite extends BaseSuite {
       _ = client.bspError = BspErrorHandler.goToLogs
       _ <- errorHandler.onError(longError)
       _ <- errorHandler.onError(exampleError1)
+      _ = client.bspError = BspErrorHandler.doNotShowErrors
+      _ <- errorHandler.onError(exampleError2)
+      _ <- errorHandler.onError(longError)
+      _ <- errorHandler.onError(exampleError2)
     } yield {
       assertNoDiff(
         client.workspaceMessageRequests,
         s"""|${BspErrorHandler.makeShortMessage(exampleError1)}
             |${BspErrorHandler.makeShortMessage(exampleError2)}
             |${BspErrorHandler.makeShortMessage(exampleError1)}
             |${BspErrorHandler.makeLongMessage(longError)}
+            |${BspErrorHandler.makeShortMessage(exampleError2)}
             |""".stripMargin,
       )
       assert(client.clientCommands.asScala.nonEmpty)
@@ -92,12 +95,13 @@ class BspErrorHandlerSuite extends BaseSuite {
 class TestBspErrorHandler(
     val languageClient: TestingClient,
     workspaceFolder: AbsolutePath,
+    tables: Tables,
 )(implicit context: ExecutionContext)
     extends BspErrorHandler(
       languageClient,
       workspaceFolder,
-      () => Future.successful(true),
       () => None,
+      tables,
     ) {
   override def shouldShowBspError: Boolean = true