utils: Changes to enable re running commands with copilot#2136
utils: Changes to enable re running commands with copilot#2136
Conversation
utils/src/copilot/copilot.ts
Outdated
| } | ||
|
|
||
| function extractActivityChildren(context: string): string { | ||
| const activityLog = JSON.parse(context) as { children?: unknown }; |
There was a problem hiding this comment.
Should probably try/catch this just in case the shape is corrupted.
utils/src/registerCommand.ts
Outdated
| async (context: types.IActionContext) => { | ||
| let injectedContext: Partial<types.IActionContext> | undefined; | ||
| if (args.length > 0) { | ||
| const metadata = args[args.length - 1]; |
There was a problem hiding this comment.
Should we just do a .find on the metadata instead of assuming that it'll always be at the end if the arg list? I think that'll make this command a little more resilient.
|
In the previous a number of changes were made:
|
| "rootDir": ".", | ||
| "declaration": false | ||
| "declaration": false, | ||
| "skipLibCheck": true |
There was a problem hiding this comment.
For some reason the @vscode-jsonrpc/node which the github copilot sdk has a dependency on causes issues. From what I have looked into the package does not have a proper package map so when we try to build the cjs it causes issues. I think this is mainly because the github copilot sdk is currently only using esm. If that is gonna cause problems for us I can revert this back to use the vscode llm.
Not sure if that is also the reason the tests are now failing.
There was a problem hiding this comment.
We have seen a number of issues where the CJS and ESM types conflict but so far we've been able to just put a targeted @ts-ignore in the code. Is that possible here?
| "vscode": "^1.105.0" | ||
| }, | ||
| "dependencies": { | ||
| "@github/copilot-sdk": "0.1.19", |
There was a problem hiding this comment.
Can we make sure this isn't wrecking our bundle size?
| "rootDir": ".", | ||
| "declaration": false | ||
| "declaration": false, | ||
| "skipLibCheck": true |
There was a problem hiding this comment.
We have seen a number of issues where the CJS and ESM types conflict but so far we've been able to just put a targeted @ts-ignore in the code. Is that possible here?
3 participants
This PR includes a couple of changes:
Some of these changes are used in RG and others in the ACA extension.