Only store tracking spans in the editor side of nav bar items#53878
Only store tracking spans in the editor side of nav bar items#53878CyrusNajmabadi merged 4 commits intodotnet:mainfrom
Conversation
|
|
||
| public async Task<IList<NavigationBarItem>?> GetItemsAsync(Document document, CancellationToken cancellationToken) | ||
| public async Task<ImmutableArray<NavigationBarItem>> GetItemsAsync( | ||
| Document document, ITextSnapshot textSnapshot, CancellationToken cancellationToken) |
There was a problem hiding this comment.
note: this is the editor portion of the API. it does pass along the textsnaphsot now as the Editor portion returns navbaritems that have tracking spans in them. so this allows us to create those tracking spans at construction time, not some point in the future after. the feature api is of course oblivious to this.
| } | ||
|
|
||
| public Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView textView, CancellationToken cancellationToken) | ||
| public Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView textView, ITextSnapshot textSnapshot, CancellationToken cancellationToken) |
There was a problem hiding this comment.
navigation also takes in the text snapshot so we can determine teh locatin of an item at the point that we navigate. However, currently only TS and F# use this. Roslyn always goes back to symbols to navigate. that will be fixed in one of the followup prs coming shortly.
| /// Returns <see langword="true"/> if navigation (or generation) happened. <see langword="false"/> otherwise. | ||
| /// </summary> | ||
| Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken); | ||
| Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, ITextSnapshot textSnapshot, CancellationToken cancellationToken); |
There was a problem hiding this comment.
these apis are EA, so it's fine to change them.
| public ImmutableArray<NavigationBarItem> ChildItems { get; } | ||
|
|
||
| public ImmutableArray<TextSpan> Spans { get; internal set; } | ||
| internal ImmutableArray<ITrackingSpan> TrackingSpans { get; set; } = ImmutableArray<ITrackingSpan>.Empty; |
There was a problem hiding this comment.
no need for 'Spans' anymore in the editor api. nothing uses that. ANd TRackingSpans is no longer mutable as it is provided at construction time.
| { | ||
| var navBarService = document.GetRequiredLanguageService<INavigationBarItemService>(); | ||
| var snapshot = _subjectBuffer.CurrentSnapshot; | ||
| item.Spans = item.TrackingSpans.SelectAsArray(ts => ts.GetSpan(snapshot).Span.ToTextSpan()); |
There was a problem hiding this comment.
how we hackily used to do things.
| : this(text, glyph, spans.ToImmutableArrayOrEmpty(), childItems.ToImmutableArrayOrEmpty(), indent, bolded, grayed) | ||
| { | ||
| } | ||
| public ImmutableArray<ITrackingSpan> TrackingSpans { get; } |
There was a problem hiding this comment.
documentation on the purpose of these tracking spans would be useful (from the PR this looks to be the spans used to determine which item is selected based on caret location)
There was a problem hiding this comment.
Yup. It' in the next PR :)
This is part of a larger refactoring for navbars. The final form will be that a navbar item can store:
this PR does '1'.