Add new command: 'Go to File in Selected Database'#4390
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new VS Code command to quickly open a file from the source archive of the currently selected CodeQL database, enabling easier selection-based result filtering workflows.
Changes:
- Introduces
searchSourceArchiveFilesto traverse the selected database’s source archive and present files via a Quick Pick. - Registers a new
codeQL.goToFilecommand and wires it into the local databases UI command set. - Exposes the command in
package.jsonso it appears in the Command Palette.
Show a summary per file
| File | Description |
|---|---|
| extensions/ql-vscode/src/databases/source-archive-file-search.ts | New Quick Pick–based file search/open implementation for source archives. |
| extensions/ql-vscode/src/databases/local-databases-ui.ts | Adds a new command handler to launch the source-archive file search for the current DB. |
| extensions/ql-vscode/src/common/commands.ts | Extends LocalDatabasesCommands type with the new command. |
| extensions/ql-vscode/package.json | Contributes the new command (and makes it visible in the Command Palette). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 4
| const quickPick = window.createQuickPick<SourceArchiveFileQuickPickItem>(); | ||
| quickPick.placeholder = "Go to File in Selected Database..."; | ||
| quickPick.matchOnDescription = true; | ||
| quickPick.busy = true; | ||
| quickPick.show(); | ||
|
|
||
| try { | ||
| const items = await collectFiles(explorerUri, sourceArchiveZipPath, ""); | ||
| // Sort items by file name, then by path |
There was a problem hiding this comment.
I believe this is correct, although it only matters in extremely niche situations.
The returned promise is ultimately only used by the command handlers, which doesn't matter for UI-triggered commands, but it can matter if the command is invoked programmatically through executeCommand.
The fix should be straightforward though, just move the Promise creation up before the show() call and store the promise in a variable.
| if (selected) { | ||
| const doc = await workspace.openTextDocument(selected.uri); | ||
| await window.showTextDocument(doc); | ||
| } | ||
| resolve(); |
There was a problem hiding this comment.
The suggestion here looks good to me.
| for (const [name, type] of entries) { | ||
| const childPath = prefix ? `${prefix}/${name}` : name; | ||
| const childUri = encodeSourceArchiveUri({ | ||
| sourceArchiveZipPath, | ||
| pathWithinSourceArchive: `${decodeSourceArchiveUri(dirUri).pathWithinSourceArchive}/${name}`, | ||
| }); | ||
|
|
| async function collectFiles( | ||
| dirUri: Uri, | ||
| sourceArchiveZipPath: string, | ||
| prefix: string, |
There was a problem hiding this comment.
| prefix: string, | |
| prefix: string, | |
| items: SourceArchiveFileQuickPickItem[] = [], |
I'd move items to an accumulator parameter to avoid the N^2 cost of building up intermediate lists
| prefix: string, | ||
| ): Promise<SourceArchiveFileQuickPickItem[]> { | ||
| const entries = await workspace.fs.readDirectory(dirUri); | ||
| const items: SourceArchiveFileQuickPickItem[] = []; |
There was a problem hiding this comment.
| const items: SourceArchiveFileQuickPickItem[] = []; |
| childPath, | ||
| ); | ||
| items.push(...subItems); |
There was a problem hiding this comment.
| childPath, | |
| ); | |
| items.push(...subItems); | |
| childPath, | |
| items, | |
| ); |
| const quickPick = window.createQuickPick<SourceArchiveFileQuickPickItem>(); | ||
| quickPick.placeholder = "Go to File in Selected Database..."; | ||
| quickPick.matchOnDescription = true; | ||
| quickPick.busy = true; | ||
| quickPick.show(); | ||
|
|
||
| try { | ||
| const items = await collectFiles(explorerUri, sourceArchiveZipPath, ""); | ||
| // Sort items by file name, then by path |
There was a problem hiding this comment.
I believe this is correct, although it only matters in extremely niche situations.
The returned promise is ultimately only used by the command handlers, which doesn't matter for UI-triggered commands, but it can matter if the command is invoked programmatically through executeCommand.
The fix should be straightforward though, just move the Promise creation up before the show() call and store the promise in a variable.
| if (selected) { | ||
| const doc = await workspace.openTextDocument(selected.uri); | ||
| await window.showTextDocument(doc); | ||
| } | ||
| resolve(); |
There was a problem hiding this comment.
The suggestion here looks good to me.
|
@copilot: please address all review comments. |
@asgerf recently introduced selection-based result filtering, which is a really great feature. However, getting to file in the source archive where you want to do the filtering currently requires running some query and then being able to interpret the result of that query to match the file where filtering is intended.
This PR adds a new command
CodeQL: Go to File in Selected Database, which works like VS Code's built-inGo to File, except it does so on the source archive of the selected database.