Merge pull request #53893 from CyrusNajmabadi/navBar16.11

Fix vb event generation in navbars
dotnet · Jun 10, 2021 · e990abe · e990abe
2 parents dcf4ce7 + 700daeb
commit e990abe
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 20 deletions.
diff --git a/src/EditorFeatures/CSharp/NavigationBar/CSharpEditorNavigationBarItemService.cs b/src/EditorFeatures/CSharp/NavigationBar/CSharpEditorNavigationBarItemService.cs
@@ -44,7 +44,10 @@ public CSharpEditorNavigationBarItemService(IThreadingContext threadingContext)
             return new VirtualTreePoint(tree, tree.GetText(cancellationToken), location.SourceSpan.Start);
         }
 
-        protected override Task NavigateToItemAsync(Document document, WrappedNavigationBarItem item, ITextView textView, CancellationToken cancellationToken)
-            => NavigateToSymbolItemAsync(document, (RoslynNavigationBarItem.SymbolItem)item.UnderlyingItem, cancellationToken);
+        protected override async Task<bool> TryNavigateToItemAsync(Document document, WrappedNavigationBarItem item, ITextView textView, CancellationToken cancellationToken)
+        {
+            await NavigateToSymbolItemAsync(document, (RoslynNavigationBarItem.SymbolItem)item.UnderlyingItem, cancellationToken).ConfigureAwait(false);
+            return true;
+        }
     }
 }
diff --git a/...EditorFeatures/Core/Extensibility/NavigationBar/AbstractEditorNavigationBarItemService.cs b/...EditorFeatures/Core/Extensibility/NavigationBar/AbstractEditorNavigationBarItemService.cs
@@ -24,11 +24,11 @@ protected AbstractEditorNavigationBarItemService(IThreadingContext threadingCont
         }
 
         protected abstract Task<VirtualTreePoint?> GetSymbolNavigationPointAsync(Document document, ISymbol symbol, CancellationToken cancellationToken);
-        protected abstract Task NavigateToItemAsync(Document document, WrappedNavigationBarItem item, ITextView textView, CancellationToken cancellationToken);
+        protected abstract Task<bool> TryNavigateToItemAsync(Document document, WrappedNavigationBarItem item, ITextView textView, CancellationToken cancellationToken);
 
         [Obsolete("Caller should call NavigateToItemAsync instead", error: true)]
         public void NavigateToItem(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
-            => throw new NotSupportedException($"Caller should call {nameof(NavigateToItemAsync)} instead");
+            => throw new NotSupportedException($"Caller should call {nameof(TryNavigateToItemAsync)} instead");
 
         public async Task<IList<NavigationBarItem>?> GetItemsAsync(Document document, CancellationToken cancellationToken)
         {
@@ -38,8 +38,8 @@ public void NavigateToItem(Document document, NavigationBarItem item, ITextView
             return items.SelectAsArray(v => (NavigationBarItem)new WrappedNavigationBarItem(v));
         }
 
-        public Task NavigateToItemAsync(Document document, NavigationBarItem item, ITextView textView, CancellationToken cancellationToken)
-            => NavigateToItemAsync(document, (WrappedNavigationBarItem)item, textView, cancellationToken);
+        public Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView textView, CancellationToken cancellationToken)
+            => TryNavigateToItemAsync(document, (WrappedNavigationBarItem)item, textView, cancellationToken);
 
         protected async Task NavigateToSymbolItemAsync(
             Document document, RoslynNavigationBarItem.SymbolItem item, CancellationToken cancellationToken)

diff --git a/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarItemService.cs b/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarItemService.cs
@@ -8,6 +8,7 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.VisualStudio.Text.Editor;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Editor
 {
@@ -23,7 +24,10 @@ internal interface INavigationBarItemServiceRenameOnceTypeScriptMovesToExternalA
     {
         Task<IList<NavigationBarItem>?> GetItemsAsync(Document document, CancellationToken cancellationToken);
         bool ShowItemGrayedIfNear(NavigationBarItem item);
-        Task NavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken);
+        /// <summary>
+        /// 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);
     }
 
     internal class NavigationBarItemServiceWrapper : INavigationBarItemServiceRenameOnceTypeScriptMovesToExternalAccess
@@ -41,10 +45,10 @@ public NavigationBarItemServiceWrapper(INavigationBarItemService service)
         public bool ShowItemGrayedIfNear(NavigationBarItem item)
             => _service.ShowItemGrayedIfNear(item);
 
-        public Task NavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
+        public Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
         {
             _service.NavigateToItem(document, item, view, cancellationToken);
-            return Task.CompletedTask;
+            return SpecializedTasks.True;
         }
     }
 }
