Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement: adds details to github issue #4966

Merged
merged 1 commit into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package scala.meta.internal.metals

import java.net.URLEncoder

import scala.util.Properties

import scala.meta.internal.bsp.BspResolvedResult
import scala.meta.internal.bsp.BspSession
import scala.meta.internal.bsp.ResolvedBloop
import scala.meta.internal.bsp.ResolvedBspOne
import scala.meta.internal.bsp.ResolvedMultiple
import scala.meta.internal.bsp.ResolvedNone
import scala.meta.internal.builds.BuildTools

import org.eclipse.lsp4j.ClientInfo

class GithubNewIssueUrlCreator(
tables: Tables,
buildTargets: BuildTargets,
currentBuildServer: () => Option[BspSession],
calculateNewBuildServer: () => BspResolvedResult,
clientInfo: ClientInfo,
buildTools: BuildTools,
) {

def buildUrl(): String = {
val scalaVersions =
buildTargets.allScala.map(_.scalaVersion).toSet.mkString("; ")
val clientVersion =
Option(clientInfo.getVersion()).map(v => s" v$v").getOrElse("")
val body =
s"""|<!--
| Describe the bug ...
|
| Reproduction steps
| 1. Go to ...
| 2. Click on ...
| 3. Scroll down to ...
| 4. See error
|-->
|
|### Expected behaviour:
|
|<!-- A clear and concise description of what you expected to happen. -->
|
|**Operating system:**
|${Properties.osName}
|
|**Java version:**
|${Properties.javaVersion}
|
|**Editor/extension:**
|${clientInfo.getName()}$clientVersion
|
|**Metals version:**
|${BuildInfo.metalsVersion}
|
|### Extra context or search terms:
|<!--
| - Any other context about the problem
| - Search terms to help others discover this
|-->
|
|### Workspace information:
|
| - **Scala versions:** $scalaVersions$selectedBuildTool$selectedBuildServer
| - **All build tools in workspace:** ${buildTools.all.mkString("; ")}
|""".stripMargin
s"https://github.com/scalameta/metals/issues/new?body=${URLEncoder.encode(body)}"
}

private def selectedBuildTool(): String = {
tables.buildTool
.selectedBuildTool()
.map { value =>
s"""|
| - **Build tool:** ${value}""".stripMargin
}
.getOrElse("")
}

private def selectedBuildServer(): String = {
val buildServer = currentBuildServer()
.map(s => s"${s.main.name} v${s.main.version}")
.getOrElse {
calculateNewBuildServer() match {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is not an actual data point here, since if you need to calcualte it then it's not connected. Maybe you could make the name soemthing like Disconnected: Bloop etc. ?

case ResolvedBloop => "Disconnected: Bloop"
case ResolvedBspOne(details) => s"Disconnected: ${details.getName()}"
case ResolvedMultiple(_, details) =>
s"Disconnected: Multiple Found ${details.map(_.getName()).mkString("; ")}"
case ResolvedNone => s"Disconnected: None Found"
}
}

s"""|
| - **Build server:** $buildServer""".stripMargin
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,15 @@ class MetalsLspService(
maybeJdkVersion,
)

private val githubNewIssueUrlCreator = new GithubNewIssueUrlCreator(
tables,
buildTargets,
() => bspSession,
() => bspConnector.resolve(),
initializeParams.getClientInfo(),
buildTools,
)

private val fileDecoderProvider: FileDecoderProvider =
new FileDecoderProvider(
workspace,
Expand Down Expand Up @@ -1871,6 +1880,10 @@ class MetalsLspService(
else Future.successful(())
}
} yield ()).asJavaObject
case ServerCommands.OpenIssue() =>
Future
.successful(Urls.openBrowser(githubNewIssueUrlCreator.buildUrl()))
.asJavaObject
case OpenBrowserCommand(url) =>
Future.successful(Urls.openBrowser(url)).asJavaObject
case ServerCommands.CascadeCompile() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,16 @@ object ServerCommands {
"Open the Metals logs to troubleshoot issues.",
)

val OpenIssue = new OpenBrowserCommand(
"https://github.com/scalameta/metals/issues/new/choose",
"Open issue on GitHub",
"Open the Metals repository on GitHub to ask a question, report a bug or request a new feature.",
val OpenIssue = new Command(
"open-new-github-issue",
kasiaMarek marked this conversation as resolved.
Show resolved Hide resolved
"Open an issue on GitHub",
"Open the Metals repository on GitHub to ask a question or report a bug.",
)

val OpenFeatureRequest = new OpenBrowserCommand(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might need to add it to the list of commands at the bottom of the file here, since they need to be registered at the start for the client to pick them up

Copy link
Contributor Author

@kasiaMarek kasiaMarek Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, those are only visible in the tree view. But we do want to make it possible to add it to the command pallet. Is that what you mean?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ach, actually my bad. This is done via EchoCommand, but maybe it's good to let it work in Metals comamnds.

"https://github.com/scalameta/metals-feature-requests/issues/new?template=feature-request.yml",
"Open a feature request",
"Open the Metals repository on GitHub to open a feature request.",
)

val MetalsGithub = new OpenBrowserCommand(
Expand Down Expand Up @@ -700,6 +706,8 @@ object ServerCommands {
SuperMethodHierarchy,
StartScalaCliServer,
StopScalaCliServer,
OpenIssue,
OpenFeatureRequest,
)

val allIds: Set[String] = all.map(_.id).toSet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ class MetalsTreeViewProvider(
echoCommand(ServerCommands.ReadBloopDocumentation, "book"),
echoCommand(ServerCommands.ChatOnDiscord, "discord"),
echoCommand(ServerCommands.OpenIssue, "issue-opened"),
echoCommand(ServerCommands.OpenFeatureRequest, "github"),
echoCommand(ServerCommands.MetalsGithub, "github"),
echoCommand(ServerCommands.BloopGithub, "github"),
echoCommand(ServerCommands.ScalametaTwitter, "twitter"),
Expand Down