-
Notifications
You must be signed in to change notification settings - Fork 84
Ensure the extension activates with a .bsp folder #1865
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?
Conversation
6d8aa91
to
9a64919
Compare
This can be verified with a project like https://github.com/spotify/sourcekit-bazel-bsp/tree/main/Example |
This can be verified with a project like https://github.com/spotify/sourcekit-bazel-bsp/tree/main/Example |
src/utilities/workspace.ts
Outdated
} | ||
|
||
await globDirectory(folder, { onlyDirectories: true }).then(async entries => { | ||
const skipFolders = new Set<string>([ |
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.
probably should make this list a setting to allow excluding others and maybe user may want to add one of these folder. I'm thinking maybe bazel could generate a package for them possibly...
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.
I agree, I'll make the change
73da576
to
38464ac
Compare
"bazel-out", | ||
"bazel-bin" | ||
], | ||
"markdownDescription": "A list of glob patterns to ignore when searching sub-folders for Swift Packages. The `swift.searchSubfoldersForPackages` must be `true` for this setting to have an effect. Always use forward-slashes in glob expressions regardless of platform. This is combined with VS Code's default `files.exclude` setting.", |
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.
Is this actually a glob pattern though? Seems like we just look at the basename of the folder and then skip if it matches something in this list?
Also wondering if the "default" could be removed from "This is combined with VS Code's default files.exclude
setting.", and just say "This is combined with VS Code's files.exclude
setting." to make it clearer?
A .bsp folder was considered a valid workspace folder, but the extension acitvation events in the package.json were not configured to search for it, meaning another valid file/folder for activation had to be present. If that was the case, then the .bsp folder would be discovered correctly, but not if it was the only file/folder in the folder that would activate the extension. Add it to the list of valid activation file types. Also clean up this code path a bit, ignoring common folders we shouldn't search for projects.
38464ac
to
e8f15fe
Compare
Description
A .bsp folder was considered a valid workspace folder, but the extension activation events in the package.json were not configured to search for it, meaning another valid file/folder for activation had to be present. If that was the case, then the .bsp folder would be discovered correctly, but not if it was the only file/folder in the folder that would activate the extension.
Add it to the list of valid activation file types. Also clean up this code path a bit, ignoring common folders we shouldn't search for projects.
Tasks
Required tests have been writtenDocumentation has been updatedAdded an entry to CHANGELOG.md if applicable