Add new table vscode_extensions #8150
Conversation
There was a problem hiding this comment.
Hello @KavetiRohith, thank for looking into this.
I did a first quick pass; beyond the comments, please use snake_case for variable names.
specs/vscode_extensions.table
Outdated
| table_name("vscode_extensions") | ||
| description("Lists all vscode extensions.") | ||
| schema([ | ||
| Column("id", TEXT, "Extension id"), |
There was a problem hiding this comment.
Interesting that this is the id. Elsewhere in vscode, this data is the name. But 🤷
I think it'd be good to include the uuid as well.
There was a problem hiding this comment.
There seem to be a few identifiers. This is referred to in the JSON as id (and sort of identifier) but in the UI as identifier
{
"identifier": {
"id": "ms-python.python",
"uuid": "f1f59ae4-9318-4f3c-a9b5-81b2eaa5f8a5"
},
"version": "2023.20.0",
"location": {
"$mid": 1,
"fsPath": "/Users/zwass/.vscode/extensions/ms-python.python-2023.20.0",
"external": "file:///Users/zwass/.vscode/extensions/ms-python.python-2023.20.0",
"path": "/Users/zwass/.vscode/extensions/ms-python.python-2023.20.0",
"scheme": "file"
},
"relativeLocation": "ms-python.python-2023.20.0",
"metadata": {
"id": "f1f59ae4-9318-4f3c-a9b5-81b2eaa5f8a5",
"publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8",
"publisherDisplayName": "Microsoft",
"targetPlatform": "undefined",
"isApplicationScoped": false,
"updated": true,
"isPreReleaseVersion": false,
"installedTimestamp": 1698970500850,
"preRelease": false
}
}
I think it might make sense to call it identifier?
There is also a friendly name. Not sure what that is referred to in the UI, and it's not available directly in the extensions.json (though it is available in the individual packages in that directory).
There was a problem hiding this comment.
Depends which json. It looks like it's id in .extensions.json but name in foo/package.json. Similarly, uuid vs __metadata.id.
Given the internal inconsistency, I think we should just pick id/uuid or name/id. As long as they're both exposed I think we'll be okay
|
I approved this but then noticed merge conflicts (which I pushed a commit to resolve). I think someone else will now need to add an approval to get it merged. |
There was a problem hiding this comment.
There's only one thing that doesn't convince me completely, and it's the getStringValuefromRjObject function used to parse every field.
I would expect those fields to have a specific type and not change; and if they do, we want to know.
Shouldn't we only parse if the type is what we expect?
|
I don't feel strongly either way, but my instinct is we should render what's in the json file regardless of whether it matches the type we expect. Possibly different versions of vscode could end up using different types? |
Note that this is already an issue for the schema as written, since it assumes that |
|
@Smjert I just pushed changes that use the expected types when reading the values (and also eliminates the helper function to try to make things a little more obvious). |
|
Forgot that tests that test tables that need to access users and groups data need to explicitly start the service to collect them on Windows. I've updated. |
There was a problem hiding this comment.
LGTM. Thank you @KavetiRohith for the initial work and @Smjert for helping get it across the finish line!
To submit a PR please make sure to follow the next steps:
CONTRIBUTING.mdguide at the root of the repo.format_checktarget.If it is not, then move the committed files to the git staging area,
build the
formattarget to format them, and then re-commit.More information is available on the wiki.
Add new table vscode_extensions. #8138