Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS extension: register call to CopilotRelated with copilot extension #228610

Merged
merged 14 commits into from
Oct 7, 2024

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Sep 13, 2024

registerRelatedFilesProvider is a new method on the copilot extension. CopilotRelated is a new tsserver command that provides information from tsserver to copilot. (The information is not necessarily copilot-specific, of course.)

Depends on microsoft/TypeScript#59963 to be available -- the typescript protocol doesn't include CopilotRelated yet.

Needs a follow-up to update the copilot API:

  • The callback should take a cancellation token in addition to a URI so the caller can cancel.
  • Registration should return a Disposable so that the TS extension can unregister.

registerRelatedFilesProvider is a new method on the copilot extension.
CopilotRelated is a new tsserver command that provides information from
tsserver to copilot. (The information is not necessarily
copilot-specific, of course.)
Copy link

@Thanksyou23 Thanksyou23 left a comment

Choose a reason for hiding this comment

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

sandersn:typescript-relatedfiles-imports

callback: (uri: vscode.Uri) => Promise<{ entries: vscode.Uri[]; traits?: { name: string; value: string }[] }>
): void;
} | undefined;
if (relatedAPI) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this also need to be gated on the TS server version?

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect it to ship in 5.7. Gating it is a good idea. How do I do that? I saw a lot of // @ts-expect-error until ts 5.6 in the code base, accompanied by if (event.body.entries) kinds of checks. Is that the right way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, the language features are gated on API.v560 (for example). Is it OK to add API.v570?

@sandersn sandersn marked this pull request as ready for review September 20, 2024 20:36
Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few comments. Looks like the call to register may also be missing

The select provided at registration contains the correct list of
languages to register for the provider.
@mjbvz mjbvz added this to the October 2024 milestone Sep 23, 2024
@sandersn
Copy link
Member Author

sandersn commented Oct 2, 2024

My latest commit now has the API that we intend to use in the short- and medium-term.

@mjbvz mjbvz enabled auto-merge (rebase) October 4, 2024 22:56
@mjbvz mjbvz merged commit a003a0d into microsoft:main Oct 7, 2024
7 checks passed
@sandersn sandersn deleted the typescript-relatedfiles-imports branch October 7, 2024 15:16
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.

4 participants