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

Fix completion in AdhocWorkspace #20108

Conversation

DustinCampbell
Copy link
Member

I noticed this while working through an issue using the CompletionService in OmniSharp. Essentially, the AbstractMemberInsertingCompletionProvider was never updated to stop calling GetOpenDocumentInCurrentContextWithChanges(...) as it did in the era before the CompletionService when every provider was passed an ITextView and ITextBuffer. Nowadays, since there's a readily available Document, it can just use that.

Without this change, it breaks a scenario with AdhocWorkspace and other simple Workspace implementations. Essentially, GetOpenDocumentInCurrentContextWithChanges(...) allows the caller to retrieve a Document from a SourceText. It achieves this be calling Workspace.TryGetWorkspace(...) with the SourceText's SourceTextContainer. However, in the case of a standard SourceText, this is a StaticContainer that changes every time SourceText.WithChanges(...) is called. So, if the SourceText is updated for a particular Document in an AdhocWorkspace, calling GetOpenDocumentInCurrentContextWithChanges(...) on the new SourceText will fail because its container is different than the one that was initially registered with the Workspace.

cc @dotnet/roslyn-ide

…ctMemberInsertingCompletionProvider

I noticed this while working through an issue using the CompletionService in OmniSharp.

`GetOpenDocumentInCurrentContextWithChanges(...)` allows the caller to retrieve a Document from a SourceText. It achieves this be calling `Workspace.TryGetWorkspace(...)` with the SourceText's SourceTextContainer. However, in the case of a standard SourceText, this is just a StaticContainer that changes every time `SourceText.WithChanges(...)` is called. So, if the SourceText is updated for a particular Document in an AdhocWorkspace, calling `GetOpenDocumentInCurrentContextWithChanges(...)` for the new SourceText will fail because its container is different than the one that was initially registered with the Workspace.
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

That extension really needs to go.


//var sourceText = textSnapshot.AsText();
Copy link
Member

Choose a reason for hiding this comment

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

This was probably a good hint that this code was suspect.

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally

@DustinCampbell DustinCampbell merged commit 17c27d0 into dotnet:dev15.6 Jun 8, 2017
@sharwell sharwell added this to the 15.5 milestone Jun 21, 2017
@DustinCampbell DustinCampbell deleted the fix-completion-in-adhocworkspace branch August 21, 2017 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants