Automatically name extension packs by koesie10 · Pull Request #2520 · github/vscode-codeql

koesie10 · 2023-06-16T11:36:46Z

This will change how extension packs are named in the data extensions editor. Before, the user had to pick a workspace folder and a name for the extension pack. Now, the workspace folder will be picked automatically if we can detect it (i.e. it follows the naming structure we expect), or the user will still need to select it. The extension pack name is always auto-generated based on the database name and the database language.

This adds a new codeQL.dataExtensions.disableAutoNameExtensionPack setting to disable this behavior while we are still working on changing how the data extensions editor works.

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.

This will change how extension packs are named in the data extensions editor. Before, the user had to pick a workspace folder and a name for the extension pack. Now, the workspace folder will be picked automatically if we can detect it (i.e. it follows the naming structure we expect), or the user will still need to select it. The extension pack name is always auto-generated based on the database name and the database language. This adds a new `codeQL.dataExtensions.disableAutoNameExtensionPack` setting to disable this behavior while we are still working on changing how the data extensions editor works.

starcke

Looks pretty good, just some smaller comments.

It felt a bit to odd to me that we just created the pack file but asked about the model file name. I would personally have expected the opposite. Later we do want the yml file to be based on the library name, so I am fine with it like this if that is easier.

extensions/ql-vscode/src/config.ts

extensions/ql-vscode/src/data-extensions-editor/extension-pack-picker.ts

koesie10

It felt a bit to odd to me that we just created the pack file but asked about the model file name. I would personally have expected the opposite. Later we do want the yml file to be based on the library name, so I am fine with it like this if that is easier.

They are essentially independent, so I've done only the extension pack name in this PR to keep the sizes of the PRs manageable. The model file name is in #2521. That means that for now the flow is a bit strange, but it helps to keep the changes minimal.

extensions/ql-vscode/src/data-extensions-editor/extension-pack-picker.ts

extensions/ql-vscode/src/config.ts

extensions/ql-vscode/src/data-extensions-editor/extension-pack-name.ts

extensions/ql-vscode/src/data-extensions-editor/extension-pack-picker.ts

extensions/ql-vscode/test/unit-tests/data-extensions-editor/extension-pack-name.test.ts

aeisenberg · 2023-06-20T18:01:24Z

extensions/ql-vscode/src/data-extensions-editor/extensions-workspace-folder.ts

+    return Uri.joinPath(workspaceFile, "..");
+  }
+
+  const workspaceFolders = getOnDiskWorkspaceFoldersObjects();


Does getOnDiskWorkspaceFoldersObjects return only file: URIs? If not, you will need to filter out the all the other ones.

Yes, that's exactly what getOnDiskWorkspaceFoldersObjects does, so this should only return workspace folders which actually exist on-disk.

aeisenberg · 2023-06-20T18:03:42Z

extensions/ql-vscode/src/data-extensions-editor/extensions-workspace-folder.ts

+  // The path closest to the workspace folders is the last element of the common root
+  const commonRootUri = commonRoot[commonRoot.length - 1];
+
+  // If we are at the root of the filesystem, we can't go up any further and there's something


This will be reasonably common in codeql projets. There's a quick-query command that creates a temp folder and adds it to the workspace. Any workspace that has a quick query folder will only have a common root at /.

Perhaps you will want to filter folders in the temp directory?

Also, maybe look for a .git folder?

I've added both filtering out temp directories and looking for the .git folder. If it can't find a common ancestor, it will now try to find the .git folder closest to one of the workspace folders and pick that.

aeisenberg

Nice.

Not necessary for this PR, but the heuristic for choosing a location for extension packs is quite complicated. It would be good to see it documented somewhere. (Either in code as comments, or in a separate document.)

…tension-pack

koesie10 added the secexp label Jun 16, 2023

koesie10 requested a review from starcke June 16, 2023 11:51

koesie10 marked this pull request as ready for review June 16, 2023 11:51

koesie10 requested review from a team as code owners June 16, 2023 11:51

Fix tests on Windows

5d83ac8

koesie10 force-pushed the koesie10/auto-name-extension-pack branch from 928fed1 to 5d83ac8 Compare June 16, 2023 14:01

starcke reviewed Jun 19, 2023

View reviewed changes

koesie10 added 4 commits June 19, 2023 13:34

Split flows for checking existing extension pack

8980aab

Separate pack naming and create interface

3c60708

Add more tests for auto pack naming

3323fd4

Add more comments

fe29a1a

koesie10 commented Jun 19, 2023

View reviewed changes

koesie10 requested a review from starcke June 19, 2023 12:34

aeisenberg reviewed Jun 19, 2023

View reviewed changes

Move workspace folder functions to separate file

ab6db71

starcke mentioned this pull request Jun 20, 2023

Automatically create different model files per library #2521

Merged

3 tasks

Store extension packs in .github/codeql/extensions

cfc66a4

koesie10 force-pushed the koesie10/auto-name-extension-pack branch from a0820d7 to cfc66a4 Compare June 20, 2023 09:16

Merge branch 'main' into koesie10/auto-name-extension-pack

1cc6aa5

koesie10 requested review from adityasharad and aeisenberg June 20, 2023 13:12

aeisenberg reviewed Jun 20, 2023

View reviewed changes

koesie10 added 2 commits June 21, 2023 09:44

Exclude workspace folders in the system temp dir

f32a240

Add detection of root workspace directory using .git folder

d092e69

aeisenberg approved these changes Jun 21, 2023

View reviewed changes

koesie10 added 2 commits June 22, 2023 10:01

Merge remote-tracking branch 'origin/main' into koesie10/auto-name-ex…

e4e849d

…tension-pack

Add comment to explain heuristic

7249f4c

koesie10 enabled auto-merge June 22, 2023 08:19

koesie10 merged commit 100b557 into main Jun 22, 2023

koesie10 deleted the koesie10/auto-name-extension-pack branch June 22, 2023 09:34

Conversation

koesie10 commented Jun 16, 2023

Checklist

Uh oh!

starcke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koesie10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aeisenberg Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

koesie10 Jun 21, 2023

Choose a reason for hiding this comment

Uh oh!

aeisenberg Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

aeisenberg Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

koesie10 Jun 21, 2023

Choose a reason for hiding this comment

Uh oh!

aeisenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants