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

Support for copilot-generated summaries in quick info. (On-the-fly docs) #12552

Merged
merged 51 commits into from
Dec 9, 2024

Conversation

spebl
Copy link
Contributor

@spebl spebl commented Aug 8, 2024

This introduces a new setting C_Cpp.onTheFlyDocsEnabled to control the display of the option to show copilot-generated summaries in the hover tooltip.
When enabled, and also authenticated with the vscode-copilot extension, the hover tooltip will display an option to generate a summary of the symbol with copilot.
The setting is defaulted to default which will check the feature flag control to determine if the feature should be enabled, which allows for slow rollout and the ability to rollback should any issues arise.

Updating the vscode requirement to 1.90.0 to support using copilot features with "vscode.lm".

The IntelliSense client changes supporting this feature are included in a separate PR against that repository.

Some workarounds were needed to support icon rendering in hover markdown and updating hover content dynamically which currently requires the hover to be closed and reopened. Potential fixes from proposals and unreleased patches are linked where applicable.

All feedback is very welcome!

OTF_DOCS_VSCODE

@spebl spebl added the Feature: Hover (Quick Info) An issue related to Hover (Quick Info) label Aug 8, 2024
Copy link
Member

@benmcmorran benmcmorran left a comment

Choose a reason for hiding this comment

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

Left a few wording comments but mostly looks good to me ✨. Leaving official sign-off for cpptools owners.

Extension/package.json Outdated Show resolved Hide resolved
Extension/package.nls.json Outdated Show resolved Hide resolved
Extension/src/LanguageServer/extension.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/extension.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/extension.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/extension.ts Show resolved Hide resolved
Extension/src/LanguageServer/settings.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/settings.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/settings.ts Outdated Show resolved Hide resolved
Extension/package.json Outdated Show resolved Hide resolved
@sean-mcmanus
Copy link
Contributor

@spebl Did you want this in our next 1.22.x or what release?

@spebl
Copy link
Contributor Author

spebl commented Aug 8, 2024

@spebl Did you want this in our next 1.22.x or what release?

Not currently targeting a specific release, no need to block anything on this going in. I first wanted to get feedback on this approach, along with the upgrading of the vscode version. I'm taking a look now at how we can best keep support for the older versions while also using the new language model apis when available.

Extension/src/LanguageServer/client.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/protocolFilter.ts Outdated Show resolved Hide resolved
Extension/src/LanguageServer/client.ts Outdated Show resolved Hide resolved
@benmcmorran
Copy link
Member

Capturing notes from a quick discussion I had with @Colengms. He's going to go ahead and move from LSP-based hover to a HoverProvider which should make it easier for us to opt-in to icons and avoid async work during settings initialization. We also discussed a potential alternative using multiple HoverProviders where a provider that actually displays Copilot results simply blocks until the user clicks the Copilot link from the first provider. FYI @spebl

@bobbrow bobbrow added this to the 1.22 milestone Aug 23, 2024
@sean-mcmanus sean-mcmanus added Feature: Copilot Hover GitHub Copilot hover "Generate Copilot summary" feature and removed Feature: Hover (Quick Info) An issue related to Hover (Quick Info) Feature: Copilot Context Integration with GitHub Copilot that provides additional context info labels Nov 27, 2024
sean-mcmanus

This comment was marked as resolved.

@sean-mcmanus sean-mcmanus requested a review from Colengms December 7, 2024 00:44
sean-mcmanus
sean-mcmanus previously approved these changes Dec 7, 2024
avoids hard coding true for security preference

Co-authored-by: Ben McMorran <[email protected]>
@spebl spebl dismissed stale reviews from sean-mcmanus and benmcmorran via ba4d436 December 7, 2024 00:51
@sean-mcmanus sean-mcmanus modified the milestones: 1.23, 1.23.3 Dec 7, 2024
@sean-mcmanus sean-mcmanus requested review from Colengms and removed request for Colengms and browntarik December 7, 2024 01:37
@spebl spebl merged commit fa80d44 into main Dec 9, 2024
6 checks passed
@spebl spebl deleted the dev/spebl/otfdocs branch December 9, 2024 18:15
@sean-mcmanus sean-mcmanus added Language Service Feature Request fixed Check the Milestone for the release in which the fix is or will be available. labels Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Copilot Hover GitHub Copilot hover "Generate Copilot summary" feature Feature Request fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants