mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-04-06 03:07:04 +02:00
ed47bceac281f5df7d98d2fde1048fa90101a9c6
11 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3f35b11cee |
CmdPal: Fix missing primary context command for late-bound items (#46131)
This PR does fix a bug where an item that starts with a null or empty primary command never adds that primary action to the context menu after the extension later provides a real command. - Creates the default primary context-menu item lazily when `Command` or `Command.Name` becomes available after `SlowInitializeProperties()` - Refreshes `AllCommands`, `SecondaryCommand`, and `HasMoreCommands` notifications for late command materialization and Show Details updates. - Adds unit tests to cover the fixed issue. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #46129 <!-- - [ ] 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 |
||
|
|
86115a54f6 |
CmdPal: Extract persistence services from SettingsModel and AppStateModel (#46312)
## Summary of the Pull Request Extracts persistence (load/save) logic from `SettingsModel` and `AppStateModel` into dedicated service classes, following the single-responsibility principle. Consumers now interact with `ISettingsService` and `IAppStateService` instead of receiving raw model objects through DI. **New services introduced:** - `IPersistenceService` / `PersistenceService` — generic `Load<T>` / `Save<T>` with AOT-compatible `JsonTypeInfo<T>`, ensures target directory exists before writing - `ISettingsService` / `SettingsService` — loads settings on construction, runs migrations, exposes `Settings` property and `SettingsChanged` event - `IAppStateService` / `AppStateService` — loads state on construction, exposes `State` property and `StateChanged` event **Key changes:** - `SettingsModel` and `AppStateModel` are now pure data models — all file I/O, migration, and directory management removed - Raw `SettingsModel` and `AppStateModel` removed from DI container; consumers receive the appropriate service - `IApplicationInfoService.ConfigDirectory` injected into services for config path resolution (no more hardcoded `Utilities.BaseSettingsPath`) - ~30 consumer files updated across `Microsoft.CmdPal.UI.ViewModels` and `Microsoft.CmdPal.UI` projects - All `#pragma warning disable SA1300` suppressions removed — convenience accessors replaced with direct `_settingsService.Settings` / `_appStateService.State` access - Namespace prefixes (`Services.ISettingsService`) replaced with proper `using` directives ## PR Checklist - [ ] **Communication:** I've discussed this with core contributors already. - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** N/A — no end-user-facing strings changed - [ ] **Dev docs:** N/A — internal refactor, no public API changes - [ ] **New binaries:** N/A — no new binaries introduced ## Detailed Description of the Pull Request / Additional comments ### Architecture Services are registered as singletons in `App.xaml.cs`: ```csharp services.AddSingleton<IPersistenceService, PersistenceService>(); services.AddSingleton<ISettingsService, SettingsService>(); services.AddSingleton<IAppStateService, AppStateService>(); ``` `PersistenceService.Save<T>` writes the serialized model directly to disk, creating the target directory if it doesn't exist. It also does not attempt to merge existing and new settings/state. `SettingsService` runs hotkey migrations on load and raises `SettingsChanged` after saves. `AppStateService` always raises `StateChanged` after saves. ### Files changed (41 files, +1169/−660) | Area | Files | What changed | |------|-------|-------------| | New services | `Services/IPersistenceService.cs`, `PersistenceService.cs`, `ISettingsService.cs`, `SettingsService.cs`, `IAppStateService.cs`, `AppStateService.cs` | New service interfaces and implementations | | Models | `SettingsModel.cs`, `AppStateModel.cs` | Stripped to pure data bags | | DI | `App.xaml.cs` | Service registration, removed raw model DI | | ViewModels | 12 files | Constructor injection of services | | UI | 10 files | Service injection replacing model access | | Settings | `DockSettings.cs` | `Colors.Transparent` replaced with struct literal to avoid WinUI3 COM dependency | | Tests | `PersistenceServiceTests.cs`, `SettingsServiceTests.cs`, `AppStateServiceTests.cs` | 38 unit tests covering all three services | | Config | `.gitignore` | Added `.squad/`, `.github/agents/` exclusions | ## Validation Steps Performed - Built `Microsoft.CmdPal.UI` with MSBuild (x64/Debug) — exit code 0, clean build - Ran 38 unit tests via `vstest.console.exe` — all passing - Verified no remaining `#pragma warning disable SA1300` blocks - Verified no remaining `Services.` namespace prefixes --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
90131e35d9 |
CmdPal: Prevent unsynchornized access to More commands (#46020)
## Summary of the Pull Request This PR fixes a crash caused by unsynchronized access to a context menu: - Fixes `System.ArgumentException: Destination array was not long enough` crash in `CommandItemViewModel.AllCommands` caused by `List<T>.AddRange()` racing with background `BuildAndInitMoreCommands` mutations - Replaces mutable `List<T>` public surfaces with immutable array snapshots protected by a `Lock`; writers hold the lock, mutate the backing list, then atomically publish new snapshots via `volatile` fields that readers access lock-free - Applies the same snapshot pattern to `ContentPageViewModel`, using a bundled `CommandSnapshot` object for atomic publication (since `PrimaryCommand` drives command invocation there, not just UI hints) - Narrows `IContextMenuContext.MoreCommands` and `AllCommands` from `List<T>`/`IEnumerable<T>` to `IReadOnlyList<T>` to prevent consumers from casting back and mutating - Moves `SafeCleanup()` calls outside locks in cleanup paths to avoid holding the lock during cross-process RPC calls <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #45975 <!-- - [ ] 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 |
||
|
|
169bfe3f04 |
CmdPal: Lightning-fast mode (#45764)
## Summary of the Pull Request
This PR unlocks lightning-fast mode for Command Palette:
- Hides visual and motion distractions when updating the result list:
- Ensures the first interactable result item is selected as early as
possible after the result list is updated, reducing flashing and
blinking caused by the selection highlight moving around.
- Removes the list item selection indicator animation (unfortunately by
removing the pill altogether for now) and prevents it from temporarily
appearing on other items as the selection moves.
- Adds a new "Results" section header above the home page results when
no other section is present.
- This ensures the first item on the home page has consistent visuals
and styling, preventing offsets and excessive visual changes when
elements are replaced in place.
- Improves update performance and container reuse:
- Fixes the `removed` output parameter in `ListHelper.UpdateInPlace` to
only include items that were actually removed (items that were merely
moved to a different position should not be reported as removed).
- Adds unit tests to prevent regression.
- Updates `ListHelper.UpdateInPlace` for `ObservableCollection` to use
`Move` instead of `Remove`/`Add`, and avoids `Clear` to prevent
`ListView` resets (which force recreation of all item containers and are
expensive).
- Adds a simple cache for list page item view models to reduce
unnecessary recreation during forward incremental search.
- `ListViewModel` and `FetchItems` have no notion of item lifetime or
incremental search phase, so the cache intentionally remains simple
rather than clever.
- Updates ListPage templates to make them a little lighter:
- Tag template uses OneTime, instead of OneWay - since Tag is immutable
- Replaces ItemsControl with ItemsRepeater for Tag list on list items
- Increases the debounce for showing the details pane and adds a
debounce for hiding it. This improves performance when browsing the list
and prevents the details pane animation from bouncing left and right
## Pictures? Moving!
https://github.com/user-attachments/assets/36428d20-cf46-4321-83c0-d94d6d4a2299
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
- [x] Closes: #44407
- [x] Closes: #45691
|
||
|
|
138c66c328 |
CmdPal: Removing Core projects (#45693)
Functionally, no differences. - Removed Core projects. - Core.Common => Microsoft.CmdPal.Common - Core.ViewModels => Microsoft.CmdPal.UI.ViewModels --------- Co-authored-by: Jiří Polášek <me@jiripolasek.com> |
||
|
|
fafb582ae2 |
Fix CmdPal apps extension ignoring the fallback results limit setting (#45716)
## Summary MainListPage hardcoded _appResultLimit = 10 instead of reading from AllAppsCommandProvider.TopLevelResultLimit, which correctly parses the user's SearchResultLimit setting. This meant changing the results limit in settings had no effect on the apps extension fallback results. ## Changes - `MainListPage.cs` — Replaced the hardcoded _appResultLimit = 10 field with a computed property AppResultLimit that delegates to AllAppsCommandProvider.TopLevelResultLimit. - `MainListPageResultFactoryTests.cs` — Added three regression tests: - `Merge_AppLimitOfOne_ReturnsOnlyTopApp` — verifies limit of 1 returns only the top app - `Merge_AppLimitOfZero_ReturnsNoApps` — verifies limit of 0 returns no apps - `Merge_AppLimitOfOne_WithOtherResults_AppsAreLimited` — verifies apps are limited even when mixed with other result types ## Validation - [X] Existing tests pass - [X] New regression tests cover edge cases for the appResultLimit parameter ## Linked Issues - Fixes #45654 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
7477b561a1 |
CmdPal: Add precomputed fuzzy string matching to Command Palette (#44090)
## Summary of the Pull Request This PR improves fuzzy matching in Command Palette by: - Precomputing normalized strings to enable faster comparisons - Reducing memory allocations during matching, effectively down to zero It also introduces several behavioral improvements: - Strips diacritics from the normalized search string to improve matching across languages - Suppresses the same-case bonus when the query consists entirely of lowercase characters -- reflecting typical user input patterns - Allows skipping word separators -- enabling queries like Power Point to match PowerPoint This implementation is currently kept internal and is used only on the home page. For other scenarios, the `FuzzyStringMatcher` from `Microsoft.CommandPalette.Extensions.Toolkit` is being improved instead. `PrecomputedFuzzyMatcher` offers up to a 100× performance improvement over the current `FuzzyStringMatcher`, and approximately 2–5× better performance compared to the improved version. The improvement might seem small, but it adds up and becomes quite noticeable when filtering the entire home page—whether the user starts a new search or changes the query non-incrementally (e.g., using backspace). <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #45226 - [x] Closes: #44066 - [ ] **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 |
||
|
|
740dbf5699 |
build(common): update project references to use $(RepoRoot) for paths (#44639)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Update project references across multiple projects to utilize `$(RepoRoot)` for paths, ensuring consistency and improving maintainability. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: N/A - [ ] **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 - [ ] **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 This change affects the following projects: - `src/common/ManagedCsWin32/ManagedCsWin32.csproj` - `src/common/Common.Search/Common.Search.csproj` - `src/common/AllExperiments/AllExperiments.csproj` - `src/modules/peek/Peek.Common/Peek.Common.csproj` - `src/common/UITestAutomation/UITestAutomation.csproj` - `src/common/GPOWrapperProjection/GPOWrapperProjection.csproj` - `src/modules/powerrename/PowerRenameUITest/PowerRename.UITests.csproj` - `src/common/LanguageModelProvider/LanguageModelProvider.csproj` - `src/modules/Hosts/Hosts.Tests/HostsEditor.UnitTests.csproj` - `tools/StylesReportTool/StylesReportTool.vcxproj` - `src/common/interop/interop-tests/Common.Interop.UnitTests.csproj` - `tools/MonitorReportTool/MonitorReportTool.vcxproj` - `src/common/ManagedTelemetry/Telemetry/ManagedTelemetry.csproj` - `src/modules/peek/Peek.FilePreviewer/Peek.FilePreviewer.csproj` - `src/settings-ui/Settings.UI.Controls/Settings.UI.Controls.csproj` - `src/common/Themes/Themes.vcxproj` - `src/common/COMUtils/COMUtils.vcxproj` - `src/modules/cmdpal/Tests/Microsoft.CmdPal.UITests/Microsoft.CmdPal.UITests.csproj` - `src/modules/imageresizer/tests/ImageResizer.UnitTests.csproj` The changes were validated by running existing unit tests, which all passed successfully. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Executed all unit tests related to the modified projects, confirming that all tests passed without issues. ``` |
||
|
|
f1e045751a |
CmdPal: Fallback ranking and global results (#43549)
> [!IMPORTANT] > For extension developers, this release includes a new required `string Id` property for `FallbackCommandItem`. While your existing extensions will continue to work, without this `Id` being set, your fallbacks will not display and will not be rankable. > Before this is released, you will want to prepare your extension fallbacks. > > As an example, we are naming our built-in extensions as: > - Calculator extension provider Id: `com.microsoft.cmdpal.builtin.calculator` > - Calculator extension fallback: `com.microsoft.cmdpal.builtin.calculator.fallback` > > While the content of the Id isn't important, what is important is that it is unique to your extension and fallback to avoid conflicting with other extensions. Now the good stuff: ## What the heck does it do!? ### The backstory In PowerToys 0.95, we released performance improvements to Command Palette. One of the many ways we improved its speed is by no longer ranking fallback commands with other "top level" commands. Instead, all fallbacks would surface at the bottom of the results and be listed in the order they were registered with Command Palette. But this was only a temporary solution until the work included in this pull request was ready. In reality, not all fallbacks were treated equally. We marked the calculator and run fallbacks as "special." Special fallbacks **were** ranked like top-level commands and allowed to surface to the top of the results. ### The new "hotness" This PR brings the power of fallback management back to the people. In the Command Palette settings, you, dear user, can specify what order you want fallbacks to display in at the bottom of the results. This keeps those fallbacks unranked by Command Palette but displays them in an order that makes sense for you. But keep in mind, these will still live at the bottom of search results. But alas, we have also heard your cries that you'd like _some_ fallbacks to be ranked by Command Palette and surface to the top of the results. So, this PR allows you to mark any fallback as "special" by choosing to include them in the global results. Special (Global) fallbacks are treated like "top level" commands and appear in the search result based on their title & description. ### Screenshots/video <img width="1005" height="611" alt="image" src="https://github.com/user-attachments/assets/8ba5d861-f887-47ed-8552-ba78937322d2" /> <img width="1501" height="973" alt="image" src="https://github.com/user-attachments/assets/9edb7675-8084-4f14-8bdc-72d7d06d500e" /> <img width="706" height="744" alt="image" src="https://github.com/user-attachments/assets/81ae0252-b87d-4172-a5ea-4d3102134baf" /> <img width="666" height="786" alt="image" src="https://github.com/user-attachments/assets/acb76acf-531d-4e60-bb44-d1edeec77dce" /> ### GitHub issue maintenance details Closes #38312 Closes #38288 Closes #42524 Closes #41024 Closes #40351 Closes #41696 Closes #40193 --------- Co-authored-by: Niels Laute <niels.laute@live.nl> Co-authored-by: Jiří Polášek <me@jiripolasek.com> |
||
|
|
4710b816b4 |
[CmdPal] Optimise MainListPage's results display by merging already-sorted lists (#44126)
## Summary of the Pull Request This PR replaces the current LINQ-based results compilation query of combining, sorting and filtering the four result sources with a 3-way merge operation plus a final append. It provides a performance increase as well as a significant reduction in allocations. <!-- 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 - [x] **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 The existing code: 1. Limits the number of apps returned to a pre-defined maximum. 2. Sorts the apps list. 3. Appends filtered items, scored fallback items and the apps list together. 4. Sorts the three lists based on their score. 5. Appends the non-scored fallback items, with empty items excluded. 6. Selects just the `Item` from each. 7. Creates an array from the enumerable. ```csharp if (_filteredApps?.Count > 0) { limitedApps = _filteredApps.OrderByDescending(s => s.Score).Take(_appResultLimit).ToList(); } var items = Enumerable.Empty<Scored<IListItem>>() .Concat(_filteredItems is not null ? _filteredItems : []) .Concat(_scoredFallbackItems is not null ? _scoredFallbackItems : []) .Concat(limitedApps) .OrderByDescending(o => o.Score) // Add fallback items post-sort so they are always at the end of the list // and eventually ordered based on user preference .Concat(_fallbackItems is not null ? _fallbackItems.Where(w => !string.IsNullOrEmpty(w.Item.Title)) : []) .Select(s => s.Item) .ToArray(); ``` We can exploit the fact that each of the three 'scored' lists are pre-ordered, and replace the query with a 3-way merge and final append of the non-scored fallback items. By pre-sizing the results array we can avoid all the extra allocations of the LINQ-based solution. ### Proof of pre-ordering In `UpdateSearchText`, each of the lists is defined by calling `ListHelpers.FilterListWithScores`: ```csharp // Produce a list of everything that matches the current filter. _filteredItems = [.. ListHelpers.FilterListWithScores<IListItem>(newFilteredItems ?? [], SearchText, scoreItem)]; ``` ```csharp _scoredFallbackItems = ListHelpers.FilterListWithScores<IListItem>(newFallbacksForScoring ?? [], SearchText, scoreItem); ``` ```csharp var scoredApps = ListHelpers.FilterListWithScores<IListItem>(newApps, SearchText, scoreItem); ... _filteredApps = [.. scoredApps]; ``` In `FilterListWithScores`, the results are ordered by score: ```csharp var scores = items .Select(li => new Scored<T>() { Item = li, Score = scoreFunction(query, li) }) .Where(score => score.Score > 0) .OrderByDescending(score => score.Score); ``` (This also makes the existing `OrderByDescending()` for `_filteredApps` before the LINQ query redundant.) ### K-way merge Since the results are pre-sorted, we can do a direct merge in linear time. This is what the new `MainListPageResultFactory`'s `Create` achieves. As the lists may be different sizes, the routine does a 3-way merge, followed by a 2-way merge and a single list drain to finish. Each element is only visited once. ### Benchmarks A separate benchmark project is [here](https://github.com/daverayment/MainListBench), written with Benchmark.net. The project compares the current LINQ-based solution against: 1. An Array-based algorithm which pre-assigns a results array and still sorts the 3 scored sets of results. This shows a naive non-LINQ solution which is still _O(n log n)_ because of the sort. 2. The k-way merge, which is described above. _O(n)_ for both time and space complexity. 3. A heap merge algorithm, which uses a priority queue instead of tracking each of the lists separately. (This is _O(n log k)_ in terms of time complexity and _O(n + k)_ for space.) Care is taken to ensure stable sorting of items. When preparing the benchmark data, items with identical scores are assigned to confirm each algorithm performs identically to the LINQ `OrderBy` approach, which performs a stable sort. Results show that the merge performs best in terms of both runtime performance and allocations, sometimes by a significant margin. Compared to the LINQ approach, merge runs 400%+ faster and with at most ~20% of the allocations: <img width="1135" height="556" alt="image" src="https://github.com/user-attachments/assets/9f9d3932-1592-49d6-8a07-4ea3ba7a0cc5" /> <img width="1149" height="553" alt="image" src="https://github.com/user-attachments/assets/ae9e9e0a-b255-4c1a-af4b-e791dea80fa4" /> See here for all charts and raw stats from the run: https://docs.google.com/spreadsheets/d/1y2mmWe8dfpbLxF_eqPbEGvaItmqp6HLfSp-rw99hzWg/edit?usp=sharing ### Cons 1. Existing performance is not currently an issue. This could be seen as a premature optimisation. 2. The new code introduces an inherent contract between the results compilation routine and the lists, i.e. that they must be sorted. This PR was really for research and learning more about CmdPal (and a bit of algorithm practice because it's Advent of Code time), so please feel free to reject if you feel the cons outweigh the pros. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed - Added unit tests to exercise the new code, which confirm that the specific ordering is preserved, and the filtering and pre-trimming of the apps list is performed as before. - Existing non-UI unit tests run. NB: I _could not_ run any UI Tests on my system and just got an early bail-out each time. - Manual testing in (non-AOT) Release mode. |
||
|
|
f55c49e15b |
CmdPal: adjust frecency weighting (#42242)
In #41959 we changed the string matcher's weighting. It ended up giving us lower scores than before. That meant that the weighting from recent commands was far too heavy, and it was polluting the results. Basically any command that you'd run would be like, 30 characters of weight heavier than anything you haven't. This increases the weight of all string matches by 10x. That means something like `Command Prompt` will get a string matched weight of `100` instead of `10`. This balances better with the weighting from frecency (where the MRU command gets +35, then `+min(5*uses,35)`, for up to 70 points of weight) It also adds a bunch of tests here, to try and catch this again in the future. Closes #42158 |