Add menu command for "Open Primer documentation for variable"#43
Add menu command for "Open Primer documentation for variable"#43siddharthkp merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the "Go to definition" feature with a dedicated menu command for opening Primer documentation. The change moves documentation-opening logic from the language server's definition provider to a standalone VS Code command, providing a more explicit and appropriate way to access variable documentation.
Key changes:
- Added a new command
primer-primitives-autocomplete.openDocswith context menu integration - Removed the definition provider from the language server
- Updated utility functions to support both VSCode and language server TextDocument types
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds command and context menu configuration for the new "Open Primer documentation for variable" command |
| src/index.ts | Implements the new command handler with variable detection and webview panel creation |
| src/language-server.ts | Removes the definition provider capability and associated handler |
| src/utils/get-current-word.ts | Updates type signature to accept both VSCode and language server TextDocument types |
| src/utils/get-css-variable.ts | Updates type signature to accept both VSCode and language server TextDocument types |
| README.md | Updates documentation to reflect the new command name and workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| client.onRequest('open-docs', ({variable, openPanelIfClosed = true}) => { | ||
| if (!panel && openPanelIfClosed) { | ||
| commands.registerCommand('primer-primitives-autocomplete.openDocs', async () => { |
There was a problem hiding this comment.
The command registration is not being added to context.subscriptions, which could lead to a resource leak. The command should be disposed when the extension is deactivated.
Add the registration to subscriptions:
context.subscriptions.push(
commands.registerCommand('primer-primitives-autocomplete.openDocs', async () => {
// ... command implementation
})
)| const variableName = getCssVariable(document, offset) | ||
| if (!variableName) { | ||
| const currentWord = getCurrentWord(document, offset) | ||
| window.showInformationMessage(`Unrecognized variable: ${currentWord}. Cannot open Primer documentation.`) | ||
| return null | ||
| } | ||
|
|
||
| const variableInfo = getVariableInfo(variableName) | ||
| if (!variableInfo) { | ||
| const currentWord = getCurrentWord(document, offset) |
There was a problem hiding this comment.
The error message duplicates the call to getCurrentWord() when both getCssVariable() and getVariableInfo() fail. This is inefficient since getCurrentWord() was already called on line 72 as part of getCssVariable().
Consider storing the result once and reusing it:
const variableName = getCssVariable(document, offset)
if (!variableName) {
const currentWord = getCurrentWord(document, offset)
window.showInformationMessage(`Unrecognized variable: ${currentWord}. Cannot open Primer documentation.`)
return null
}
const variableInfo = getVariableInfo(variableName)
if (!variableInfo) {
window.showInformationMessage(`Unrecognized variable: ${variableName}. Cannot open Primer documentation.`)
return null
}| const variableName = getCssVariable(document, offset) | |
| if (!variableName) { | |
| const currentWord = getCurrentWord(document, offset) | |
| window.showInformationMessage(`Unrecognized variable: ${currentWord}. Cannot open Primer documentation.`) | |
| return null | |
| } | |
| const variableInfo = getVariableInfo(variableName) | |
| if (!variableInfo) { | |
| const currentWord = getCurrentWord(document, offset) | |
| const currentWord = getCurrentWord(document, offset) | |
| const variableName = getCssVariable(document, offset) | |
| if (!variableName) { | |
| window.showInformationMessage(`Unrecognized variable: ${currentWord}. Cannot open Primer documentation.`) | |
| return null | |
| } | |
| const variableInfo = getVariableInfo(variableName) | |
| if (!variableInfo) { |
| <a href=${variableInfo.docsUrl}> | ||
| Open in Browser | ||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16" fill="currentColor"> | ||
| <path d="M3.75 2h3.5a.75.75 0 0 1 0 1.5h-3.5a.25.25 0 0 0-.25.25v8.5c0 .138.112.25.25.25h8.5a.25.25 0 0 0 .25-.25v-3.5a.75.75 0 0 1 1.5 0v3.5A1.75 1.75 0 0 1 12.25 14h-8.5A1.75 1.75 0 0 1 2 12.25v-8.5C2 2.784 2.784 2 3.75 2Zm6.854-1h4.146a.25.25 0 0 1 .25.25v4.146a.25.25 0 0 1-.427.177L13.03 4.03 9.28 7.78a.751.751 0 0 1-1.042-.018.751.751 0 0 1-.018-1.042l3.75-3.75-1.543-1.543A.25.25 0 0 1 10.604 1Z"></path> | ||
| </svg> | ||
| </a> | ||
|
|
||
| <iframe sandbox="allow-same-origin allow-scripts allow-popups allow-forms" src="${variable.docsUrl}"></iframe> | ||
| <iframe sandbox="allow-same-origin allow-scripts allow-popups allow-forms" src="${variableInfo.docsUrl}"></iframe> |
There was a problem hiding this comment.
The HTML template has missing quotes around attribute values, which could cause XSS vulnerabilities or incorrect rendering if the URLs contain special characters.
Add quotes around the href and src attributes:
panel.webview.html = `
<style>
body {
padding: 0;
}
a {
padding: 16px;
text-decoration: none;
display: flex;
gap: 4px;
}
iframe {
width: calc(100% + 20px);
height: 100vh;
border: none;
}
</style>
<a href="${variableInfo.docsUrl}">
Open in Browser
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16" fill="currentColor">
<path d="M3.75 2h3.5a.75.75 0 0 1 0 1.5h-3.5a.25.25 0 0 0-.25.25v8.5c0 .138.112.25.25.25h8.5a.25.25 0 0 0 .25-.25v-3.5a.75.75 0 0 1 1.5 0v3.5A1.75 1.75 0 0 1 12.25 14h-8.5A1.75 1.75 0 0 1 2 12.25v-8.5C2 2.784 2.784 2 3.75 2Zm6.854-1h4.146a.25.25 0 0 1 .25.25v4.146a.25.25 0 0 1-.427.177L13.03 4.03 9.28 7.78a.751.751 0 0 1-1.042-.018.751.751 0 0 1-.018-1.042l3.75-3.75-1.543-1.543A.25.25 0 0 1 10.604 1Z"></path>
</svg>
</a>
<iframe sandbox="allow-same-origin allow-scripts allow-popups allow-forms" src="${variableInfo.docsUrl}"></iframe>
`
We were using "Go to definition" as a way to open documentation. Replaced that with a dedicated command.
open.documentation.mov