-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add support for navigation to mapped file paths. #45956
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
Conversation
This supports go to def / impl / far from a c# file to a razor file.
| public bool TryNavigateToSpan(Workspace workspace, DocumentId documentId, TextSpan textSpan, OptionSet options) | ||
| { | ||
| // Navigation should not change the context of linked files and Shared Projects. | ||
| documentId = workspace.GetDocumentIdInCurrentContext(documentId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the logic in this method was duplicated across multiple methods, I just combined them all.
There's basically 3 different parameters - TextSpan, line position + character, and just position. The APIs we call require both a TextSpan (just a start and end location) or a VsTextSpan(line positions + characters)
|
|
||
| static VsTextSpan GetVsTextSpan(SourceText text, int position, int virtualSpace) | ||
| { | ||
| var boundedPosition = GetPositionWithinDocumentBounds(position, text.Length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved from line 225 in original file
| { | ||
| var document = OpenDocument(workspace, documentId); | ||
| // Before attempting to open the document, check if the location maps to a different file that should be opened instead. | ||
| var document = workspace.CurrentSolution.GetDocument(documentId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is new logic
| } | ||
| } | ||
|
|
||
| private bool TryNavigateToMappedFile(Workspace workspace, ISpanMappingService spanMappingService, Document generatedDocument, TextSpan textSpan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also entirely new logic.
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
jasonmalinowski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is potentially adding a deadlock risk because of use of GetService in a MEF constructor; that definitely needs to be fixed up. Other comments are less critical. 😄
...ualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.OpenFileTracker.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/ProjectSystem/RunningDocumentTableEventTracker.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs
Show resolved
Hide resolved
src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioDocumentNavigationService.cs
Outdated
Show resolved
Hide resolved
JoeRobich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
src/VisualStudio/Core/Def/Implementation/ProjectSystem/RunningDocumentTableEventTracker.cs
Outdated
Show resolved
Hide resolved
|
@jinujoseph for m2 approval |
This supports go to def / impl / far from a c# file to a razor file.
With #45954 we can remove a bit of the logic around opening the file and reading the RDT.
Resolves #45767 and partially #45063 TODO).