From 79d9b0e6675ad34815359ab3ca2285e368f13a55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Tue, 24 Mar 2026 20:51:04 +0100 Subject: [PATCH] CmdPal: Keep TimeDateExtensionPage simple and update every time (#46396) ## Summary of the Pull Request This PR updates Time & Date extension page to calculate current results every time. This breaks possible infinite loop. ## PR Checklist - [x] Closes: #46329 - [ ] **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 --- .../QueryTests.cs | 37 ++++++++++-- .../Pages/TimeDateExtensionPage.cs | 59 ++----------------- 2 files changed, 37 insertions(+), 59 deletions(-) diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/QueryTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/QueryTests.cs index cba7614044..80290283a8 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/QueryTests.cs +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/QueryTests.cs @@ -68,7 +68,7 @@ public class QueryTests : CommandPaletteUnitTestBase { var settings = new Settings(); var page = new TimeDateExtensionPage(settings); - page.UpdateSearchText(string.Empty, input); + page.SearchText = input; var resultLists = page.GetItems(); var result = Query(input, resultLists); @@ -117,7 +117,7 @@ public class QueryTests : CommandPaletteUnitTestBase { var settings = new Settings(); var page = new TimeDateExtensionPage(settings); - page.UpdateSearchText(string.Empty, input); + page.SearchText = input; var resultLists = page.GetItems(); var firstItem = resultLists.FirstOrDefault(); @@ -158,7 +158,7 @@ public class QueryTests : CommandPaletteUnitTestBase { var settings = new Settings(); var page = new TimeDateExtensionPage(settings); - page.UpdateSearchText(string.Empty, query); + page.SearchText = query; var results = page.GetItems(); // Assert @@ -176,7 +176,8 @@ public class QueryTests : CommandPaletteUnitTestBase { var settings = new Settings(); var page = new TimeDateExtensionPage(settings); - page.UpdateSearchText("abc", input); + page.SearchText = "abc"; + page.SearchText = input; var results = page.GetItems(); // Assert @@ -193,7 +194,7 @@ public class QueryTests : CommandPaletteUnitTestBase { var settings = new Settings(); var page = new TimeDateExtensionPage(settings); - page.UpdateSearchText(string.Empty, query); + page.SearchText = query; var resultsList = page.GetItems(); var results = Query(query, resultsList); @@ -211,7 +212,7 @@ public class QueryTests : CommandPaletteUnitTestBase { var settings = new Settings(); var page = new TimeDateExtensionPage(settings); - page.UpdateSearchText(string.Empty, query); + page.SearchText = query; var resultsList = page.GetItems(); // Assert @@ -219,4 +220,28 @@ public class QueryTests : CommandPaletteUnitTestBase var firstResult = resultsList.FirstOrDefault(); Assert.IsTrue(firstResult.Title.Contains(expectedResult, StringComparison.CurrentCulture), $"Delimiter query '{query}' result not match {expectedResult} current result {firstResult.Title}"); } + + [TestMethod] + public void UpdateSearchTextMatchesSearchTextSetter() + { + var settings = new Settings(); + var pageUsingSetter = new TimeDateExtensionPage(settings); + var pageUsingMethod = new TimeDateExtensionPage(settings); + const string query = "time::12:30:45"; + + pageUsingSetter.SearchText = query; + pageUsingMethod.UpdateSearchText(string.Empty, query); + + var setterResults = pageUsingSetter.GetItems(); + var methodResults = pageUsingMethod.GetItems(); + + Assert.AreEqual(setterResults.Length, methodResults.Length, "UpdateSearchText should produce the same number of results as setting SearchText."); + + var setterFirstItem = setterResults.FirstOrDefault(); + var methodFirstItem = methodResults.FirstOrDefault(); + Assert.IsNotNull(setterFirstItem); + Assert.IsNotNull(methodFirstItem); + Assert.AreEqual(setterFirstItem.Title, methodFirstItem.Title, "UpdateSearchText should keep the stored query in sync with SearchText."); + Assert.AreEqual(setterFirstItem.Subtitle, methodFirstItem.Subtitle, "UpdateSearchText should keep the stored query in sync with SearchText."); + } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Pages/TimeDateExtensionPage.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Pages/TimeDateExtensionPage.cs index 36eb39461f..bcd00f6dc1 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Pages/TimeDateExtensionPage.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Pages/TimeDateExtensionPage.cs @@ -3,9 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading; using Microsoft.CmdPal.Ext.TimeDate.Helpers; using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions.Toolkit; @@ -14,12 +11,7 @@ namespace Microsoft.CmdPal.Ext.TimeDate.Pages; internal sealed partial class TimeDateExtensionPage : DynamicListPage { - private readonly Lock _resultsLock = new(); - - private IList _results = new List(); - private bool _dataLoaded; - - private ISettingsInterface _settingsManager; + private readonly ISettingsInterface _settingsManager; public TimeDateExtensionPage(ISettingsInterface settingsManager) { @@ -33,39 +25,10 @@ internal sealed partial class TimeDateExtensionPage : DynamicListPage } public override IListItem[] GetItems() - { - ListItem[] results; - lock (_resultsLock) - { - if (_dataLoaded) - { - results = _results.ToArray(); - _dataLoaded = false; - return results; - } - } - - DoExecuteSearch(string.Empty); - - lock (_resultsLock) - { - results = _results.ToArray(); - _dataLoaded = false; - return results; - } - } - - public override void UpdateSearchText(string oldSearch, string newSearch) - { - DoExecuteSearch(newSearch); - } - - private void DoExecuteSearch(string query) { try { - var result = TimeDateCalculator.ExecuteSearch(_settingsManager, query); - UpdateResult(result); + return [.. TimeDateCalculator.ExecuteSearch(_settingsManager, SearchText)]; } catch (Exception) { @@ -74,23 +37,13 @@ internal sealed partial class TimeDateExtensionPage : DynamicListPage // So, we need to clean the result. // But in that time, empty result may cause exception. // So, we need to add at least on item to user. - var items = new List - { - ResultHelper.CreateInvalidInputErrorResult(), - }; - - UpdateResult(items); + return [ResultHelper.CreateInvalidInputErrorResult()]; } } - private void UpdateResult(IList result) + public override void UpdateSearchText(string oldSearch, string newSearch) { - lock (_resultsLock) - { - this._results = result; - _dataLoaded = true; - } - - RaiseItemsChanged(this._results.Count); + SetSearchNoUpdate(newSearch); + RaiseItemsChanged(-2); } }