From 9ae355b96359a2073d93a75ff87dbf2dd1ad56f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Thu, 26 Feb 2026 13:52:55 +0100 Subject: [PATCH] CmdPal: Fix All Apps search result limit default (#45804) ## Summary of the Pull Request This PR adds a new default option to the All Apps extension settings. - Adds an explicit "Default (n)" option to the choices list as the default. This option is not tied to a concrete number of items, allowing the value to change in the future. - Fixes backward compatibility for users who installed the extension when `"0"` was accidentally used as the default search result limit (d07f40eec3a). Existing `"0"` values are now treated as "use default (10)" instead of producing zero results. - Renames the intentional "0 results" setting value from `"0"` to `"none"` so it can be clearly distinguished from the previous accidental default. - Moves result-limit parsing from `AllAppsCommandProvider` into `AllAppsSettings`, exposing a parsed `int?` property instead of a raw string. - Adds `IgnoreUnknownValue` to `ChoiceSetSetting` so unrecognized stored values (such as the old `"0"`) are silently ignored on load, preserving the initialized default. - Without this the drop-down value in the adaptive card shows as empty. image ## 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 --- .../AllAppsCommandProvider.cs | 23 +------- .../AllAppsSettings.cs | 53 ++++++++++++++++--- .../Properties/Resources.Designer.cs | 11 +++- .../Properties/Resources.resx | 4 ++ .../ChoiceSetSetting.cs | 17 +++++- 5 files changed, 78 insertions(+), 30 deletions(-) diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs index a223dcab01..6d46a3bb7b 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs @@ -16,6 +16,7 @@ namespace Microsoft.CmdPal.Ext.Apps; public partial class AllAppsCommandProvider : CommandProvider { public const string WellKnownId = "AllApps"; + internal const int DefaultResultLimit = 10; public static readonly AllAppsPage Page = new(); @@ -44,27 +45,7 @@ public partial class AllAppsCommandProvider : CommandProvider PinnedAppsManager.Instance.PinStateChanged += OnPinStateChanged; } - public static int TopLevelResultLimit - { - get - { - var limitSetting = AllAppsSettings.Instance.SearchResultLimit; - - if (limitSetting is null) - { - return 10; - } - - var quantity = 10; - - if (int.TryParse(limitSetting, out var result)) - { - quantity = result < 0 ? quantity : result; - } - - return quantity; - } - } + public static int TopLevelResultLimit => AllAppsSettings.Instance.SearchResultLimit ?? DefaultResultLimit; public override ICommandItem[] TopLevelCommands() => [_listItem, .. _page.GetPinnedApps()]; diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsSettings.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsSettings.cs index bf326221f9..924285e1ef 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsSettings.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsSettings.cs @@ -4,7 +4,9 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; +using System.Text; using Microsoft.CmdPal.Ext.Apps.Helpers; using Microsoft.CmdPal.Ext.Apps.Programs; using Microsoft.CmdPal.Ext.Apps.Properties; @@ -14,16 +16,28 @@ namespace Microsoft.CmdPal.Ext.Apps; public class AllAppsSettings : JsonSettingsManager, ISettingsInterface { + // "none" instead of "0": the original default was accidentally "0", so existing + // users may have "0" stored. Using "none" lets us distinguish intentional "show + // no results" from the old accidental default (which is now treated as "use default"). + private const string NoneResultLimitValue = "none"; + + private static readonly CompositeFormat DefaultLimitItemTitleFormat = CompositeFormat.Parse(Resources.limit_default); + private static readonly string DefaultLimitItemTitle = string.Format( + CultureInfo.CurrentCulture, + DefaultLimitItemTitleFormat.Format, + AllAppsCommandProvider.DefaultResultLimit); + private static readonly string _namespace = "apps"; private static string Namespaced(string propertyName) => $"{_namespace}.{propertyName}"; private static readonly List _searchResultLimitChoices = [ - new ChoiceSetSetting.Choice(Resources.limit_0, "0"), - new ChoiceSetSetting.Choice(Resources.limit_1, "1"), - new ChoiceSetSetting.Choice(Resources.limit_5, "5"), - new ChoiceSetSetting.Choice(Resources.limit_10, "10"), + new(DefaultLimitItemTitle, "-1"), + new(Resources.limit_0, NoneResultLimitValue), + new(Resources.limit_1, "1"), + new(Resources.limit_5, "5"), + new(Resources.limit_10, "10"), ]; #pragma warning disable SA1401 // Fields should be private @@ -52,9 +66,36 @@ public class AllAppsSettings : JsonSettingsManager, ISettingsInterface Namespaced(nameof(SearchResultLimit)), Resources.limit_fallback_results_source, Resources.limit_fallback_results_source_description, - _searchResultLimitChoices); + _searchResultLimitChoices) + { + IgnoreUnknownValue = true, + }; - public string SearchResultLimit => _searchResultLimitSource.Value ?? string.Empty; + /// + /// Parsed search result limit. Returns when the caller should + /// use its own default (unrecognized value, empty, or old stored "0"). + /// + public int? SearchResultLimit + { + get + { + var raw = _searchResultLimitSource.Value ?? string.Empty; + + if (string.Equals(raw, NoneResultLimitValue, StringComparison.Ordinal)) + { + return 0; + } + + if (string.IsNullOrWhiteSpace(raw) + || !int.TryParse(raw, out var result) + || result <= 0) //// <= 0: treats old stored "0" as "use default" + { + return null; + } + + return result; + } + } private readonly ToggleSetting _enableStartMenuSource = new( Namespaced(nameof(EnableStartMenuSource)), diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.Designer.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.Designer.cs index 9fdf833b0a..8b16bdc2f5 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.Designer.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.Designer.cs @@ -19,7 +19,7 @@ namespace Microsoft.CmdPal.Ext.Apps.Properties { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "18.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { @@ -204,6 +204,15 @@ namespace Microsoft.CmdPal.Ext.Apps.Properties { } } + /// + /// Looks up a localized string similar to Default ({0}). + /// + internal static string limit_default { + get { + return ResourceManager.GetString("limit_default", resourceCulture); + } + } + /// /// Looks up a localized string similar to Limit the number of applications returned from the top level. /// diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.resx b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.resx index 758c61e20f..e7fdbb0013 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.resx +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Properties/Resources.resx @@ -234,4 +234,8 @@ Unlimited + + Default ({0}) + default option; {0} = number of items + \ No newline at end of file diff --git a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ChoiceSetSetting.cs b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ChoiceSetSetting.cs index 51beb0b5e7..f40b75a236 100644 --- a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ChoiceSetSetting.cs +++ b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ChoiceSetSetting.cs @@ -27,6 +27,8 @@ public sealed class ChoiceSetSetting : Setting public List Choices { get; set; } + public bool IgnoreUnknownValue { get; set; } + private ChoiceSetSetting() : base() { @@ -65,10 +67,21 @@ public sealed class ChoiceSetSetting : Setting public override void Update(JsonObject payload) { // If the key doesn't exist in the payload, don't do anything - if (payload[Key] is not null) + if (payload[Key] is null) { - Value = payload[Key]?.GetValue(); + return; } + + var value = payload[Key]?.GetValue(); + + // if value doesn't match any of the choices, reset to default if IgnoreUnknownValue is true; otherwise ignore the update + var valueIsValid = Choices.Any(choice => choice.Value == value); + if (!valueIsValid && IgnoreUnknownValue) + { + return; + } + + Value = value; } public override string ToState() => $"\"{Key}\": {JsonSerializer.Serialize(Value, JsonSerializationContext.Default.String)}";