diff --git a/src/EditorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptNavigationBarItemService.cs b/src/EditorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptNavigationBarItemService.cs
@@ -40,16 +40,18 @@ public VSTypeScriptNavigationBarItemService(
             return items.Select(x => ConvertToNavigationBarItem(x)).ToList();
         }
 
-        public async Task NavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
+        public async Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
         {
-            if (item.Spans.Length <= 0)
-                return;
+            if (item.Spans.Length > 0)
+            {
+                await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
 
-            await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
+                var workspace = document.Project.Solution.Workspace;
+                var navigationService = VSTypeScriptDocumentNavigationServiceWrapper.Create(workspace);
+                navigationService.TryNavigateToPosition(workspace, document.Id, item.Spans[0].Start, virtualSpace: 0, options: null, cancellationToken: cancellationToken);
+            }
 
-            var workspace = document.Project.Solution.Workspace;
-            var navigationService = VSTypeScriptDocumentNavigationServiceWrapper.Create(workspace);
-            navigationService.TryNavigateToPosition(workspace, document.Id, item.Spans[0].Start, virtualSpace: 0, options: null, cancellationToken: cancellationToken);
+            return true;
         }
 
         public bool ShowItemGrayedIfNear(NavigationBarItem item)

diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs
@@ -388,8 +388,14 @@ private async Task ProcessItemSelectionAsync(NavigationBarItem item, Cancellatio
                     item.Spans = item.TrackingSpans.SelectAsArray(ts => ts.GetSpan(snapshot).Span.ToTextSpan());
                     var view = _presenter.TryGetCurrentView();
 
-                    // ConfigureAwait(true) as we have to come back to UI thread in order to kick of the refresh task below.
-                    await navBarService.NavigateToItemAsync(document, item, view, cancellationToken).ConfigureAwait(true);
+                    // ConfigureAwait(true) as we have to come back to UI thread in order to kick of the refresh task
+                    // below. Note that we only want to refresh if selecting the item had an effect (either navigating
+                    // or generating).  If nothing happened to don't want to refresh.  This is important as some items
+                    // exist in the type list that are only there to show a set a particular set of items in the member
+                    // list.  So selecting such an item should only update the member list, and we do not want a refresh
+                    // to wipe that out.
+                    if (!await navBarService.TryNavigateToItemAsync(document, item, view, cancellationToken).ConfigureAwait(true))
+                        return;
                 }
             }
 

diff --git a/src/EditorFeatures/VisualBasic/NavigationBar/VisualBasicEditorNavigationBarItemService.vb b/src/EditorFeatures/VisualBasic/NavigationBar/VisualBasicEditorNavigationBarItemService.vb
@@ -73,16 +73,21 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.NavigationBar
             Return location
         End Function
 
-        Protected Overrides Async Function NavigateToItemAsync(document As Document, item As WrappedNavigationBarItem, textView As ITextView, cancellationToken As CancellationToken) As Task
+        Protected Overrides Async Function TryNavigateToItemAsync(
+                document As Document, item As WrappedNavigationBarItem, textView As ITextView, cancellationToken As CancellationToken) As Task(Of Boolean)
             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)
+                Return True
             ElseIf symbolItem IsNot Nothing Then
                 Await NavigateToSymbolItemAsync(document, symbolItem, cancellationToken).ConfigureAwait(False)
+                Return True
             End If
+
+            Return False
         End Function
     End Class
 End Namespace
diff --git a/src/Tools/ExternalAccess/FSharp/Internal/Editor/FSharpNavigationBarItemService.cs b/src/Tools/ExternalAccess/FSharp/Internal/Editor/FSharpNavigationBarItemService.cs
@@ -44,7 +44,7 @@ public FSharpNavigationBarItemService(
             return items?.Select(x => ConvertToNavigationBarItem(x)).ToList();
         }
 
-        public async Task NavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
+        public async Task<bool> TryNavigateToItemAsync(Document document, NavigationBarItem item, ITextView view, CancellationToken cancellationToken)
         {
             // The logic here was ported from FSharp's implementation. The main reason was to avoid shimming INotificationService.
             if (!item.Spans.IsEmpty)
@@ -65,6 +65,8 @@ public async Task NavigateToItemAsync(Document document, NavigationBarItem item,
                     notificationService.SendNotification(EditorFeaturesResources.The_definition_of_the_object_is_hidden, severity: NotificationSeverity.Error);
                 }
             }
+
+            return true;
         }
 
         public bool ShowItemGrayedIfNear(NavigationBarItem item)