-
Notifications
You must be signed in to change notification settings - Fork 188
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
Add ignoredWorkspaceFolders setting #3617
base: main
Are you sure you want to change the base?
Conversation
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.
This applies to more than just extension packs and would affect everywhere that the extension tries to discover files in the workspace. So it's quite a big change, but not necessarily a bad once and I'm not opposed to it. It could be quite useful for certain cases when you have very large workspace folders that you want to ignore.
extensions/ql-vscode/package.json
Outdated
@@ -198,6 +198,11 @@ | |||
"maximum": 1024, | |||
"description": "Number of threads for running queries." | |||
}, | |||
"codeQL.runningQueries.ignoredFolders": { |
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 suggest we rename to codeQL.runningQueries.ignoredWorkspaceFolders
. It's a bit longer but it makes it clear just from the name that it only applies to workspace folders and not to arbitrary folders.
Thanks @robertbrignull, I updated with the suggested name and included a line in the changelog. Let me know if you have any other feedback. Regarding this line in the "Checklist", I'm not sure what needs to be done:
|
Thanks for adding a changelog entry.
Don't worry about that. You can ignore it, and we will tidy up the PR template. |
Ping @robertbrignull. Anything else you need from me? |
I'm very sorry for the delay. We have now discussed this internally. Unfortunately we're not sure we want to go this route because the implementation looks more complicated than initially thought. We don't always use As well as the implementation cost it's also not fully clear if this config option is what we will want long term. Is it the right level of granularity or should it apply to things other than workspace folders? Unfortunately it would be hard to change later without breaking user's workspaces. Overall we might have to ask you to keep using your workaround for now. If this continues to be a problem or if there are other users experiencing the same issue, we can revisit this. |
Thanks for the response. I think your concerns are reasonable. That being said, I'm wondering if there's any path forward for this by just focusing on
Funnily enough, the workaround started causing issues this morning. I think after I updated to 2.17.4, |
This PR adds an
ignoredFolders
setting to the extension. Currently,getOnDiskWorkspaceFolders
gets all root folders in a multi-root workspace to use for things like--additional-packs
when invoking the CLI tool. This can cause major latency (multiple minutes) when the workspace folders are large. By ignoring these folders, the extension is significantly faster.Another way to get the speed up is to add an empty
codeql-workspace.yml
in the root of the large workspace folder to prevent them from being searched, but I don't want to have to create this file every time I need to do this, as usually I can't add it to upstream projects.I'll update the CHANGELOG once I get some confirmation this is OK to add.
Checklist
ready-for-doc-review
label there.