From c406ddc7f61416db94241a3b7721d2ef9d86e696 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 27 Oct 2023 15:45:15 +0200 Subject: [PATCH] review fixes --- .../internal/bsp/ConnectionBspStatus.scala | 62 ++++++++++--------- .../meta/internal/metals/doctor/Doctor.scala | 24 +++---- .../meta/internal/metals/ReportContext.scala | 7 ++- 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala b/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala index 772d7e3e2fa..c883ba87d2c 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala @@ -18,7 +18,7 @@ class ConnectionBspStatus( icons: Icons, )(implicit rc: ReportContext) { private val status = new AtomicReference[BspStatusState]( - BspStatusState(Disconnected, None, None) + BspStatusState(Disconnected, None, None, shouldShow = false) ) private val currentSessionErrors = new AtomicReference[Set[String]](Set()) @@ -41,9 +41,8 @@ class ConnectionBspStatus( } def onReportsUpdate(): Unit = { - val currentState = status.get() - currentState.currentState match { - case error @ ErrorMessage(_) => showState(error, currentState) + status.get().currentState match { + case ErrorMessage(_) => showState(status.get()) case _ => } } @@ -59,22 +58,21 @@ class ConnectionBspStatus( newState: BspServerState, errorReports: Set[String] = currentSessionErrors.get(), ) = { - val old = status.getAndUpdate(_.changeState(newState)._2) - val (newServerState, newBspState) = old.changeState(newState) - newServerState.foreach(showState(_, newBspState, errorReports)) + val newServerState = status.updateAndGet(_.changeState(newState)) + if (newServerState.shouldShow) { + showState(newServerState, errorReports) + } } private def showState( - state: BspServerState, - currState: BspStatusState, + statusState: BspStatusState, errorReports: Set[String] = currentSessionErrors.get(), ) = { - def serverName = currState.serverName.getOrElse("bsp") val showParams = - state match { + statusState.currentState match { case Disconnected => ConnectionBspStatus.disconnectedParams case NoResponse => - ConnectionBspStatus.noResponseParams(serverName, icons) + ConnectionBspStatus.noResponseParams(statusState.serverName, icons) case Connected(serverName) => ConnectionBspStatus.connectedParams(serverName, icons) case ErrorMessage(message) => @@ -82,10 +80,10 @@ class ConnectionBspStatus( rc.bloop.getReports().map(_.toPath.toUri().toString()).toSet ) if (currentSessionReports.isEmpty) - ConnectionBspStatus.connectedParams(serverName, icons) + ConnectionBspStatus.connectedParams(statusState.serverName, icons) else ConnectionBspStatus.bspErrorParams( - serverName, + statusState.serverName, icons, message, currentSessionReports.size, @@ -143,21 +141,25 @@ case object NoResponse extends BspServerState case class BspStatusState( currentState: BspServerState, - previousState: Option[BspServerState], - serverName: Option[String], + lastError: Option[ErrorMessage], + optServerName: Option[String], + shouldShow: Boolean, ) { + val serverName: String = optServerName.getOrElse("bsp") def changeState( newState: BspServerState - ): (Option[BspServerState], BspStatusState) = { + ): BspStatusState = { newState match { case Disconnected if currentState != Disconnected => moveTo(Disconnected) case NoResponse if currentState != NoResponse => moveTo(NoResponse) case ErrorMessage(msg) => currentState match { case NoResponse => - ( - None, - BspStatusState(NoResponse, Some(ErrorMessage(msg)), serverName), + BspStatusState( + NoResponse, + Some(ErrorMessage(msg)), + optServerName, + shouldShow = false, ) case _ => moveTo(ErrorMessage(msg)) } @@ -165,27 +167,27 @@ case class BspStatusState( currentState match { case Disconnected => moveTo(Connected(serverName)) case NoResponse => - previousState match { - case Some(ErrorMessage(msg)) => moveTo(ErrorMessage(msg)) + lastError match { + case Some(error) => moveTo(error) case _ => moveTo(Connected(serverName)) } - case _ => (None, this) + case _ => this.copy(shouldShow = false) } - case _ => (None, this) + case _ => this.copy(shouldShow = false) } } def moveTo( newState: BspServerState - ): (Some[BspServerState], BspStatusState) = { + ): BspStatusState = { val newServerName = newState match { case Connected(serverName) => Some(serverName) - case _ => serverName + case _ => optServerName } - ( - Some(newState), - BspStatusState(newState, Some(currentState), newServerName), - ) + val lastError = Some(currentState).collect { case error: ErrorMessage => + error + } + BspStatusState(newState, lastError, newServerName, shouldShow = true) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index 604ca8e60d1..99d9a21d5e0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -409,23 +409,11 @@ final class Doctor( case (v1 -> _, v2 -> _) => v1.orderingNumber < v2.orderingNumber } .foreach { case (group, reports) => - def id = - group match { - case Bloop => "bloop" - case BuildTarget(name) => name - case Other => "other" - } - def name = - group match { - case Bloop => "Bloop error reports" - case BuildTarget(name) => name - case Other => "Other error reports" - } html.element("details")(details => details - .element("summary", s"id=reports-$id}")( + .element("summary", s"id=reports-${group.id}")( _.element("b")( - _.text(s"$name (${reports.length}):") + _.text(s"${group.name} (${reports.length}):") ) ) .element("table") { table => @@ -683,15 +671,23 @@ object Doctor { sealed trait ErrorReportsGroup { def orderingNumber: Int + def id: String + def name: String } + case object Bloop extends ErrorReportsGroup { def orderingNumber = 1 + def id: String = "bloop" + def name: String = "Bloop error reports" } case class BuildTarget(name: String) extends ErrorReportsGroup { def orderingNumber = 2 + def id: String = name } case object Other extends ErrorReportsGroup { def orderingNumber = 3 + def id: String = "other" + def name: String = "Other error reports" } object ErrorReportsGroup { diff --git a/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala b/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala index 8a2af5866fe..e1e6370060f 100644 --- a/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala +++ b/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala @@ -104,7 +104,7 @@ class StdReporter( private val reported = new AtomicReference(Map[String, Path]()) def readInIds(): Unit = { - reported.updateAndGet(_ ++ getReports().flatMap { report => + val reports = getReports().flatMap { report => val lines = Files.readAllLines(report.file.toPath()) if (lines.size() > 0) { lines.get(0) match { @@ -113,7 +113,8 @@ class StdReporter( case _ => None } } else None - }.toMap) + }.toMap + reported.updateAndGet(_ ++ reports) } override def create( @@ -134,7 +135,7 @@ class StdReporter( reportedMap = reported.getAndUpdate(map => if (map.contains(id)) map else map + (id -> path) ) - duplicate <- reportedMap.get(sanitizedId.get) + duplicate <- reportedMap.get(id) } yield duplicate optDuplicate.orElse {