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

Expose FAR to cohosting #76002

Merged
merged 2 commits into from
Nov 27, 2024
Merged

Expose FAR to cohosting #76002

merged 2 commits into from
Nov 27, 2024

Conversation

davidwengier
Copy link
Member

Roslyn side of dotnet/razor#11237

@davidwengier davidwengier requested a review from a team November 21, 2024 06:44
@davidwengier davidwengier requested a review from a team as a code owner November 21, 2024 06:44
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 21, 2024
@davidwengier davidwengier merged commit 6f2ed01 into dotnet:main Nov 27, 2024
25 checks passed
@davidwengier davidwengier deleted the CohostFAR branch November 27, 2024 02:28
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Nov 27, 2024
Copy link
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

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

lgtm, apologies for not getting to this sooner - def ping me directly on teams if its taking too long.


internal static class FindAllReferences
{
public static async Task<SumType<VSInternalReferenceItem, Location>[]?> FindReferencesAsync(Workspace workspace, Document document, LinePosition linePosition, bool supportsVSExtensions, CancellationToken cancellationToken)
Copy link
Member

Choose a reason for hiding this comment

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

You might consider having a progress parameter here and passing it in from Razor - otherwise you could be waiting a while for anything to stream in. I don't think you need all the references before you can do anything with them right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now Razor doesn't support streaming at all, because the LSP client middle layer doesn't. Cohosting removes that limitation, but none of our shared code on the Razor file knows how to deal with it, plus we'd have to work out how to stream things from OOP back to devenv to send to the language client, so I just decided to punt all of that until later :)

davidwengier added a commit to dotnet/razor that referenced this pull request Dec 1, 2024
Fixes #11237
Needs dotnet/roslyn#76002 and a version bump

Very simple one, though I was a little cheeky. There is basically no
code that could be shared, except for code that removes `__o` and
`k_BackingField` from the results, but those methods operate on VS LSP
types. Given cohosting largely uses Roslyn LSP types, and will be
exclusively FUSE, and FUSE doesn't have `__o`, I just skipped that bit.
Confirmed in VS that having cohosting and FUSE on makes for nice looking
results with no generated code artifacts.

Side note: I don't think anything will ever have `k_BackingField` in the
results, but there is zero test coverage so who knows! 😁
@dibarbet dibarbet modified the milestones: Next, 17.13 P3 Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants