mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-04-03 09:46:54 +02:00
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
(d07f40eec3). 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.
<img width="842" height="226" alt="image"
src="https://github.com/user-attachments/assets/ae79f173-2e81-43d8-9103-c548ff6e0dc1"
/>
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
- [ ] Closes: #xxx
<!-- - [ ] Closes: #yyy (add separate lines for additional resolved
issues) -->
- [ ] **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
<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
This commit is contained in:
@@ -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()];
|
||||
|
||||
|
||||
@@ -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<ChoiceSetSetting.Choice> _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;
|
||||
/// <summary>
|
||||
/// Parsed search result limit. Returns <see langword="null"/> when the caller should
|
||||
/// use its own default (unrecognized value, empty, or old stored "0").
|
||||
/// </summary>
|
||||
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)),
|
||||
|
||||
@@ -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 {
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Looks up a localized string similar to Default ({0}).
|
||||
/// </summary>
|
||||
internal static string limit_default {
|
||||
get {
|
||||
return ResourceManager.GetString("limit_default", resourceCulture);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Looks up a localized string similar to Limit the number of applications returned from the top level.
|
||||
/// </summary>
|
||||
|
||||
@@ -234,4 +234,8 @@
|
||||
<data name="limit_none" xml:space="preserve">
|
||||
<value>Unlimited</value>
|
||||
</data>
|
||||
<data name="limit_default" xml:space="preserve">
|
||||
<value>Default ({0})</value>
|
||||
<comment>default option; {0} = number of items</comment>
|
||||
</data>
|
||||
</root>
|
||||
@@ -27,6 +27,8 @@ public sealed class ChoiceSetSetting : Setting<string>
|
||||
|
||||
public List<Choice> Choices { get; set; }
|
||||
|
||||
public bool IgnoreUnknownValue { get; set; }
|
||||
|
||||
private ChoiceSetSetting()
|
||||
: base()
|
||||
{
|
||||
@@ -65,10 +67,21 @@ public sealed class ChoiceSetSetting : Setting<string>
|
||||
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<string>();
|
||||
return;
|
||||
}
|
||||
|
||||
var value = payload[Key]?.GetValue<string>();
|
||||
|
||||
// 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)}";
|
||||
|
||||
Reference in New Issue
Block a user