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.

Sign up for GitHub

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

Jump to bottom

Add supportsVSExtensions to DocumentSymbolService #11121

Closed
wants to merge 6 commits into from
Closed

Add supportsVSExtensions to DocumentSymbolService #11121

wants to merge 6 commits into from

Conversation

ryzngard
Copy link
Contributor

@ryzngard ryzngard commented Oct 30, 2024 •
edited
Loading

Last commit only, rest are from #11113

alexgav and others added 5 commits October 29, 2024 08:04
@alexgav
Update Roslyn package references to latest available and do minor fixup
0005295
@alexgav
Updating Versions.Details.xml to match Versions.props
3bad566
@alexgav
PR suggestion
ba26b3a
@alexgav
Fixing test failures
0a36f91 
Roslyn was requesting new IRazorMappingService and our TestRazorDocumentServiceProvider was throwing. Returning null for now per suggestion from Andrew.
@ryzngard
Add supportsVSExtensions
9770bc1
@ryzngard ryzngard requested review from a team as code owners October 30, 2024 00:23
@ryzngard ryzngard mentioned this pull request Oct 30, 2024
Merged
davidwengier
davidwengier approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

This is good, but it please don't have this PR close #11102

That issue is about not sending the entire client capabilities to OOP with every completion request, and is still needed.

@ryzngard
Copy link
Contributor Author

This is good, but it please don't have this PR close #11102

That issue is about not sending the entire client capabilities to OOP with every completion request, and is still needed.

It would help if I read the issue instead of just the discussion about it. I'll make sure it doesn't close :)

@alexgav
Copy link
Contributor

alexgav commented Oct 30, 2024 •
edited
Loading

@davidwengier correct me if I'm wrong, but this will also be unnecessary (the entire change in this PR) once #11102 is implemented and will need to get undone.

I wonder if it makes sense to submit it at all.

@davidwengier
Copy link
Contributor

Strictly speaking nothing about this becomes wrong when that issue is fixed, but it certainly could be reverted and we could save sending a bool over the wire.

@alexgav
Copy link
Contributor

alexgav commented Oct 30, 2024

Strictly speaking nothing about this becomes wrong when that issue is fixed, but it certainly could be reverted and we could save sending a bool over the wire.

FWIW I'm working on implementation of #11102 right now.

@ryzngard ryzngard closed this Oct 30, 2024
@ryzngard
Copy link
Contributor Author

Closing in favor of @alexgav work on #11102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidwengier davidwengier davidwengier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ryzngard @alexgav @davidwengier