CmdPal: Replace localized strings used as setting values in WebSearch extension with literals (#41331)

## Summary of the Pull Request

For WebSearch extension:

- Replaces localized string identifiers with invariant literal keys to
ensure stable and consistent setting values, avoiding issues when
switching cultures or if display strings change.
- Renames the `ShowHistory` property to `HistoryItemCount`.  
- Changes the type from `string` to `int` and centralizes parsing logic
in `SettingsManager`.
- Retains backward compatibility by preserving the legacy settings key
`"ShowHistory"` in `SettingsManager`.


<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] Closes: #40547 
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [x] **Tests:** Added/updated and all pass
- [x] **Localization:** All end-user-facing strings can be localized
- [x] **Dev docs:** none
- [x] **New binaries:** none
- [x] **Documentation updated:** none

<!-- 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:
Jiří Polášek
2025-09-02 13:16:54 +02:00
committed by GitHub
parent c5c6a3658f
commit 95b19739f6
8 changed files with 42 additions and 41 deletions

View File

@@ -15,13 +15,13 @@ public class MockSettingsInterface : ISettingsInterface
public bool GlobalIfURI { get; set; } public bool GlobalIfURI { get; set; }
public string ShowHistory { get; set; } public uint HistoryItemCount { get; set; }
public MockSettingsInterface(string showHistory = "none", bool globalIfUri = true, List<HistoryItem> mockHistory = null) public MockSettingsInterface(uint historyItemCount = 0, bool globalIfUri = true, List<HistoryItem> mockHistory = null)
{ {
_historyItems = mockHistory ?? new List<HistoryItem>(); _historyItems = mockHistory ?? new List<HistoryItem>();
GlobalIfURI = globalIfUri; GlobalIfURI = globalIfUri;
ShowHistory = showHistory; HistoryItemCount = historyItemCount;
} }
public List<ListItem> LoadHistory() public List<ListItem> LoadHistory()
@@ -50,9 +50,9 @@ public class MockSettingsInterface : ISettingsInterface
_historyItems.Add(historyItem); _historyItems.Add(historyItem);
// Simulate the same logic as SettingsManager // Simulate the same logic as SettingsManager
if (int.TryParse(ShowHistory, out var maxHistoryItems) && maxHistoryItems > 0) if (HistoryItemCount > 0)
{ {
while (_historyItems.Count > maxHistoryItems) while (_historyItems.Count > HistoryItemCount)
{ {
_historyItems.RemoveAt(0); // Remove the oldest item _historyItems.RemoveAt(0); // Remove the oldest item
} }

View File

@@ -54,7 +54,7 @@ public class QueryTests : CommandPaletteUnitTestBase
new HistoryItem("another search", DateTime.Parse("2024-01-02 13:00:00", CultureInfo.CurrentCulture)), new HistoryItem("another search", DateTime.Parse("2024-01-02 13:00:00", CultureInfo.CurrentCulture)),
}; };
var settings = new MockSettingsInterface(mockHistory: mockHistoryItems, showHistory: "5"); var settings = new MockSettingsInterface(mockHistory: mockHistoryItems, historyItemCount: 5);
var page = new WebSearchListPage(settings); var page = new WebSearchListPage(settings);
@@ -89,7 +89,7 @@ public class QueryTests : CommandPaletteUnitTestBase
new HistoryItem("another search4", DateTime.Parse("2024-01-05 13:00:00", CultureInfo.CurrentCulture)), new HistoryItem("another search4", DateTime.Parse("2024-01-05 13:00:00", CultureInfo.CurrentCulture)),
}; };
var settings = new MockSettingsInterface(mockHistory: mockHistoryItems, showHistory: "5"); var settings = new MockSettingsInterface(mockHistory: mockHistoryItems, historyItemCount: 5);
var page = new WebSearchListPage(settings); var page = new WebSearchListPage(settings);
@@ -122,7 +122,7 @@ public class QueryTests : CommandPaletteUnitTestBase
new HistoryItem("another search5", DateTime.Parse("2024-01-06 13:00:00", CultureInfo.CurrentCulture)), new HistoryItem("another search5", DateTime.Parse("2024-01-06 13:00:00", CultureInfo.CurrentCulture)),
}; };
var settings = new MockSettingsInterface(mockHistory: mockHistoryItems, showHistory: "None"); var settings = new MockSettingsInterface(mockHistory: mockHistoryItems, historyItemCount: 0);
var page = new WebSearchListPage(settings); var page = new WebSearchListPage(settings);

View File

@@ -34,7 +34,7 @@ internal sealed partial class SearchWebCommand : InvokableCommand
return CommandResult.KeepOpen(); return CommandResult.KeepOpen();
} }
if (_settingsManager.ShowHistory != Resources.history_none) if (_settingsManager.HistoryItemCount != 0)
{ {
_settingsManager.SaveHistory(new HistoryItem(Arguments, DateTime.Now)); _settingsManager.SaveHistory(new HistoryItem(Arguments, DateTime.Now));
} }

View File

@@ -11,7 +11,7 @@ public interface ISettingsInterface
{ {
public bool GlobalIfURI { get; } public bool GlobalIfURI { get; }
public string ShowHistory { get; } public uint HistoryItemCount { get; }
public List<ListItem> LoadHistory(); public List<ListItem> LoadHistory();

View File

@@ -16,6 +16,7 @@ namespace Microsoft.CmdPal.Ext.WebSearch.Helpers;
public class SettingsManager : JsonSettingsManager, ISettingsInterface public class SettingsManager : JsonSettingsManager, ISettingsInterface
{ {
private const string HistoryItemCountLegacySettingsKey = "ShowHistory";
private readonly string _historyPath; private readonly string _historyPath;
private static readonly string _namespace = "websearch"; private static readonly string _namespace = "websearch";
@@ -24,11 +25,11 @@ public class SettingsManager : JsonSettingsManager, ISettingsInterface
private static readonly List<ChoiceSetSetting.Choice> _choices = private static readonly List<ChoiceSetSetting.Choice> _choices =
[ [
new ChoiceSetSetting.Choice(Resources.history_none, Resources.history_none), new ChoiceSetSetting.Choice(Resources.history_none, "None"),
new ChoiceSetSetting.Choice(Resources.history_1, Resources.history_1), new ChoiceSetSetting.Choice(Resources.history_1, "1"),
new ChoiceSetSetting.Choice(Resources.history_5, Resources.history_5), new ChoiceSetSetting.Choice(Resources.history_5, "5"),
new ChoiceSetSetting.Choice(Resources.history_10, Resources.history_10), new ChoiceSetSetting.Choice(Resources.history_10, "10"),
new ChoiceSetSetting.Choice(Resources.history_20, Resources.history_20), new ChoiceSetSetting.Choice(Resources.history_20, "20"),
]; ];
private readonly ToggleSetting _globalIfURI = new( private readonly ToggleSetting _globalIfURI = new(
@@ -37,15 +38,15 @@ public class SettingsManager : JsonSettingsManager, ISettingsInterface
Resources.plugin_global_if_uri, Resources.plugin_global_if_uri,
false); false);
private readonly ChoiceSetSetting _showHistory = new( private readonly ChoiceSetSetting _historyItemCount = new(
Namespaced(nameof(ShowHistory)), Namespaced(HistoryItemCountLegacySettingsKey),
Resources.plugin_show_history, Resources.plugin_history_item_count,
Resources.plugin_show_history, Resources.plugin_history_item_count,
_choices); _choices);
public bool GlobalIfURI => _globalIfURI.Value; public bool GlobalIfURI => _globalIfURI.Value;
public string ShowHistory => _showHistory.Value ?? string.Empty; public uint HistoryItemCount => uint.TryParse(_historyItemCount.Value, out var value) ? value : 0;
internal static string SettingsJsonPath() internal static string SettingsJsonPath()
{ {
@@ -90,11 +91,11 @@ public class SettingsManager : JsonSettingsManager, ISettingsInterface
// Add the new history item // Add the new history item
historyItems.Add(historyItem); historyItems.Add(historyItem);
// Determine the maximum number of items to keep based on ShowHistory // Determine the maximum number of items to keep based on HistoryItemCount
if (int.TryParse(ShowHistory, out var maxHistoryItems) && maxHistoryItems > 0) if (HistoryItemCount > 0)
{ {
// Keep only the most recent `maxHistoryItems` items // Keep only the most recent `maxHistoryItems` items
while (historyItems.Count > maxHistoryItems) while (historyItems.Count > HistoryItemCount)
{ {
historyItems.RemoveAt(0); // Remove the oldest item historyItems.RemoveAt(0); // Remove the oldest item
} }
@@ -150,7 +151,7 @@ public class SettingsManager : JsonSettingsManager, ISettingsInterface
_historyPath = HistoryStateJsonPath(); _historyPath = HistoryStateJsonPath();
Settings.Add(_globalIfURI); Settings.Add(_globalIfURI);
Settings.Add(_showHistory); Settings.Add(_historyItemCount);
// Load settings from file upon initialization // Load settings from file upon initialization
LoadSettings(); LoadSettings();
@@ -188,11 +189,11 @@ public class SettingsManager : JsonSettingsManager, ISettingsInterface
base.SaveSettings(); base.SaveSettings();
try try
{ {
if (ShowHistory == Resources.history_none) if (HistoryItemCount == 0)
{ {
ClearHistory(); ClearHistory();
} }
else if (int.TryParse(ShowHistory, out var maxHistoryItems) && maxHistoryItems > 0) else if (HistoryItemCount > 0)
{ {
// Trim the history file if there are more items than the new limit // Trim the history file if there are more items than the new limit
if (File.Exists(_historyPath)) if (File.Exists(_historyPath))
@@ -201,10 +202,10 @@ public class SettingsManager : JsonSettingsManager, ISettingsInterface
var historyItems = JsonSerializer.Deserialize<List<HistoryItem>>(existingContent, WebSearchJsonSerializationContext.Default.ListHistoryItem) ?? []; var historyItems = JsonSerializer.Deserialize<List<HistoryItem>>(existingContent, WebSearchJsonSerializationContext.Default.ListHistoryItem) ?? [];
// Check if trimming is needed // Check if trimming is needed
if (historyItems.Count > maxHistoryItems) if (historyItems.Count > HistoryItemCount)
{ {
// Trim the list to keep only the most recent `maxHistoryItems` items // Trim the list to keep only the most recent `HistoryItemCount` items
historyItems = historyItems.Skip(historyItems.Count - maxHistoryItems).ToList(); historyItems = historyItems.Skip((int)(historyItems.Count - HistoryItemCount)).ToList();
// Save the trimmed history back to the file // Save the trimmed history back to the file
var trimmedHistoryJson = JsonSerializer.Serialize(historyItems, WebSearchJsonSerializationContext.Default.ListHistoryItem); var trimmedHistoryJson = JsonSerializer.Serialize(historyItems, WebSearchJsonSerializationContext.Default.ListHistoryItem);

View File

@@ -33,7 +33,7 @@ internal sealed partial class WebSearchListPage : DynamicListPage
_allItems = []; _allItems = [];
Id = "com.microsoft.cmdpal.websearch"; Id = "com.microsoft.cmdpal.websearch";
_settingsManager = settingsManager; _settingsManager = settingsManager;
_historyItems = _settingsManager.ShowHistory != Resources.history_none ? _settingsManager.LoadHistory() : null; _historyItems = _settingsManager.HistoryItemCount != 0 ? _settingsManager.LoadHistory() : null;
if (_historyItems is not null) if (_historyItems is not null)
{ {
_allItems.AddRange(_historyItems); _allItems.AddRange(_historyItems);
@@ -57,7 +57,7 @@ internal sealed partial class WebSearchListPage : DynamicListPage
if (_historyItems is not null) if (_historyItems is not null)
{ {
filteredHistoryItems = _settingsManager.ShowHistory != Resources.history_none ? ListHelpers.FilterList(_historyItems, query).OfType<ListItem>() : null; filteredHistoryItems = _settingsManager.HistoryItemCount != 0 ? ListHelpers.FilterList(_historyItems, query).OfType<ListItem>() : null;
} }
var results = new List<ListItem>(); var results = new List<ListItem>();

View File

@@ -168,6 +168,15 @@ namespace Microsoft.CmdPal.Ext.WebSearch.Properties {
} }
} }
/// <summary>
/// Looks up a localized string similar to Determines the number of history items to show from previous searches.
/// </summary>
public static string plugin_history_item_count {
get {
return ResourceManager.GetString("plugin_history_item_count", resourceCulture);
}
}
/// <summary> /// <summary>
/// Looks up a localized string similar to In the default browser. /// Looks up a localized string similar to In the default browser.
/// </summary> /// </summary>
@@ -231,15 +240,6 @@ namespace Microsoft.CmdPal.Ext.WebSearch.Properties {
} }
} }
/// <summary>
/// Looks up a localized string similar to Determines the number of history items to show from previous searches.
/// </summary>
public static string plugin_show_history {
get {
return ResourceManager.GetString("plugin_show_history", resourceCulture);
}
}
/// <summary> /// <summary>
/// Looks up a localized string similar to Settings. /// Looks up a localized string similar to Settings.
/// </summary> /// </summary>

View File

@@ -172,7 +172,7 @@
<data name="plugin_search_failed" xml:space="preserve"> <data name="plugin_search_failed" xml:space="preserve">
<value>Failed to open {0}.</value> <value>Failed to open {0}.</value>
</data> </data>
<data name="plugin_show_history" xml:space="preserve"> <data name="plugin_history_item_count" xml:space="preserve">
<value>Determines the number of history items to show from previous searches</value> <value>Determines the number of history items to show from previous searches</value>
</data> </data>
<data name="settings_page_name" xml:space="preserve"> <data name="settings_page_name" xml:space="preserve">