Use VisualStudioWorkspaceImpl to perform navigation for Inheritance margin#62053
Use VisualStudioWorkspaceImpl to perform navigation for Inheritance margin#62053Cosifne merged 3 commits intodotnet:mainfrom
Conversation
|
this looks ok to me... but also somewhat off. I thought we had dedicated services to map between teh workspaces we get for MAS/Decompile/Etc (like ISymbolMappingService?). @davidwengier for more thoughts here. |
jasonmalinowski
left a comment
There was a problem hiding this comment.
Echoing @CyrusNajmabadi concern's I'm a bit confused why this is working -- I would have expected the symbol mapping service be used here rather than hardcoding this.
src/VisualStudio/Core/Def/InheritanceMargin/InheritanceGlyphFactoryProvider.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMarginProvider.cs
Outdated
Show resolved
Hide resolved
Combined both threads here. The OOP side of the inheritance margin has already used ISymbolMappingService. So it has the correct DocumentId + Span passed back to devenv. Or should the OOP send the symbolKey + projectId back to devenv and later we restore the symbol and use ISymbolMappingService to get the correct navigation location? |
|
I have nothing interesting to add. It makes sense if OOP is using the mapping service, its going to get a DocumentId from the VS workspace, so looking that up in the MAS workspace wouldn't work. |
c66df05 to
28c619d
Compare
…argin (dotnet#62053) * Use VisualStudioWorkspaceImpl * Use VisualStudioWorkspace * Add an integration test to cover the case
Fix the issue that in the current build, user couldn't navigate using inheritance margin when it is in the Metadata workspace.
Like
The reason is, here if we are passing the workspace associated with the document to the glyph.
For symbol in source, inheritance margin use DocumentIdSpan to perform navigation, so if it is in the metadata workspace then it can't find the corresponding document in workspace.
Fix this issue by always using VisuaStudioWorkspaceImpl to perform navigation.
Currently, there is no way to cover this by a test, it would rely on #62047 merged in so that this scenario could be covered by Integration test.