Push the two-step navigation system further through roslyn. #59802
Conversation
|
@jasonmalinowski this is ready for review. Note: there continues to be more coming down the line :) |
| editorWorkspace, documentId, resolvedRenameToken.Span, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| if (location != null && | ||
| await location.NavigateToAsync(cancellationToken).ConfigureAwait(false)) |
There was a problem hiding this comment.
not this pattern gets simpler in the next followup.
|
@jasonmalinowski this is ready for review. |
src/EditorFeatures/Core/Extensibility/NavigationBar/AbstractEditorNavigationBarItemService.cs
Outdated
Show resolved
Hide resolved
| Presenter._workspace, options, cancellationToken); // Only activate the tab if requested | ||
| public async Task NavigateToAsync(NavigationOptions options, CancellationToken cancellationToken) | ||
| { | ||
| // Only activate the tab if requested |
There was a problem hiding this comment.
Not sure the comment makes sense here?
| @@ -117,6 +117,7 @@ private class ExternalDefinitionItem : DefinitionItem | |||
| public override Task<bool> CanNavigateToAsync(Workspace workspace, CancellationToken cancellationToken) | |||
| => SpecializedTasks.True; | |||
|
|
|||
| [Obsolete] | |||
There was a problem hiding this comment.
What is preventing deletion here?
There was a problem hiding this comment.
i will look into this.
There was a problem hiding this comment.
this subclasses hte DefinitionItem guy that i'm not sure is totally isolated from TS/F#. I will figure out what the EA story is there and delete if i can in followup.
| if (!await this.CanNavigateToSpanAsync(workspace, documentId, textSpan, allowInvalidSpan, cancellationToken).ConfigureAwait(false)) | ||
| return null; |
There was a problem hiding this comment.
GetNavigableLocationAsync won't already return null if it's not navigable here?
There was a problem hiding this comment.
(comment applies to the other methods here)
There was a problem hiding this comment.
it should... but i wanted to make absolutely sure there was no disagreement on behavior so this helps enforce that.
| @@ -422,6 +397,23 @@ static VsTextSpan GetVsTextSpan(SourceText text, int position, int virtualSpace) | |||
| private static int GetPositionWithinDocumentBounds(int position, int documentLength) | |||
| => Math.Min(documentLength, Math.Max(position, 0)); | |||
|
|
|||
| private static VsTextSpan GetVsTextSpan(SourceText text, TextSpan textSpan, bool allowInvalidSpan) | |||
There was a problem hiding this comment.
Not your doing, but oh goodness is a parameter called "allowInvalidSpan" terrifying.
There was a problem hiding this comment.
yeah. for context. this allows us to use 'cached navto results from previous vs session' without having to do the costly work to ensure they're valid. IN a sense the feature is allowing navigation to invalid locations, albeit, with a message to the user that this may be happening. it enables huge speedups, but it means we need to be resilient to old data being stale and innacurate.
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
Precursor to the background-work-indicator work.
Followup to #59801.
THis just takes the concept further as we have many navigation components taht call into each other. This way we can do as much work as possible in stage one and then return the deepest level stage-2 navigation step as possible.