- 
                Notifications
    
You must be signed in to change notification settings  - Fork 13
 
Add configurable link to pkl doc in hover #104
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -626,3 +626,102 @@ fun Appendable.renderParameterList( | |
| 
     | 
||
| fun <T> List<T>.withReplaced(idx: Int, elem: T): List<T> = | ||
| toMutableList().apply { this[idx] = elem } | ||
| 
     | 
||
| fun PklNode.getDocumentationUrl(): String? { | ||
| val module = if (this is PklModule) this else enclosingModule ?: return null | ||
| val file = module.containingFile | ||
| 
     | 
||
| val packageDep = file.`package` | ||
| 
     | 
||
| val authorityUrl = | ||
| when { | ||
| packageDep == null && file.pklAuthority == Origin.STDLIB.name.lowercase() -> | ||
| DEFAULT_PKL_AUTHORITY | ||
| packageDep != null -> packageDep.packageUri.authority | ||
| else -> return null | ||
| } | ||
| 
     | 
||
| val packagePath = | ||
| when { | ||
| isInStdlib -> { | ||
| "pkl" | ||
| } | ||
| packageDep != null -> { | ||
| val packageUri = packageDep.packageUri | ||
| val authority = packageUri.authority | ||
| 
     | 
||
| authority + "/" + packageUri.path.substringBeforeLast('@').removePrefix("/") | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make   | 
||
| } | ||
| else -> { | ||
| return null | ||
| } | ||
| } | ||
| 
     | 
||
| val version = | ||
| when { | ||
| packageDep == null -> module.effectivePklVersion.toString() | ||
| else -> packageDep.packageUri.version.toString() | ||
| } | ||
| 
     | 
||
| val urlPattern = | ||
| module.project.clientOptions.packageDocumentationUrls[authorityUrl] ?: return null | ||
| 
     | 
||
| // Extract the relative module path within the package | ||
| val moduleFileUri = file.uri.toString() | ||
| val modulePath = | ||
| when { | ||
| // Handle jar: URLs from packages | ||
| isInStdlib -> { | ||
| module.moduleName?.removePrefix("pkl.") ?: return null | ||
| } | ||
| moduleFileUri.startsWith("jar:") -> { | ||
| // Extract the path after the zip file reference | ||
| val afterJar = moduleFileUri.substringAfter("!/") | ||
| afterJar.removeSuffix(".pkl") | ||
| } | ||
| else -> { | ||
| // Fallback: use the module name | ||
| module.moduleName?.removeSuffix(".pkl") ?: return null | ||
| } | ||
| } | ||
| 
     | 
||
| val docPath = | ||
| when { | ||
| // For modules, use index.html | ||
| this is PklModule -> "index.html" | ||
                
      
                  gordonbondon marked this conversation as resolved.
               
          
            Show resolved
            Hide resolved
         | 
||
| this is PklClass -> "$name.html" | ||
| 
     | 
||
| // For class members, use ClassName.html#memberName | ||
| else -> { | ||
| val enclosingClass = this.parentOfType<PklClass>() | ||
| if (enclosingClass != null) { | ||
| val className = enclosingClass.identifier?.text ?: return null | ||
| val fragmentId = | ||
| when (this) { | ||
| is PklProperty -> name | ||
| is PklMethod -> methodHeader.identifier?.text?.let { "$it()" } | ||
| is PklMethodHeader -> identifier?.text?.let { "$it()" } | ||
| else -> null | ||
| } | ||
| if (fragmentId != null) "$className.html#$fragmentId" else "$className.html" | ||
| } else { | ||
| // For top-level members, use index.html#memberName | ||
| val fragmentId = | ||
| when (this) { | ||
| is PklProperty -> name | ||
| is PklMethod -> methodHeader.identifier?.text?.let { "$it()" } | ||
| is PklMethodHeader -> identifier?.text?.let { "$it()" } | ||
| is PklTypeAlias -> identifier?.text | ||
| else -> null | ||
| } | ||
| if (fragmentId != null) "index.html#$fragmentId" else "index.html" | ||
| } | ||
| } | ||
| } | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all of these, we should also encode the path (you can copy the algorithm here: https://github.com/apple/pkl/blob/63f89fb679a021fddb0ac340d2822a74c1e661b7/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java#L805) Here's the SPICE that describes the algorithm: https://github.com/apple/pkl-evolution/blob/main/spices/SPICE-0003-windows-safe-paths.adoc  | 
||
| 
     | 
||
| return urlPattern | ||
| .replace("{packagePath}", packagePath.removePrefix("/")) | ||
| .replace("{version}", version) | ||
| .replace("{modulePath}", modulePath) | ||
| .replace("{path}", docPath) | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above re: it doesn't really make sense to have placeholders  | 
||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -229,6 +229,34 @@ private fun showDocCommentAndModule(node: PklNode?, text: String, context: PklPr | |
| appendLine() | ||
| appendLine() | ||
| append("in [${module.moduleName}](${module.getLocationUri(forDocs = true)})") | ||
| 
     | 
||
| val docUrl = node?.getDocumentationUrl() | ||
| if (docUrl != null) { | ||
| val domain = | ||
| try { | ||
| java.net.URI(docUrl).host | ||
| } catch (e: Exception) { | ||
| "documentation" | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be a misconfiguration error if this happens; seems like we should catch this at configuration time.  | 
||
| } | ||
| 
     | 
||
| val linkText = | ||
| when (node) { | ||
| is PklModule -> module.moduleName | ||
| is PklProperty -> node.name | ||
| is PklMethod -> node.methodHeader.identifier?.text?.let { "$it()" } ?: "method" | ||
| is PklMethodHeader -> node.identifier?.text?.let { "$it()" } ?: "method" | ||
| is PklClass -> node.identifier?.text ?: "class" | ||
| is PklTypeAlias -> node.identifier?.text ?: "type" | ||
| else -> module.moduleName | ||
| } | ||
| 
     | 
||
| appendLine() | ||
| appendLine() | ||
| append("---") | ||
| appendLine() | ||
| appendLine() | ||
| append("[`$linkText` on $domain]($docUrl)") | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -173,4 +173,104 @@ class HoverTest : LspTestBase() { | |
| val hoverText = getHoverText() | ||
| assertThat(hoverText).contains("foo: UInt8") | ||
| } | ||
| 
     | 
||
| @Test | ||
| fun `module with documentation URL`() { | ||
| createPklFile( | ||
| """ | ||
| import "package://pkg.pkl-lang.org/pkl-k8s/[email protected]#/k8sSchema.pkl" | ||
| 
     | 
||
| // Hover over the module reference | ||
| local schema = k8s<caret>Schema | ||
| """ | ||
| .trimIndent() | ||
| ) | ||
| val hoverText = getHoverText() | ||
| 
     | 
||
| assertThat(hoverText).contains("`k8s.k8sSchema` on pkl-lang.org") | ||
| assertThat(hoverText) | ||
| .contains( | ||
| "https://pkl-lang.org/package-docs/pkg.pkl-lang.org/pkl-k8s/k8s/1.2.1/k8sSchema/index.html" | ||
| ) | ||
| } | ||
| 
     | 
||
| @Test | ||
| fun `property with documentation URL`() { | ||
| createPklFile( | ||
| """ | ||
| amends "package://pkg.pkl-lang.org/pkl-k8s/[email protected]#/k8sSchema.pkl" | ||
| 
     | 
||
| // Hover over a property from the package module | ||
| local templates = module.resourceTem<caret>plates | ||
| """ | ||
| .trimIndent() | ||
| ) | ||
| val hoverText = getHoverText() | ||
| 
     | 
||
| assertThat(hoverText).contains("`resourceTemplates` on pkl-lang.org") | ||
| assertThat(hoverText) | ||
| .contains( | ||
| "https://pkl-lang.org/package-docs/pkg.pkl-lang.org/pkl-k8s/k8s/1.2.1/k8sSchema/index.html#resourceTemplates" | ||
| ) | ||
| } | ||
| 
     | 
||
| @Test | ||
| fun `class property with documentation URL`() { | ||
| createPklFile( | ||
| """ | ||
| import "package://pkg.pkl-lang.org/pkl-k8s/[email protected]#/api/apps/v1/Deployment.pkl" | ||
| 
     | 
||
| local deployment = new Deployment { | ||
| // Hover over a property inside the Deployment class | ||
| spec { | ||
| repli<caret>cas = 1 | ||
| } | ||
| } | ||
| """ | ||
| .trimIndent() | ||
| ) | ||
| val hoverText = getHoverText() | ||
| 
     | 
||
| assertThat(hoverText).contains("`replicas` on pkl-lang.org") | ||
| assertThat(hoverText) | ||
| .contains( | ||
| "https://pkl-lang.org/package-docs/pkg.pkl-lang.org/pkl-k8s/k8s/1.2.1/api/apps/v1/Deployment/DeploymentSpec.html#replicas" | ||
| ) | ||
| } | ||
| 
     | 
||
| @Test | ||
| fun `method with documentation URL`() { | ||
| createPklFile( | ||
| """ | ||
| import "package://pkg.pkl-lang.org/pkl-k8s/[email protected]#/K8sObject.pkl" | ||
| 
     | 
||
| local result = new K8sObject {}.hasUnique<caret>PortNames() | ||
| """ | ||
| .trimIndent() | ||
| ) | ||
| val hoverText = getHoverText() | ||
| 
     | 
||
| assertThat(hoverText).contains("`hasUniquePortNames()` on pkl-lang.org") | ||
| assertThat(hoverText) | ||
| .contains( | ||
| "https://pkl-lang.org/package-docs/pkg.pkl-lang.org/pkl-k8s/k8s/1.2.1/K8sObject/index.html#hasUniquePortNames()" | ||
| ) | ||
| } | ||
| 
     | 
||
| @Test | ||
| fun `sdtlib base documentation URL`() { | ||
| createPklFile( | ||
| """ | ||
| result: Str<caret>ing | ||
| """ | ||
| .trimIndent() | ||
| ) | ||
| val hoverText = getHoverText() | ||
| 
     | 
||
| assertThat(hoverText).contains("`String` on pkl-lang.org") | ||
| assertThat(hoverText) | ||
| .contains( | ||
| "https://pkl-lang.org/package-docs/pkl/${fakeProject.stdlib.version}/base/String.html" | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need users to provide any placeholders here. The relative path within a docsite can be derived by the module/class/method/property name.
It's always going to be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LSP doesn't really know where a package's documentation might exist. It might make sense to just accept a list of docsite URLs, where pkl-lsp tries each one to see if documentation exists. But, if we do this, we'd need some way to cache this so that we aren't hammering the documentation site needlessly.
Not all packages from
pkg.pkl-lang.orgexist inpkl-lang.org/package-docs. For example, packages published by users can be fetched from there, but we only publish package docs for the packages that we maintain ourselves.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we have a config with package/module prefixes mapped to doc site url prefix?
Something like:
Default will be:
Trying multiple urls and caching them for each hover seems like an overhead.
Looking at how other extensions handle this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future improvement, how about having a url shema for docs similar to source code schema in package description? https://pkl-lang.org/package-docs/pkl/current/Project/Package.html#sourceCodeUrlScheme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
godoc is a central docsite (all public go modules are available on godoc).
Currently, we don't have the concept of a central pkldoc site, but this is something that we've talked about hosting in the past.
If we have that, then we only need one setting for "this is where the docsite lives".
For the
pkg.pkl-lang.orgstuff, we kind of need a "negative match" on packages that start withpkg.pkl-lang.org/github.com, rather than a prefix match.The package metadata actually does have a field for documentation: https://pkl-lang.org/package-docs/pkl/current/Project/Package.html#documentation. This field will appear in the package metadata if set.
Perhaps, for now, we can just use that field in lieu of first checking a docsite.