From 5db0d6dd9dc5f193b004f0696901ad9fa649fcb0 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 10 Nov 2023 16:38:52 +0100 Subject: [PATCH] improvement: Set icons properly for all parts of tree view Previously, only some nodes would have their icons set, which would result with some having folder icons added by VS Code but not all. Now, we set icons for each node manually. The only problem I have is with packages, which doesn't really have a great icon anywhere. symbol-package is the same as symbol-object, which is too similar. So I decided to go with folder, which seems the next best thing. --- .../meta/internal/tvp/ClasspathTreeView.scala | 10 ++- .../internal/tvp/MetalsTreeViewProvider.scala | 56 +++++++------- .../test/scala/tests/TreeViewLspSuite.scala | 77 ++++++++++--------- 3 files changed, 75 insertions(+), 68 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala b/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala index 6a81132e02f..f94ea152f67 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala @@ -28,17 +28,19 @@ class ClasspathTreeView[Value, Key]( valueTooltip: Value => String, toplevels: () => Iterator[Value], loadSymbols: (Key, String) => Iterator[TreeViewSymbolInformation], + toplevelIcon: String, ) { val scheme: String = s"$schemeId-${folder.path.toString()}" val rootUri: String = scheme + ":" - def root(showFolderName: Boolean): TreeViewNode = { + def root(showFolderName: Boolean, icon: String): TreeViewNode = { val folderPart = if (showFolderName) s" (${folder.nameOrUri})" else "" TreeViewNode( viewId, rootUri, title + folderPart + s" (${toplevels().size})", collapseState = MetalsTreeItemCollapseState.collapsed, + icon = icon, ) } @@ -50,7 +52,7 @@ class ClasspathTreeView[Value, Key]( def children(uri: String): Array[TreeViewNode] = { if (uri == rootUri) { - TreeViewNode.sortAlphabetically(toplevels().map(toViewNode).toArray) + TreeViewNode.sortAlphabetically(toplevels().map(toplevelNode).toArray) } else { val node = fromUri(uri) @@ -119,6 +121,7 @@ class ClasspathTreeView[Value, Key]( case k.FIELD => "symbol-field" case k.TYPE_PARAMETER => "symbol-type-parameter" case k.TYPE => "symbol-type-parameter" + case k.PACKAGE => "symbol-folder" case _ => null } @@ -170,7 +173,7 @@ class ClasspathTreeView[Value, Key]( } } - def toViewNode(value: Value): TreeViewNode = { + def toplevelNode(value: Value): TreeViewNode = { val uri = toUri(id(value)).toUri TreeViewNode( viewId, @@ -178,6 +181,7 @@ class ClasspathTreeView[Value, Key]( valueTitle(value), tooltip = valueTooltip(value), collapseState = MetalsTreeItemCollapseState.collapsed, + icon = toplevelIcon, ) } diff --git a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala index 3fd12d4b333..d712d7ee380 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala @@ -260,15 +260,15 @@ class FolderTreeViewProvider( private val pendingProjectUpdates = ConcurrentHashSet.empty[BuildTargetIdentifier] val libraries = new ClasspathTreeView[AbsolutePath, AbsolutePath]( - definitionIndex, - TreeViewProvider.Project, - s"libraries", - s"Libraries", - folder, - identity, - _.toURI.toString(), - _.toAbsolutePath(followSymlink = false), - path => { + definitionIndex = definitionIndex, + viewId = TreeViewProvider.Project, + schemeId = s"libraries", + title = s"Libraries", + folder = folder, + id = identity, + encode = _.toURI.toString(), + decode = _.toAbsolutePath(followSymlink = false), + valueTitle = path => { if (path.filename == JdkSources.zipFileName) { maybeUsedJdkVersion .map(ver => s"jdk-${ver}-sources") @@ -276,31 +276,32 @@ class FolderTreeViewProvider( } else path.filename }, - _.toString, - () => buildTargets.allSourceJars, - (path, symbol) => { + valueTooltip = _.toString, + toplevels = () => buildTargets.allSourceJars, + loadSymbols = (path, symbol) => { val dialect = ScalaVersions.dialectForDependencyJar(path.filename) classpath.jarSymbols(path, symbol, dialect) }, + toplevelIcon = "package", ) val projects = new ClasspathTreeView[BuildTarget, BuildTargetIdentifier]( - definitionIndex, - TreeViewProvider.Project, - s"projects", - s"Projects", - folder, - _.getId(), - _.getUri(), - uri => new BuildTargetIdentifier(uri), - _.getName(), - _.baseDirectory, - { () => + definitionIndex = definitionIndex, + viewId = TreeViewProvider.Project, + schemeId = s"projects", + title = s"Projects", + folder = folder, + id = _.getId(), + encode = _.getUri(), + decode = uri => new BuildTargetIdentifier(uri), + valueTitle = _.getName(), + valueTooltip = _.baseDirectory, + toplevels = { () => buildTargets.all.filter(target => buildTargets.buildTargetSources(target.getId()).nonEmpty ) }, - { (id, symbol) => + loadSymbols = { (id, symbol) => val tops = for { scalaTarget <- buildTargets.scalaTarget(id).iterator source <- buildTargets.buildTargetSources(id) @@ -308,6 +309,7 @@ class FolderTreeViewProvider( } yield classpath.workspaceSymbols(source, symbol) tops.flatten }, + toplevelIcon = "target", ) def setVisible(viewId: String, visibility: Boolean): Unit = { @@ -325,7 +327,7 @@ class FolderTreeViewProvider( if (toUpdate.nonEmpty) { val nodes = toUpdate.map { target => projects - .toViewNode(target) + .toplevelNode(target) .copy(collapseState = MetalsTreeItemCollapseState.expanded) } Some(nodes) @@ -387,8 +389,8 @@ class FolderTreeViewProvider( nodeUri match { case None if buildTargets.all.nonEmpty => Array( - projects.root(showFolderName), - libraries.root(showFolderName), + projects.root(showFolderName, "project"), + libraries.root(showFolderName, "library"), ) case Some(uri) => if (libraries.matches(uri)) { diff --git a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala index ceb86022b0b..e13dd6e0dcf 100644 --- a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala +++ b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala @@ -60,7 +60,8 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { lazy val expectedLibrariesString: String = (this.expectedLibraries.toVector .map { (s: String) => - if (s != jdkSourcesName) s"${s}.jar -" else s"$jdkSourcesName -" + if (s != jdkSourcesName) s"${s}.jar package -" + else s"$jdkSourcesName package -" }) .mkString("\n") @@ -125,8 +126,8 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { ) _ = server.assertTreeViewChildren( s"projects-$folder:${server.buildTarget("a")}", - """|_empty_/ - - |a/ - + """|_empty_/ symbol-folder - + |a/ symbol-folder - |""".stripMargin, ) _ = server.assertTreeViewChildren( @@ -207,7 +208,7 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { _ = { server.assertTreeViewChildren( s"libraries-$folder:${server.jar("sourcecode")}", - "sourcecode/ +", + "sourcecode/ symbol-folder +", ) server.assertTreeViewChildren( s"libraries-$folder:", @@ -232,12 +233,12 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { ) server.assertTreeViewChildren( s"libraries-$folder:${server.jar("circe-core")}!/_root_/", - """|io/ + + """|io/ symbol-folder + |""".stripMargin, ) server.assertTreeViewChildren( s"libraries-$folder:${server.jar("cats-core")}!/_root_/", - """|cats/ + + """|cats/ symbol-folder + |""".stripMargin, ) server.assertTreeViewChildren( @@ -265,11 +266,11 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { "class Paths", ), s"""|root - | Libraries (22) - | $jdkSourcesName - | java/ - | nio/ - | file/ + | Libraries (22) library + | $jdkSourcesName package + | java/ symbol-folder + | nio/ symbol-folder + | file/ symbol-folder | Paths symbol-class |""".stripMargin, ) @@ -292,19 +293,19 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { "sourcecode/SourceContext.scala", "object File", isIgnored = { label => - label.endsWith(".jar") && + label.endsWith(".jar package") && !label.contains("sourcecode") }, ), s"""|root - | Projects (0) - | Libraries (22) - | Libraries (22) - | $jdkSourcesName - | sourcecode_2.13-0.1.7-sources.jar - | sourcecode_2.13-0.1.7-sources.jar - | sourcecode/ - | sourcecode/ + | Projects (0) project + | Libraries (22) library + | Libraries (22) library + | $jdkSourcesName package + | sourcecode_2.13-0.1.7-sources.jar package + | sourcecode_2.13-0.1.7-sources.jar package + | sourcecode/ symbol-folder + | sourcecode/ symbol-folder | Args symbol-class | Args symbol-object | ArgsMacros symbol-interface @@ -345,27 +346,27 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { "org/eclipse/lsp4j/services/LanguageClient.java", "registerCapability", isIgnored = { label => - label.endsWith(".jar") && + label.endsWith(".jar package") && !label.contains("lsp4j-0") }, ), s"""|root - | Projects (0) - | Libraries (${expectedLibrariesCount}) - | Libraries (${expectedLibrariesCount}) - | $jdkSourcesName - | org.eclipse.lsp4j-0.5.0-sources.jar - | org.eclipse.lsp4j-0.5.0-sources.jar - | org/ - | org/ - | eclipse/ - | eclipse/ - | lsp4j/ - | lsp4j/ - | adapters/ - | launch/ - | services/ - | util/ + | Projects (0) project + | Libraries (${expectedLibrariesCount}) library + | Libraries (${expectedLibrariesCount}) library + | $jdkSourcesName package + | org.eclipse.lsp4j-0.5.0-sources.jar package + | org.eclipse.lsp4j-0.5.0-sources.jar package + | org/ symbol-folder + | org/ symbol-folder + | eclipse/ symbol-folder + | eclipse/ symbol-folder + | lsp4j/ symbol-folder + | lsp4j/ symbol-folder + | adapters/ symbol-folder + | launch/ symbol-folder + | services/ symbol-folder + | util/ symbol-folder | ApplyWorkspaceEditParams symbol-class | ApplyWorkspaceEditResponse symbol-class | ClientCapabilities symbol-class @@ -529,7 +530,7 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { | WorkspaceFoldersOptions symbol-class | WorkspaceServerCapabilities symbol-class | WorkspaceSymbolParams symbol-class - | services/ + | services/ symbol-folder | LanguageClient symbol-interface | LanguageClientAware symbol-interface | LanguageClientExtensions symbol-interface