From 82dc4cdc184b2cea7faec9093e354a0b161dfa1b Mon Sep 17 00:00:00 2001 From: moooyo <42196638+moooyo@users.noreply.github.com> Date: Sat, 25 Oct 2025 03:28:30 +0800 Subject: [PATCH] [CmdPal] Replace complex cancellation token mechanism with a simple task queue. (#42356) ## Summary of the Pull Request Just consider user are trying to search a long name such as "Visual Studio Code" The old mechanism: User input: V Task: V Then input: i Task cancel for V and start task i etc... The problem is: 1. I don't think we can really cancel the most time-cost part (Find packages from WinGet). 2. User cannot see anything before they really end the input and the last task complete. UX exp is so bad. 3. It's so complex to maintain. Hard to understand for the new contributor. New mechanism: User input: V Task: V Then input: i Prev Task is still running but mark the next task is i Input: s Prev Task is still running but override the next task to s etc... We can get: 1. User can see some results if prev task complete. 2. It's simple to understand 3. The extra time cost I think will not too much. Because we ignored the middle input. Compare: https://github.com/user-attachments/assets/f45f4073-efab-4f43-87f0-f47b727f36dc ## PR Checklist - [ ] Closes: #xxx - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed Co-authored-by: Yu Leng --- .../Pages/WinGetExtensionPage.cs | 136 +++++++++--------- 1 file changed, 64 insertions(+), 72 deletions(-) diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WinGet/Pages/WinGetExtensionPage.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WinGet/Pages/WinGetExtensionPage.cs index 1d7758769a..e84802b8fa 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WinGet/Pages/WinGetExtensionPage.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WinGet/Pages/WinGetExtensionPage.cs @@ -28,9 +28,10 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable public bool HasTag => !string.IsNullOrEmpty(_tag); private readonly Lock _resultsLock = new(); + private readonly Lock _taskLock = new(); - private CancellationTokenSource? _cancellationTokenSource; - private Task>? _currentSearchTask; + private string? _nextSearchQuery; + private bool _isTaskRunning; private List? _results; @@ -85,7 +86,6 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable stopwatch.Stop(); Logger.LogDebug($"Building ListItems took {stopwatch.ElapsedMilliseconds}ms", memberName: nameof(GetItems)); - IsLoading = false; return results; } } @@ -98,8 +98,6 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable Properties.Resources.winget_no_packages_found, }; - IsLoading = false; - return []; } @@ -117,64 +115,70 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable private void DoUpdateSearchText(string newSearch) { - // Cancel any ongoing search - if (_cancellationTokenSource is not null) + lock (_taskLock) { - Logger.LogDebug("Cancelling old search", memberName: nameof(DoUpdateSearchText)); - _cancellationTokenSource.Cancel(); - } - - _cancellationTokenSource = new CancellationTokenSource(); - - var cancellationToken = _cancellationTokenSource.Token; - - IsLoading = true; - - try - { - // Save the latest search task - _currentSearchTask = DoSearchAsync(newSearch, cancellationToken); - } - catch (OperationCanceledException) - { - // DO NOTHING HERE - return; - } - catch (Exception ex) - { - // Handle other exceptions - ExtensionHost.LogMessage($"[WinGet] DoUpdateSearchText throw exception: {ex.Message}"); - return; - } - - // Await the task to ensure only the latest one gets processed - _ = ProcessSearchResultsAsync(_currentSearchTask, newSearch); - } - - private async Task ProcessSearchResultsAsync( - Task> searchTask, - string newSearch) - { - try - { - var results = await searchTask; - - // Ensure this is still the latest task - if (_currentSearchTask == searchTask) + if (_isTaskRunning) { - // Process the results (e.g., update UI) - UpdateWithResults(results, newSearch); + // If a task is running, queue the next search query + // Keep IsLoading = true since we still have work to do + Logger.LogDebug($"Task is running, queueing next search: '{newSearch}'", memberName: nameof(DoUpdateSearchText)); + _nextSearchQuery = newSearch; + } + else + { + // No task is running, start a new search + Logger.LogDebug($"Starting new search: '{newSearch}'", memberName: nameof(DoUpdateSearchText)); + _isTaskRunning = true; + _nextSearchQuery = null; + IsLoading = true; + + _ = ExecuteSearchChainAsync(newSearch); } } - catch (OperationCanceledException) + } + + private async Task ExecuteSearchChainAsync(string query) + { + while (true) { - // Handle cancellation gracefully (e.g., log or ignore) - Logger.LogDebug($" Cancelled search for '{newSearch}'"); - } - catch (Exception ex) - { - // Handle other exceptions - Logger.LogError("Unexpected error while processing results", ex); + try + { + Logger.LogDebug($"Executing search for '{query}'", memberName: nameof(ExecuteSearchChainAsync)); + + var results = await DoSearchAsync(query); + + // Update UI with results + UpdateWithResults(results, query); + } + catch (Exception ex) + { + Logger.LogError($"Unexpected error while searching for '{query}'", ex); + } + + // Check if there's a next query to process + string? nextQuery; + lock (_taskLock) + { + if (_nextSearchQuery is not null) + { + // There's a queued search, execute it + nextQuery = _nextSearchQuery; + _nextSearchQuery = null; + + Logger.LogDebug($"Found queued search, continuing with: '{nextQuery}'", memberName: nameof(ExecuteSearchChainAsync)); + } + else + { + // No more searches queued, mark task as completed + _isTaskRunning = false; + IsLoading = false; + Logger.LogDebug("No more queued searches, task chain completed", memberName: nameof(ExecuteSearchChainAsync)); + break; + } + } + + // Continue with the next query + query = nextQuery; } } @@ -189,11 +193,8 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable RaiseItemsChanged(); } - private async Task> DoSearchAsync(string query, CancellationToken ct) + private async Task> DoSearchAsync(string query) { - // Were we already canceled? - ct.ThrowIfCancellationRequested(); - Stopwatch stopwatch = new(); stopwatch.Start(); @@ -230,9 +231,6 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable opts.Filters.Add(tagFilter); } - // Clean up here, then... - ct.ThrowIfCancellationRequested(); - var catalogTask = HasTag ? WinGetStatics.CompositeWingetCatalog : WinGetStatics.CompositeAllCatalog; // Both these catalogs should have been instantiated by the @@ -251,13 +249,11 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable findPackages_stopwatch.Start(); Logger.LogDebug($" Searching {catalog.Info.Name} ({query})", memberName: nameof(DoSearchAsync)); - ct.ThrowIfCancellationRequested(); - Logger.LogDebug($"Preface for \"{searchDebugText}\" took {stopwatch.ElapsedMilliseconds}ms", memberName: nameof(DoSearchAsync)); // BODGY, re: microsoft/winget-cli#5151 // FindPackagesAsync isn't actually async. - var internalSearchTask = Task.Run(() => catalog.FindPackages(opts), ct); + var internalSearchTask = Task.Run(() => catalog.FindPackages(opts)); var searchResults = await internalSearchTask; findPackages_stopwatch.Stop(); @@ -271,8 +267,6 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable return []; } - ct.ThrowIfCancellationRequested(); - Logger.LogDebug($" got results for ({query})", memberName: nameof(DoSearchAsync)); // FYI Using .ToArray or any other kind of enumerable loop @@ -282,8 +276,6 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable { var match = searchResults.Matches[i]; - ct.ThrowIfCancellationRequested(); - var package = match.CatalogPackage; results.Add(package); }