Add CopilotRelated command #59963
Add CopilotRelated command #59963
Conversation
Add a command to provide information about a file for copilot.
typescript-bot
added
Author: Team
For Uncommitted Bug
|
Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary. |
|
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
I couldn't come up with a test for JSX implicit import base, so that code may be incorrect. I also wonder whether it's necessary to explicitly only skip tslib and jsx implicit import bases. Probably either all implicit imports or none of them should be skipped. In fact, if the content of synthetic imports usually varies, it would be better to include them. If they're the same, the LLM will have already seen them many times.
The code is a lot simpler now.
src/server/protocol.ts
Outdated
|
|
||
| export interface CopilotRelatedItems { | ||
| relatedFiles: readonly string[]; | ||
| traits: { name: string; value: string; }[]; |
There was a problem hiding this comment.
what is this supposed to be?
There was a problem hiding this comment.
Project-related information for copilot like module, lib, target, etc; not implemented in this PR, but coming soon. (and already implemented by the C# extension and I think C++ as well)
There was a problem hiding this comment.
But do we know how the structure will look like. May be add this when we actually set?
There was a problem hiding this comment.
It's not going to change substantially, although it may add some optional properties -- I believe there's a PR from the C++ team to do so. I'm going to start returning some project information as traits quite soon and C# is already doing so. So I'd like to add it now, and I can add any optional properties that we use then.
| specifiers = specifiers.slice(specifiers.findIndex(nodeIsSynthesized) + 1); | ||
| if (specifiers.length === 0) return []; | ||
| for (const specifier of specifiers) { | ||
| const name = program.getResolvedModuleFromModuleSpecifier(specifier, file)?.resolvedModule?.resolvedFileName; |
There was a problem hiding this comment.
it may happen that we have resolvedFileName = say "module.js" but its not part of program. Is this ok to send back that file name ?
There was a problem hiding this comment.
Yes, files that are not part of the program are fine; we don't expect to get information from Typescript about them -- they are fine if they exist on disk.
src/services/services.ts
Outdated
| const imports: Set<string> = new Set(); | ||
| let specifiers = file.imports; | ||
| specifiers = specifiers.slice(specifiers.findIndex(nodeIsSynthesized) + 1); | ||
| if (specifiers.length === 0) return []; |
There was a problem hiding this comment.
I thought emptyArray was slower than []. Also, what happens if somebody mutates the array returned from getImports? Is that a concern for services?
Use emptyArray instead of [] Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
Inline init of lazily constructed Set Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
Add a command to provide information about a file for copilot.
Associated VSCode PR: microsoft/vscode#228610
@genlu you were interested in seeing this.