feat(config): Add an option to limit quick pick queries to the nearest package #458
Conversation
geoffreyheller
requested review from
cameron-martin,
coeuvre,
jfirebaugh and
vogelsgesang
as code owners
There was a problem hiding this comment.
Thank you for your contribution! I have left some comments in the code, some are simply my opinion and you can of course decide on whether you agree.
As for the feature itself: I feel like limiting to the current package only could be a little to strict for many users and that the mentioned project file would be the better way to go. Maybe: Would it already serve your usecase if the list of results is ordered in a way that shows the targets of the current package first and the rest later?
Note: I am not a maintainer, so consider this just as thoughts for discussion
There was a problem hiding this comment.
File added by mistake? Maybe you can directly add it to the .gitignore file?
| but you can opt into this by enabling the **Bazel: Buildifier Fix on Format** | ||
| setting. | ||
|
|
||
| ### Limiting quick pick queries to the nearest package |
There was a problem hiding this comment.
(opinionated) I feel like the separate output base is a configuration that will affect more users then the limiting of quick pick queries. Hence I would place the new config option below the existing one.
| export function getBazelPackageFolder(fsPath: string): string | undefined { | ||
| // These are the names of the files that mark the root of a repository | ||
| // or workspace. | ||
| const PACKAGE_FILE_NAMES = ["BUILD.bazel"]; |
There was a problem hiding this comment.
Both variants are valid:
| const PACKAGE_FILE_NAMES = ["BUILD.bazel"]; | |
| const PACKAGE_FILE_NAMES = ["BUILD.bazel", "BUILD"]; |
| * Search for the path to the directory that has the Bazel WORKSPACE file for | ||
| * the given file. |
There was a problem hiding this comment.
| * Search for the path to the directory that has the Bazel WORKSPACE file for | |
| * the given file. | |
| * Search for the path to the directory that has a file marking the root of a Bazel workspace |
| * If multiple directories along the path to the file have files called | ||
| * "WORKSPACE", the lowest path is returned. |
There was a problem hiding this comment.
| * If multiple directories along the path to the file have files called | |
| * "WORKSPACE", the lowest path is returned. | |
| * If multiple directories along the path to the file have matching files, the closest one is returned. |
| * undefined if the target file is not found. | ||
| */ | ||
| export function getBazelWorkspaceFolder(fsPath: string): string | undefined { | ||
| function getTargetFolderForFile( |
There was a problem hiding this comment.
The word target is already overloaded in the Bazel ecosystem. How about findAncestorDirectoryContaining or something similar that better reflects the intention?
| "bazel.limitQuickPickQueriesToNearestPackage": { | ||
| "type": "boolean", | ||
| "default": false, | ||
| "markdownDescription": "Limit quick pick queries to the nearest package." |
There was a problem hiding this comment.
Let's extend the description here a little bit, so that it is not the exact same as the heading. How about:
| "markdownDescription": "Limit quick pick queries to the nearest package." | |
| "markdownDescription": "If active, the selection dialog for targets or packages will limit the results to the current package." |
| getDefaultBazelExecutablePath(), | ||
| workspaceInfo.workspaceFolder.uri.fsPath, | ||
| ).queryPackages(query ?? "//..."); | ||
| getWorkingDirectory(workspaceInfo), |
There was a problem hiding this comment.
Is it actually expected to limit the results to just the current package for this queryQuickPickPackage function? This means that the result will always be of length 1
|
Hi @geoffreyheller could you have a look at |
2 participants
Adds an option to limit quick pick queries to the closest package associated with the file currently active in the text editor.
For large projects, this helps improve query performance and relevance by avoiding unnecessary traversal of unrelated packages. If no file is open, the extension will use the workspace root as the base package.
Set
bazel.limitQuickPickQueriesToNearestPackagetotrueto enable this feature.Related to #451