-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Small nav-to changes prior to some intensive rewrites. #53869
Conversation
childItems:=rightHandMemberItems.ToImmutableArray()) | ||
Else | ||
Return New ActionlessItem( | ||
String.Format(VBFeaturesResources._0_Events, containingType.Name), | ||
Glyph.EventPublic, | ||
indent:=1, | ||
spans:=allMethodSpans.ToImmutableArray(), | ||
childItems:=rightHandMemberItems.ToImmutableArray()) |
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.
so this loks like we're losing data. However here's why this is safe:
- Only VB makes these ActionlessItem's
- We literally do nothing when an ActionlessItem is clicked on. You can see that here:
roslyn/src/EditorFeatures/VisualBasic/NavigationBar/VisualBasicEditorNavigationBarItemService.vb
Lines 76 to 85 in e126f50
Protected Overrides Async Function NavigateToItemAsync(document As Document, item As WrappedNavigationBarItem, textView As ITextView, cancellationToken As CancellationToken) As Task Dim underlying = item.UnderlyingItem Dim generateCodeItem = TryCast(underlying, AbstractGenerateCodeItem) Dim symbolItem = TryCast(underlying, SymbolItem) If generateCodeItem IsNot Nothing Then Await GenerateCodeForItemAsync(document, generateCodeItem, textView, cancellationToken).ConfigureAwait(False) ElseIf symbolItem IsNot Nothing Then Await NavigateToSymbolItemAsync(document, symbolItem, cancellationToken).ConfigureAwait(False) End If
As we can, VB only supports doing something when a SymbolItem or AbstractGenerateCodeItem was the one selected. ActionlessItems are solely for having a hierarchy in the UI.
|
||
// Set when kind == RoslynNavigationBarItemKind.Symbol | ||
|
||
[DataMember(Order = 7)] | ||
public readonly ImmutableArray<TextSpan> Spans; |
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.
spans are now a thing only for symbol-items. no other items need that.
spans: GetSpansInDocument(type, tree, cancellationToken), | ||
selectionSpan: GetSelectionSpan(type, tree), |
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.
these values are added for LSP purposes. note that spans/selectionSpan will be refactored again in my next pr which unifies span handling. It will unify handling of both 'entire entity span' vs 'navigation span' as well as 'span to navigate to if in the starting document' vs 'span to navigate to in another document'.
public readonly TextSpan? SelectionSpan; | ||
[DataMember(Order = 11)] | ||
public readonly SymbolKey? NavigationSymbolId; | ||
[DataMember(Order = 12)] | ||
public readonly int NavigationSymbolIndex; |
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.
NavigationSymbolId and NavigationSymbolIndex currently exist but will be removed in my followup PR. tools must not go back from tehse to symbols. instead, the service should accurately and uniquely define the behavior the consuming services need through simple data it passes back.
No description provided.