Skip to content

Conversation

@koesie10
Copy link
Member

This will change the behavior of the "Create new query" command to create the new query in the same folder as the first selected item in the queries panel. If no items are selected, the behavior is the same as before.

I've used events to communicate the selection from the queries panel to the local queries module. This is some more code and some extra complexity, but it ensures that we don't have a dependency from the local queries module to the queries panel module. This makes testing easier.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@koesie10 koesie10 marked this pull request as ready for review October 24, 2023 11:57
@koesie10 koesie10 requested review from a team as code owners October 24, 2023 11:57
@koesie10 koesie10 force-pushed the koesie10/use-selected-queries-item branch from b5a416c to 6b741b4 Compare October 24, 2023 14:00
Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't taken a really close look at the code yet, but I've been trying to get it to work locally!

At the moment, I can't create a query inside most folders because the codeql-custom-queries-xxx folder doesn't exist. E.g. when highlighting the ql/cpp/ql folder, I get

Could not create query example file: Error: ENOENT: no such file or directory, scandir 'c:\git-repo\vscode-codeql-starter\ql\cpp\ql\codeql-custom-queries-cpp'

Am I missing something about to use this? 😅 Or maybe I'm hitting a Windows bug 🐛 👀

This will change the behavior of the "Create new query" command to
create the new query in the same folder as the first selected item in
the queries panel. If no items are selected, the behavior is the same
as before.

I've used events to communicate the selection from the queries panel to
the local queries module. This is some more code and some extra
complexity, but it ensures that we don't have a dependency from the
local queries module to the queries panel module. This makes testing
easier.
Before, if you had selected a folder or file within for example
`codeql-custom-queries-java` and selected `java` as the language, it
would create a nested folder within `codeql-custom-queries-java` with
the name `codeql-custom-queries-java`. This is unexpected for the user,
who would expect a new query to be created within
`codeql-custom-queries-java`. This fixes that by checking for this
specific condition. It does not fix it for all scenarios, such as where
the selected file/folder is nested multiple levels deep within the
`codeql-custom-queries-java` folder.
@koesie10 koesie10 force-pushed the koesie10/use-selected-queries-item branch from 6b741b4 to f0f5538 Compare October 25, 2023 12:37
@koesie10
Copy link
Member Author

Am I missing something about to use this? 😅 Or maybe I'm hitting a Windows bug 🐛 👀

I was testing in a different workspace which didn't have the codeql-custom-queries-* folders as workspace folders, but it seems like I missed this scenario. I've now removed isFolderAlreadyInWorkspace as a check since this should already be handled by pathExists(join(this.qlPackStoragePath, this.folderName)). Could you try again?

Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good to me! And thank you for fixing the no such file or directory errors I had earlier 📂 🎉

One more thing I'm wondering about (but feel free to just merge this and potentially follow up with James later):
Could we create the query (and the ql pack) directly inside the highlighted folder, instead of nested inside a codeql-custom-queries-* folder? E.g. in this video, I'd expect the query to be created directly inside example/snippets, since that's what the file explorer does.

create-query-inside-folder.mp4

However, it might be awkward to make sure we're in a suitable QL pack, so perhaps this isn't feasible!

@koesie10
Copy link
Member Author

Thanks @shati-patel, I agree that that behavior would be more useful. I've created an internal issue to track this, but I'll merge this now since it's already an improvement over the previous behavior.

@koesie10 koesie10 merged commit ff36088 into main Oct 26, 2023
@koesie10 koesie10 deleted the koesie10/use-selected-queries-item branch October 26, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants