From 523ba06c9572778768a377b554ba00bd34bf09ea Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Tue, 19 Nov 2024 10:25:31 -0800 Subject: [PATCH 01/11] Display copilot suggestions progress bar when preparing for request --- .../UI/SmartRename/SmartRenameViewModel.cs | 73 +++++++++++-------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index 73e39b7eab634..b1a8aec821c5c 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -38,6 +38,7 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi private bool _isDisposed; private TimeSpan AutomaticFetchDelay => _smartRenameSession.AutomaticFetchDelay; private Task _getSuggestionsTask = Task.CompletedTask; + private bool IsInPreparation = false; private TimeSpan _semanticContextDelay; private bool _semanticContextError; private bool _semanticContextUsed; @@ -52,7 +53,7 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi public bool HasSuggestions => _smartRenameSession.HasSuggestions; - public bool IsInProgress => _smartRenameSession.IsInProgress; + public bool IsInProgress => IsInPreparation || _smartRenameSession.IsInProgress; public string StatusMessage => _smartRenameSession.StatusMessage; @@ -167,47 +168,55 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) { - if (isAutomaticOnInitialization) + try { - await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) - .ConfigureAwait(false); - } + this.IsInPreparation = true; + if (isAutomaticOnInitialization) + { + await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) + .ConfigureAwait(false); + } - if (cancellationToken.IsCancellationRequested || _isDisposed) - { - return; - } + if (cancellationToken.IsCancellationRequested || _isDisposed) + { + return; + } - if (IsUsingSemanticContext) - { - var stopwatch = SharedStopwatch.StartNew(); - _semanticContextUsed = true; - var document = this.BaseViewModel.Session.TriggerDocument; - var smartRenameContext = ImmutableDictionary>.Empty; - try + if (IsUsingSemanticContext) { - var editorRenameService = document.GetRequiredLanguageService(); - var renameLocations = await this.BaseViewModel.Session.AllRenameLocationsTask.JoinAsync(cancellationToken) + var stopwatch = SharedStopwatch.StartNew(); + _semanticContextUsed = true; + var document = this.BaseViewModel.Session.TriggerDocument; + var smartRenameContext = ImmutableDictionary>.Empty; + try + { + var editorRenameService = document.GetRequiredLanguageService(); + var renameLocations = await this.BaseViewModel.Session.AllRenameLocationsTask.JoinAsync(cancellationToken) + .ConfigureAwait(false); + var context = await editorRenameService.GetRenameContextAsync(this.BaseViewModel.Session.RenameInfo, renameLocations, cancellationToken) + .ConfigureAwait(false); + smartRenameContext = ImmutableDictionary.CreateRange>( + context + .Select(n => new KeyValuePair>(n.Key, n.Value))); + _semanticContextDelay = stopwatch.Elapsed; + } + catch (Exception e) when (FatalError.ReportAndCatch(e, ErrorSeverity.Diagnostic)) + { + _semanticContextError = true; + // use empty smartRenameContext + } + _ = await _smartRenameSession.GetSuggestionsAsync(smartRenameContext, cancellationToken) .ConfigureAwait(false); - var context = await editorRenameService.GetRenameContextAsync(this.BaseViewModel.Session.RenameInfo, renameLocations, cancellationToken) - .ConfigureAwait(false); - smartRenameContext = ImmutableDictionary.CreateRange>( - context - .Select(n => new KeyValuePair>(n.Key, n.Value))); - _semanticContextDelay = stopwatch.Elapsed; } - catch (Exception e) when (FatalError.ReportAndCatch(e, ErrorSeverity.Diagnostic)) + else { - _semanticContextError = true; - // use empty smartRenameContext + _ = await _smartRenameSession.GetSuggestionsAsync(cancellationToken) + .ConfigureAwait(false); } - _ = await _smartRenameSession.GetSuggestionsAsync(smartRenameContext, cancellationToken) - .ConfigureAwait(false); } - else + finally { - _ = await _smartRenameSession.GetSuggestionsAsync(cancellationToken) - .ConfigureAwait(false); + this.IsInPreparation = false; } } From 02c0b5c618d23853299fb8cf0040425d569b87ff Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Wed, 20 Nov 2024 10:56:05 -0800 Subject: [PATCH 02/11] keyboard shortcut or button click to cancel ongoing request --- .../UI/SmartRename/SmartRenameViewModel.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index b1a8aec821c5c..64509e439ba39 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -171,6 +171,7 @@ private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, Can try { this.IsInPreparation = true; + NotifyPropertyChanged(nameof(IsInProgress)); if (isAutomaticOnInitialization) { await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) @@ -217,6 +218,7 @@ await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) finally { this.IsInPreparation = false; + NotifyPropertyChanged(nameof(IsInProgress)); } } @@ -297,8 +299,8 @@ public void Dispose() /// /// When smart rename operates in explicit mode, this method gets the suggestions. - /// When smart rename operates in automatic mode, this method toggles the automatic suggestions, - /// and gets the suggestions if it was just enabled. + /// When smart rename operates in automatic mode, this method toggles the automatic suggestions: + /// gets the suggestions if it was just enabled, and cancels the ongoing request if it was just disabled. /// public void ToggleOrTriggerSuggestions() { @@ -309,6 +311,10 @@ public void ToggleOrTriggerSuggestions() { this.FetchSuggestions(isAutomaticOnInitialization: false); } + else + { + _cancellationTokenSource?.Cancel(); + } NotifyPropertyChanged(nameof(IsSuggestionsPanelExpanded)); NotifyPropertyChanged(nameof(IsAutomaticSuggestionsEnabled)); // Use existing "CollapseSuggestionsPanel" option (true if user does not wish to get suggestions automatically) to honor user's choice. From bf7a20c439586724025946c80ef37f745ffd3ae7 Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Thu, 21 Nov 2024 10:46:41 -0800 Subject: [PATCH 03/11] NotifyPropertyChanged IsInProgress from IsInPreparation setter --- .../UI/SmartRename/SmartRenameViewModel.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index 64509e439ba39..038d5b2b7cede 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -38,7 +38,7 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi private bool _isDisposed; private TimeSpan AutomaticFetchDelay => _smartRenameSession.AutomaticFetchDelay; private Task _getSuggestionsTask = Task.CompletedTask; - private bool IsInPreparation = false; + private bool _isInPreparation = false; private TimeSpan _semanticContextDelay; private bool _semanticContextError; private bool _semanticContextUsed; @@ -53,6 +53,16 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi public bool HasSuggestions => _smartRenameSession.HasSuggestions; + public bool IsInPreparation + { + get => _isInPreparation; + set + { + _isInPreparation = value; + NotifyPropertyChanged(nameof(IsInProgress)); // UI is bound to IsInProgress + } + } + public bool IsInProgress => IsInPreparation || _smartRenameSession.IsInProgress; public string StatusMessage => _smartRenameSession.StatusMessage; @@ -171,7 +181,6 @@ private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, Can try { this.IsInPreparation = true; - NotifyPropertyChanged(nameof(IsInProgress)); if (isAutomaticOnInitialization) { await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) @@ -218,7 +227,6 @@ await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) finally { this.IsInPreparation = false; - NotifyPropertyChanged(nameof(IsInProgress)); } } From 6370ac995f7b5a3d3f547e77fc24d5374a8bbce8 Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Tue, 3 Dec 2024 11:27:18 -0800 Subject: [PATCH 04/11] PR feedback --- .../UI/SmartRename/SmartRenameViewModel.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index 038d5b2b7cede..ba877e7c34459 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -34,7 +34,7 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi private readonly IGlobalOptionService _globalOptionService; private readonly IThreadingContext _threadingContext; private readonly IAsynchronousOperationListener _asyncListener; - private CancellationTokenSource? _cancellationTokenSource; + private CancellationTokenSource _cancellationTokenSource = new(); private bool _isDisposed; private TimeSpan AutomaticFetchDelay => _smartRenameSession.AutomaticFetchDelay; private Task _getSuggestionsTask = Task.CompletedTask; @@ -170,7 +170,7 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) if (_getSuggestionsTask.Status is TaskStatus.RanToCompletion or TaskStatus.Faulted or TaskStatus.Canceled) { var listenerToken = _asyncListener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); - _cancellationTokenSource?.Dispose(); + _cancellationTokenSource.Dispose(); _cancellationTokenSource = new CancellationTokenSource(); _getSuggestionsTask = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } @@ -178,6 +178,8 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) { + _threadingContext.ThrowIfNotOnUIThread(); + RoslynDebug.Assert(!this.IsInPreparation); try { this.IsInPreparation = true; @@ -282,7 +284,7 @@ private async Task SessionPropertyChangedAsync(object sender, PropertyChangedEve public void Cancel() { - _cancellationTokenSource?.Cancel(); + _cancellationTokenSource.Cancel(); // It's needed by editor-side telemetry. _smartRenameSession.OnCancel(); PostTelemetry(isCommit: false); @@ -301,8 +303,8 @@ public void Dispose() _smartRenameSession.PropertyChanged -= SessionPropertyChanged; BaseViewModel.PropertyChanged -= BaseViewModelPropertyChanged; _smartRenameSession.Dispose(); - _cancellationTokenSource?.Cancel(); - _cancellationTokenSource?.Dispose(); + _cancellationTokenSource.Cancel(); + _cancellationTokenSource.Dispose(); } /// @@ -321,7 +323,7 @@ public void ToggleOrTriggerSuggestions() } else { - _cancellationTokenSource?.Cancel(); + _cancellationTokenSource.Cancel(); } NotifyPropertyChanged(nameof(IsSuggestionsPanelExpanded)); NotifyPropertyChanged(nameof(IsAutomaticSuggestionsEnabled)); From de74b78d53958f85e932396bb64888e54270a3a4 Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Tue, 3 Dec 2024 12:10:23 -0800 Subject: [PATCH 05/11] pr feedback --- .../InlineRename/UI/SmartRename/SmartRenameViewModel.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index ba877e7c34459..a1ee81fe99af2 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -178,8 +178,9 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) { - _threadingContext.ThrowIfNotOnUIThread(); RoslynDebug.Assert(!this.IsInPreparation); + await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); + try { this.IsInPreparation = true; @@ -228,6 +229,7 @@ await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) } finally { + await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(_threadingContext.DisposalToken); this.IsInPreparation = false; } } From f725e691d0e4c45f4a8db0acdaab53906ce3c051 Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Tue, 3 Dec 2024 16:22:11 -0800 Subject: [PATCH 06/11] simplify task and IsInProgress --- .../UI/SmartRename/SmartRenameViewModel.cs | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index a1ee81fe99af2..a9a8c9750dbfa 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -37,11 +37,15 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi private CancellationTokenSource _cancellationTokenSource = new(); private bool _isDisposed; private TimeSpan AutomaticFetchDelay => _smartRenameSession.AutomaticFetchDelay; - private Task _getSuggestionsTask = Task.CompletedTask; - private bool _isInPreparation = false; private TimeSpan _semanticContextDelay; private bool _semanticContextError; private bool _semanticContextUsed; + private Task? _getSuggestionsTask; + + /// + /// Backing field for . + /// + private bool _isInProgress = false; public event PropertyChangedEventHandler? PropertyChanged; @@ -53,18 +57,23 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi public bool HasSuggestions => _smartRenameSession.HasSuggestions; - public bool IsInPreparation + /// + /// Indicates whether a request to get suggestions is in progress. + /// The request to get suggestions is comprised of initial short delay, + /// and call to . + /// When true, the UI shows the progress bar, and prevents from making parallel request. + /// + public bool IsInProgress { - get => _isInPreparation; + get => _isInProgress; set { - _isInPreparation = value; - NotifyPropertyChanged(nameof(IsInProgress)); // UI is bound to IsInProgress + _threadingContext.ThrowIfNotOnUIThread(); + _isInProgress = value; + NotifyPropertyChanged(nameof(IsInProgress)); } } - public bool IsInProgress => IsInPreparation || _smartRenameSession.IsInProgress; - public string StatusMessage => _smartRenameSession.StatusMessage; public bool StatusMessageVisibility => _smartRenameSession.StatusMessageVisibility; @@ -161,29 +170,26 @@ public SmartRenameViewModel( private void FetchSuggestions(bool isAutomaticOnInitialization) { _threadingContext.ThrowIfNotOnUIThread(); - if (this.SuggestedNames.Count > 0 || _isDisposed) + if (this.SuggestedNames.Count > 0 || _isDisposed || this.IsInProgress) { // Don't get suggestions again return; } - if (_getSuggestionsTask.Status is TaskStatus.RanToCompletion or TaskStatus.Faulted or TaskStatus.Canceled) - { - var listenerToken = _asyncListener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); - _cancellationTokenSource.Dispose(); - _cancellationTokenSource = new CancellationTokenSource(); - _getSuggestionsTask = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); - } + var listenerToken = _asyncListener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); + _cancellationTokenSource.Dispose(); + _cancellationTokenSource = new CancellationTokenSource(); + _getSuggestionsTask = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) { - RoslynDebug.Assert(!this.IsInPreparation); + RoslynDebug.Assert(!this.IsInProgress); + this.IsInProgress = true; await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); try { - this.IsInPreparation = true; if (isAutomaticOnInitialization) { await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) @@ -230,7 +236,7 @@ await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) finally { await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(_threadingContext.DisposalToken); - this.IsInPreparation = false; + this.IsInProgress = false; } } From b61a737ef89679bed9c0bce8d432efcb02ccaa2b Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Tue, 3 Dec 2024 16:55:16 -0800 Subject: [PATCH 07/11] doc comment --- .../InlineRename/UI/SmartRename/SmartRenameViewModel.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index a9a8c9750dbfa..1d79afe244a47 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -34,13 +34,17 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi private readonly IGlobalOptionService _globalOptionService; private readonly IThreadingContext _threadingContext; private readonly IAsynchronousOperationListener _asyncListener; + + /// + /// Cancellation token source for . + /// Each call uses a new instance. Mutliple calls are allowed only if previous call failed or was canceled. + /// private CancellationTokenSource _cancellationTokenSource = new(); private bool _isDisposed; private TimeSpan AutomaticFetchDelay => _smartRenameSession.AutomaticFetchDelay; private TimeSpan _semanticContextDelay; private bool _semanticContextError; private bool _semanticContextUsed; - private Task? _getSuggestionsTask; /// /// Backing field for . @@ -179,7 +183,7 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) var listenerToken = _asyncListener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); _cancellationTokenSource.Dispose(); _cancellationTokenSource = new CancellationTokenSource(); - _getSuggestionsTask = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); + _ = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) From 2d29d0c9368914be4397af986c3c6c7f6e2b951c Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Fri, 6 Dec 2024 11:16:58 -0800 Subject: [PATCH 08/11] documentation --- .../UI/SmartRename/SmartRenameViewModel.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index 1d79afe244a47..0688392042f1d 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -38,6 +38,10 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi /// /// Cancellation token source for . /// Each call uses a new instance. Mutliple calls are allowed only if previous call failed or was canceled. + /// The request is canceled through + /// 1. when user types in the text box. + /// 2. when user toggles the automatic suggestions. + /// 3. when the dialog is closed. /// private CancellationTokenSource _cancellationTokenSource = new(); private bool _isDisposed; @@ -186,6 +190,12 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) _ = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } + /// + /// The request for rename suggestions. It's made of three parts: + /// 1. Short delay of duration . + /// 2. Get definition and references if is set. + /// 3. Call to . + /// private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) { RoslynDebug.Assert(!this.IsInProgress); @@ -355,6 +365,7 @@ private void BaseViewModelPropertyChanged(object sender, PropertyChangedEventArg { if (e.PropertyName == nameof(BaseViewModel.IdentifierText)) { + // User is typing the new identifier name, cancel the ongoing request to get suggestions. _cancellationTokenSource?.Cancel(); } } From cf8c6ab2936b614acc41f730e4ce3d9c484de9f7 Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Fri, 6 Dec 2024 13:11:37 -0800 Subject: [PATCH 09/11] add threading guarantees, and if possible cancel previous token on fetch --- .../UI/SmartRename/SmartRenameViewModel.cs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index 0688392042f1d..d3f230c5149c3 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -38,7 +38,7 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi /// /// Cancellation token source for . /// Each call uses a new instance. Mutliple calls are allowed only if previous call failed or was canceled. - /// The request is canceled through + /// The request is canceled on UI thread through one of the following user interactions: /// 1. when user types in the text box. /// 2. when user toggles the automatic suggestions. /// 3. when the dialog is closed. @@ -73,7 +73,11 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi /// public bool IsInProgress { - get => _isInProgress; + get + { + _threadingContext.ThrowIfNotOnUIThread(); + return _isInProgress; + } set { _threadingContext.ThrowIfNotOnUIThread(); @@ -185,9 +189,16 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) } var listenerToken = _asyncListener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); - _cancellationTokenSource.Dispose(); + try + { + _cancellationTokenSource.Cancel(); + } + finally + { + _cancellationTokenSource.Dispose(); + } _cancellationTokenSource = new CancellationTokenSource(); - _ = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); + GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } /// @@ -249,6 +260,7 @@ await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) } finally { + // cancellationToken might be already canceled. Fallback to the disposal token. await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(_threadingContext.DisposalToken); this.IsInProgress = false; } @@ -321,6 +333,7 @@ public void Commit(string finalIdentifierName) public void Dispose() { + _threadingContext.ThrowIfNotOnUIThread(); _isDisposed = true; _smartRenameSession.PropertyChanged -= SessionPropertyChanged; BaseViewModel.PropertyChanged -= BaseViewModelPropertyChanged; @@ -336,6 +349,7 @@ public void Dispose() /// public void ToggleOrTriggerSuggestions() { + _threadingContext.ThrowIfNotOnUIThread(); if (this.SupportsAutomaticSuggestions) { this.IsAutomaticSuggestionsEnabled = !this.IsAutomaticSuggestionsEnabled; @@ -363,10 +377,11 @@ private void NotifyPropertyChanged([CallerMemberName] string? name = null) private void BaseViewModelPropertyChanged(object sender, PropertyChangedEventArgs e) { + _threadingContext.ThrowIfNotOnUIThread(); if (e.PropertyName == nameof(BaseViewModel.IdentifierText)) { // User is typing the new identifier name, cancel the ongoing request to get suggestions. - _cancellationTokenSource?.Cancel(); + _cancellationTokenSource.Cancel(); } } } From 934eb0b79af85370b2709c84da6a28fb4ca080b5 Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Mon, 9 Dec 2024 11:04:24 -0800 Subject: [PATCH 10/11] don't dispose CTS, add guard to .Cancel --- .../UI/SmartRename/SmartRenameViewModel.cs | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index d3f230c5149c3..77a4c369d0ae5 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -193,10 +193,7 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) { _cancellationTokenSource.Cancel(); } - finally - { - _cancellationTokenSource.Dispose(); - } + catch { } _cancellationTokenSource = new CancellationTokenSource(); GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } @@ -318,7 +315,11 @@ private async Task SessionPropertyChangedAsync(object sender, PropertyChangedEve public void Cancel() { - _cancellationTokenSource.Cancel(); + try + { + _cancellationTokenSource.Cancel(); + } + catch { } // It's needed by editor-side telemetry. _smartRenameSession.OnCancel(); PostTelemetry(isCommit: false); @@ -338,8 +339,11 @@ public void Dispose() _smartRenameSession.PropertyChanged -= SessionPropertyChanged; BaseViewModel.PropertyChanged -= BaseViewModelPropertyChanged; _smartRenameSession.Dispose(); - _cancellationTokenSource.Cancel(); - _cancellationTokenSource.Dispose(); + try + { + _cancellationTokenSource.Cancel(); + } + catch { } } /// @@ -381,7 +385,11 @@ private void BaseViewModelPropertyChanged(object sender, PropertyChangedEventArg if (e.PropertyName == nameof(BaseViewModel.IdentifierText)) { // User is typing the new identifier name, cancel the ongoing request to get suggestions. - _cancellationTokenSource.Cancel(); + try + { + _cancellationTokenSource.Cancel(); + } + catch { } } } } From 7dffaa53f8f7f2c8dc6109d4073c71d2e781987f Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Mon, 9 Dec 2024 15:17:45 -0800 Subject: [PATCH 11/11] remove try-catch around CTS.Cancel --- .../UI/SmartRename/SmartRenameViewModel.cs | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index 77a4c369d0ae5..337c4373f3a2b 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -189,11 +189,7 @@ private void FetchSuggestions(bool isAutomaticOnInitialization) } var listenerToken = _asyncListener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); - try - { - _cancellationTokenSource.Cancel(); - } - catch { } + _cancellationTokenSource.Cancel(); _cancellationTokenSource = new CancellationTokenSource(); GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } @@ -315,11 +311,7 @@ private async Task SessionPropertyChangedAsync(object sender, PropertyChangedEve public void Cancel() { - try - { - _cancellationTokenSource.Cancel(); - } - catch { } + _cancellationTokenSource.Cancel(); // It's needed by editor-side telemetry. _smartRenameSession.OnCancel(); PostTelemetry(isCommit: false); @@ -339,11 +331,7 @@ public void Dispose() _smartRenameSession.PropertyChanged -= SessionPropertyChanged; BaseViewModel.PropertyChanged -= BaseViewModelPropertyChanged; _smartRenameSession.Dispose(); - try - { - _cancellationTokenSource.Cancel(); - } - catch { } + _cancellationTokenSource.Cancel(); } /// @@ -385,11 +373,7 @@ private void BaseViewModelPropertyChanged(object sender, PropertyChangedEventArg if (e.PropertyName == nameof(BaseViewModel.IdentifierText)) { // User is typing the new identifier name, cancel the ongoing request to get suggestions. - try - { - _cancellationTokenSource.Cancel(); - } - catch { } + _cancellationTokenSource.Cancel(); } } }