-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Break code for handling a request to FindRefs/GoToBase/GoToImpl out of the editor command handlers for those features. #78751
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
Changes from all commits
001b8e9
a2db5aa
bb4403f
767f1de
adcda73
962a187
cd84ab8
79b6987
2330fff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using Microsoft.CodeAnalysis.Editor.Shared.Extensions; | ||
| using Microsoft.CodeAnalysis.Text; | ||
| using Microsoft.VisualStudio.Commanding; | ||
| using Microsoft.VisualStudio.Text.Editor.Commanding; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.GoOrFind; | ||
|
|
||
| internal abstract class AbstractGoOrFindCommandHandler<TCommandArgs>( | ||
| IGoOrFindNavigationService navigationService) : ICommandHandler<TCommandArgs> | ||
| where TCommandArgs : EditorCommandArgs | ||
| { | ||
| private readonly IGoOrFindNavigationService _navigationService = navigationService; | ||
|
|
||
| public string DisplayName => _navigationService.DisplayName; | ||
|
|
||
| public CommandState GetCommandState(TCommandArgs args) | ||
| { | ||
| var document = args.SubjectBuffer.CurrentSnapshot.GetOpenDocumentInCurrentContextWithChanges(); | ||
| return _navigationService.IsAvailable(document) | ||
| ? CommandState.Available | ||
| : CommandState.Unspecified; | ||
| } | ||
|
|
||
| public bool ExecuteCommand(TCommandArgs args, CommandExecutionContext context) | ||
| { | ||
| var subjectBuffer = args.SubjectBuffer; | ||
| var caret = args.TextView.GetCaretPoint(subjectBuffer); | ||
| if (!caret.HasValue) | ||
| return false; | ||
|
|
||
| var document = args.SubjectBuffer.CurrentSnapshot.GetOpenDocumentInCurrentContextWithChanges(); | ||
| if (!_navigationService.IsAvailable(document)) | ||
| return false; | ||
|
|
||
| return _navigationService.ExecuteCommand(document, caret.Value.Position); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.CodeAnalysis.Classification; | ||
|
|
@@ -17,22 +18,22 @@ | |
| using Microsoft.CodeAnalysis.Options; | ||
| using Microsoft.CodeAnalysis.Shared.Extensions; | ||
| using Microsoft.CodeAnalysis.Shared.TestHooks; | ||
| using Microsoft.CodeAnalysis.Text; | ||
| using Microsoft.CodeAnalysis.Threading; | ||
| using Microsoft.VisualStudio.Commanding; | ||
| using Microsoft.VisualStudio.Text; | ||
| using Microsoft.VisualStudio.Text.Editor.Commanding; | ||
| using Microsoft.VisualStudio.Threading; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.GoToDefinition; | ||
| namespace Microsoft.CodeAnalysis.GoOrFind; | ||
|
|
||
| internal abstract class AbstractGoOrFindCommandHandler<TLanguageService, TCommandArgs>( | ||
| /// <summary> | ||
| /// Core service responsible for handling an operation (like 'go to base, go to impl, find references') | ||
| /// and trying to navigate quickly to them if possible, or show their results in the find-usages window. | ||
| /// </summary> | ||
| internal abstract class AbstractGoOrFindNavigationService<TLanguageService>( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the core code for the absstract handler was broken out into this abstract service. This service has no command-handler dependencies (though it does have eidtor dependencies) and thus can be used simply by the SolutionExplorer rewrite i'm in the middle of. |
||
| IThreadingContext threadingContext, | ||
| IStreamingFindUsagesPresenter streamingPresenter, | ||
| IAsynchronousOperationListener listener, | ||
| IGlobalOptionService globalOptions) : ICommandHandler<TCommandArgs> | ||
| IGlobalOptionService globalOptions) | ||
| : IGoOrFindNavigationService | ||
| where TLanguageService : class, ILanguageService | ||
| where TCommandArgs : EditorCommandArgs | ||
| { | ||
| private readonly IThreadingContext _threadingContext = threadingContext; | ||
| private readonly IStreamingFindUsagesPresenter _streamingPresenter = streamingPresenter; | ||
|
|
@@ -59,7 +60,7 @@ internal abstract class AbstractGoOrFindCommandHandler<TLanguageService, TComman | |
| /// the presenter. In that case, the presenter will notify us that it has be re-purposed and we will also cancel | ||
| /// this source. | ||
| /// </remarks> | ||
| private readonly CancellationSeries _cancellationSeries = new(threadingContext.DisposalToken); | ||
| private CancellationTokenSource _cancellationTokenSource = new(); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i restored this to the original logic. it's acutaly subtly different from a cancellation series in a way i didn't realize. |
||
|
|
||
| /// <summary> | ||
| /// This hook allows for stabilizing the asynchronous nature of this command handler for integration testing. | ||
|
|
@@ -80,49 +81,34 @@ protected virtual StreamingFindUsagesPresenterOptions GetStreamingPresenterOptio | |
|
|
||
| protected abstract Task FindActionAsync(IFindUsagesContext context, Document document, TLanguageService service, int caretPosition, CancellationToken cancellationToken); | ||
|
|
||
| private static (Document?, TLanguageService?) GetDocumentAndService(ITextSnapshot snapshot) | ||
| { | ||
| var document = snapshot.GetOpenDocumentInCurrentContextWithChanges(); | ||
| return (document, document?.GetLanguageService<TLanguageService>()); | ||
| } | ||
| public bool IsAvailable([NotNullWhen(true)] Document? document) | ||
| => document?.GetLanguageService<TLanguageService>() != null; | ||
|
|
||
| public CommandState GetCommandState(TCommandArgs args) | ||
| { | ||
| var (_, service) = GetDocumentAndService(args.SubjectBuffer.CurrentSnapshot); | ||
| return service != null | ||
| ? CommandState.Available | ||
| : CommandState.Unspecified; | ||
| } | ||
|
|
||
| public bool ExecuteCommand(TCommandArgs args, CommandExecutionContext context) | ||
| public bool ExecuteCommand(Document document, int position) | ||
| { | ||
| _threadingContext.ThrowIfNotOnUIThread(); | ||
|
|
||
| var subjectBuffer = args.SubjectBuffer; | ||
| var caret = args.TextView.GetCaretPoint(subjectBuffer); | ||
| if (!caret.HasValue) | ||
| if (document is null) | ||
| return false; | ||
|
|
||
| var (document, service) = GetDocumentAndService(subjectBuffer.CurrentSnapshot); | ||
| var service = document.GetLanguageService<TLanguageService>(); | ||
| if (service == null) | ||
| return false; | ||
|
|
||
| Contract.ThrowIfNull(document); | ||
|
|
||
| // cancel any prior find-refs that might be in progress. | ||
| var cancellationToken = _cancellationSeries.CreateNext(); | ||
| _cancellationTokenSource.Cancel(); | ||
| _cancellationTokenSource = new(); | ||
|
|
||
| // we're going to return immediately from ExecuteCommand and kick off our own async work to invoke the | ||
| // operation. Once this returns, the editor will close the threaded wait dialog it created. | ||
| _inProgressCommand = ExecuteCommandAsync(document, service, caret.Value.Position, cancellationToken); | ||
| _inProgressCommand = ExecuteCommandAsync(document, service, position, _cancellationTokenSource); | ||
| return true; | ||
| } | ||
|
|
||
| private async Task ExecuteCommandAsync( | ||
| Document document, | ||
| TLanguageService service, | ||
| int position, | ||
| CancellationToken cancellationToken) | ||
| CancellationTokenSource cancellationTokenSource) | ||
| { | ||
| // This is a fire-and-forget method (nothing guarantees observing it). As such, we have to handle cancellation | ||
| // and failure ourselves. | ||
|
|
@@ -141,7 +127,7 @@ private async Task ExecuteCommandAsync( | |
| // any failures from it. Technically this should not be possible as it should be inside this same | ||
| // try/catch. however this code wants to be very resilient to any prior mistakes infecting later operations. | ||
| await _inProgressCommand.NoThrowAwaitable(captureContext: false); | ||
| await ExecuteCommandWorkerAsync(document, service, position, cancellationToken).ConfigureAwait(false); | ||
| await ExecuteCommandWorkerAsync(document, service, position, cancellationTokenSource).ConfigureAwait(false); | ||
| } | ||
| catch (OperationCanceledException) | ||
| { | ||
|
|
@@ -155,7 +141,7 @@ private async Task ExecuteCommandWorkerAsync( | |
| Document document, | ||
| TLanguageService service, | ||
| int position, | ||
| CancellationToken cancellationToken) | ||
| CancellationTokenSource cancellationTokenSource) | ||
| { | ||
| // Switch to the BG immediately so we can keep as much work off the UI thread. | ||
| await TaskScheduler.Default; | ||
|
|
@@ -173,6 +159,7 @@ private async Task ExecuteCommandWorkerAsync( | |
| // IStreamingFindUsagesPresenter. | ||
| var findContext = new BufferedFindUsagesContext(); | ||
|
|
||
| var cancellationToken = cancellationTokenSource.Token; | ||
| var delayBeforeShowingResultsWindowTask = DelayAsync(cancellationToken); | ||
| var findTask = FindResultsAsync(findContext, document, service, position, cancellationToken); | ||
|
|
||
|
|
@@ -203,7 +190,7 @@ await _streamingPresenter.TryPresentLocationOrNavigateIfOneAsync( | |
| // We either got no results, or 1.5 has passed and we didn't figure out the symbols to navigate to or | ||
| // present. So pop up the presenter to show the user that we're involved in a longer search, without | ||
| // blocking them. | ||
| await PresentResultsInStreamingPresenterAsync(document, findContext, findTask, cancellationToken).ConfigureAwait(false); | ||
| await PresentResultsInStreamingPresenterAsync(document, findContext, findTask, cancellationTokenSource).ConfigureAwait(false); | ||
| } | ||
|
|
||
| private Task DelayAsync(CancellationToken cancellationToken) | ||
|
|
@@ -213,7 +200,7 @@ private Task DelayAsync(CancellationToken cancellationToken) | |
| return delayHook(cancellationToken); | ||
| } | ||
|
|
||
| // If we want to navigate to a single result if it is found quickly, then delay showing the find-refs winfor | ||
| // If we want to navigate to a single result if it is found quickly, then delay showing the find-refs window | ||
| // for 1.5 seconds to see if a result comes in by then. If we're not navigating and are always showing the | ||
| // far window, then don't have any delay showing the window. | ||
| var delay = this.NavigateToSingleResultIfQuick | ||
|
|
@@ -227,8 +214,9 @@ private async Task PresentResultsInStreamingPresenterAsync( | |
| Document document, | ||
| BufferedFindUsagesContext findContext, | ||
| Task findTask, | ||
| CancellationToken cancellationToken) | ||
| CancellationTokenSource cancellationTokenSource) | ||
| { | ||
| var cancellationToken = cancellationTokenSource.Token; | ||
| await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); | ||
| var (presenterContext, presenterCancellationToken) = _streamingPresenter.StartSearch(DisplayName, GetStreamingPresenterOptions(document)); | ||
|
|
||
|
|
@@ -244,7 +232,7 @@ private async Task PresentResultsInStreamingPresenterAsync( | |
| // Hook up the presenter's cancellation token to our overall governing cancellation token. In other | ||
| // words, if something else decides to present in the presenter (like a find-refs call) we'll hear about | ||
| // that and can cancel all our work. | ||
| presenterCancellationToken.Register(() => _cancellationSeries.CreateNext()); | ||
| presenterCancellationToken.Register(() => cancellationTokenSource.Cancel()); | ||
|
|
||
| // now actually wait for the find work to be done. | ||
| await findTask.ConfigureAwait(false); | ||
|
|
@@ -291,9 +279,9 @@ internal TestAccessor GetTestAccessor() | |
|
|
||
| internal readonly struct TestAccessor | ||
| { | ||
| private readonly AbstractGoOrFindCommandHandler<TLanguageService, TCommandArgs> _instance; | ||
| private readonly AbstractGoOrFindNavigationService<TLanguageService> _instance; | ||
|
|
||
| internal TestAccessor(AbstractGoOrFindCommandHandler<TLanguageService, TCommandArgs> instance) | ||
| internal TestAccessor(AbstractGoOrFindNavigationService<TLanguageService> instance) | ||
| => _instance = instance; | ||
|
|
||
| internal ref Func<CancellationToken, Task>? DelayHook | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.ComponentModel.Composition; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Editor; | ||
| using Microsoft.CodeAnalysis.GoOrFind; | ||
| using Microsoft.VisualStudio.Commanding; | ||
| using Microsoft.VisualStudio.Text.Editor.Commanding.Commands; | ||
| using Microsoft.VisualStudio.Utilities; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.FindReferences; | ||
|
|
||
| [Export(typeof(ICommandHandler))] | ||
| [ContentType(ContentTypeNames.RoslynContentType)] | ||
| [Name(PredefinedCommandHandlerNames.FindReferences)] | ||
| [method: ImportingConstructor] | ||
| [method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")] | ||
| internal sealed class FindReferencesCommandHandler(FindReferencesNavigationService navigationService) | ||
| : AbstractGoOrFindCommandHandler<FindReferencesCommandArgs>(navigationService); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the command handler is now trivial. it just pulls in the appropriate service and defers to it. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.ComponentModel.Composition; | ||
| using Microsoft.CodeAnalysis.Editor; | ||
| using Microsoft.CodeAnalysis.GoOrFind; | ||
| using Microsoft.CodeAnalysis.Host.Mef; | ||
| using Microsoft.VisualStudio.Text.Editor.Commanding.Commands; | ||
| using Microsoft.VisualStudio.Utilities; | ||
| using VSCommanding = Microsoft.VisualStudio.Commanding; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.GoToBase; | ||
|
|
||
| [Export(typeof(VSCommanding.ICommandHandler))] | ||
| [ContentType(ContentTypeNames.RoslynContentType)] | ||
| [Name(PredefinedCommandHandlerNames.GoToBase)] | ||
| [method: ImportingConstructor] | ||
| [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| internal sealed class GoToBaseCommandHandler(GoToBaseNavigationService navigationService) | ||
| : AbstractGoOrFindCommandHandler<GoToBaseCommandArgs>(navigationService); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.ComponentModel.Composition; | ||
| using Microsoft.CodeAnalysis.Editor; | ||
| using Microsoft.CodeAnalysis.Editor.Commanding.Commands; | ||
| using Microsoft.CodeAnalysis.GoOrFind; | ||
| using Microsoft.CodeAnalysis.GoToDefinition; | ||
| using Microsoft.CodeAnalysis.Host.Mef; | ||
| using Microsoft.VisualStudio.Commanding; | ||
| using Microsoft.VisualStudio.Utilities; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.GoToImplementation; | ||
|
|
||
| [Export(typeof(ICommandHandler))] | ||
| [ContentType(ContentTypeNames.RoslynContentType)] | ||
| [Name(PredefinedCommandHandlerNames.GoToImplementation)] | ||
| [method: ImportingConstructor] | ||
| [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| internal sealed class GoToImplementationCommandHandler(GoToImplementationNavigationService navigationService) | ||
| : AbstractGoOrFindCommandHandler<GoToImplementationCommandArgs>(navigationService); |
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.
base command handler for GoToBase/GoToImpl/FindRefs.
defers most operations to the provided IGoOrFindNavigationService passed in.
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 type basically jsut bridges from from the ICommandHandler/TCommandArgs world to the core roslyn world.