From 26e415956784ae77aa56a3e1e9b8c082c2d03269 Mon Sep 17 00:00:00 2001 From: Michael Jolley Date: Sat, 20 Jun 2026 22:05:32 -0500 Subject: [PATCH 1/3] CmdPal: Add extension reordering support (#38595) Add ability for users to reorder Command Palette extensions via a drag-and-drop dialog in the Extensions settings page. Changes: - Add ExtensionOrder string[] to SettingsModel for persisting user's preferred extension order - Sort extensions by configured order in TopLevelCommandManager when loading (ordered providers first, then remaining in load order) - Add ExtensionRanker and ExtensionRankerDialog controls (matching existing FallbackRanker pattern) with drag-and-drop reorder support - Add 'Manage extension order' menu item to Extensions page More flyout - Add ApplyExtensionOrder to SettingsViewModel to persist and reload - Add unit tests for ExtensionOrder serialization roundtrip Fixes #38595 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SettingsModel.cs | 10 +++ .../SettingsViewModel.cs | 27 ++++++ .../TopLevelCommandManager.cs | 88 +++++++++++++++++++ .../Controls/ExtensionRanker.xaml | 70 +++++++++++++++ .../Controls/ExtensionRanker.xaml.cs | 31 +++++++ .../Controls/ExtensionRankerDialog.xaml | 43 +++++++++ .../Controls/ExtensionRankerDialog.xaml.cs | 21 +++++ .../Settings/ExtensionsPage.xaml | 6 ++ .../Settings/ExtensionsPage.xaml.cs | 12 +++ .../Strings/en-us/Resources.resw | 9 ++ .../ExtensionOrderTests.cs | 60 +++++++++++++ 11 files changed, 377 insertions(+) create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsModel.cs index 86a183411855..10396174ecaa 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsModel.cs @@ -59,6 +59,14 @@ public string[] FallbackRanks init => _fallbackRanks = value; } + private string[]? _extensionOrder = []; + + public string[] ExtensionOrder + { + get => _extensionOrder ?? []; + init => _extensionOrder = value; + } + private ImmutableDictionary? _aliases = ImmutableDictionary.Empty; @@ -145,12 +153,14 @@ public SettingsModel( ImmutableList? pinnedCommands = null, ImmutableDictionary? providerSettings = null, string[]? fallbackRanks = null, + string[]? extensionOrder = null, ImmutableDictionary? aliases = null, ImmutableList? commandHotkeys = null) { PinnedCommands = pinnedCommands ?? ImmutableList.Empty; ProviderSettings = providerSettings ?? ImmutableDictionary.Empty; FallbackRanks = fallbackRanks ?? []; + ExtensionOrder = extensionOrder ?? []; Aliases = aliases ?? ImmutableDictionary.Empty; CommandHotkeys = commandHotkeys ?? ImmutableList.Empty; } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsViewModel.cs index 51faf86640a3..3bae86aea608 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsViewModel.cs @@ -253,6 +253,8 @@ public bool EnableDock public ObservableCollection FallbackRankings { get; set; } = new(); + public ObservableCollection ExtensionOrderRankings { get; set; } = new(); + public ObservableCollection MonitorConfigs { get; } = new(); public SettingsExtensionsViewModel Extensions { get; } @@ -322,6 +324,24 @@ public SettingsViewModel( } FallbackRankings = new ObservableCollection(fallbackRankings.OrderBy(o => o.Score).Select(fr => fr.Item)); + + // Build extension order rankings: providers in ExtensionOrder first (in order), then the rest + var currentExtensionOrder = _settingsService.Settings.ExtensionOrder; + var extensionOrderLookup = new Dictionary(currentExtensionOrder.Length, StringComparer.Ordinal); + for (var i = 0; i < currentExtensionOrder.Length; i++) + { + extensionOrderLookup.TryAdd(currentExtensionOrder[i], i); + } + + var orderedProviders = new List>(CommandProviders.Count); + foreach (var provider in CommandProviders) + { + var score = extensionOrderLookup.TryGetValue(provider.Id, out var idx) ? idx : CommandProviders.Count + orderedProviders.Count; + orderedProviders.Add(new Scored() { Item = provider, Score = score }); + } + + ExtensionOrderRankings = new ObservableCollection(orderedProviders.OrderBy(o => o.Score).Select(o => o.Item)); + Extensions = new SettingsExtensionsViewModel(CommandProviders, scheduler); if (needsSave) @@ -342,6 +362,13 @@ public void ApplyFallbackSort() PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(FallbackRankings))); } + public void ApplyExtensionOrder() + { + _settingsService.UpdateSettings(s => s with { ExtensionOrder = ExtensionOrderRankings.Select(p => p.Id).ToArray() }); + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(ExtensionOrderRankings))); + WeakReferenceMessenger.Default.Send(); + } + /// /// Builds or refreshes the collection by reconciling /// connected monitors with persisted per-monitor settings. diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs index 3f4dbc6d328e..b5e06308edb5 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs @@ -240,6 +240,14 @@ private async Task UpdateCommandsForProvider(CommandProviderWrapper sender, IIte var startIndex = FindIndexForFirstProviderItem(clone, sender.ProviderId); clone.RemoveAll(item => item.CommandProviderId == sender.ProviderId); + + if (startIndex >= clone.Count) + { + // Provider wasn't in the list yet — find position based on ExtensionOrder + var extensionOrder = _serviceProvider.GetRequiredService().Settings.ExtensionOrder; + startIndex = FindInsertIndexByExtensionOrder(clone, sender.ProviderId, extensionOrder); + } + clone.InsertRange(startIndex, newItems); ListHelpers.InPlaceUpdateList(TopLevelCommands, clone); @@ -446,6 +454,12 @@ private async Task RegisterAndLoadCommandsAsync(ICollect lock (TopLevelCommands) { + var extensionOrder = _serviceProvider.GetRequiredService().Settings.ExtensionOrder; + if (extensionOrder.Length > 0) + { + commandsToAdd = SortByExtensionOrder(commandsToAdd, extensionOrder); + } + foreach (var c in commandsToAdd) { TopLevelCommands.Add(c); @@ -473,6 +487,80 @@ private async Task RegisterAndLoadCommandsAsync(ICollect return new RegisterAndLoadSummary(totalCommands, totalDockBands); } + /// + /// Sorts commands so that providers listed in + /// appear first (in that order), followed by the remaining providers in their + /// original load order. + /// + internal static List SortByExtensionOrder(List commands, string[] extensionOrder) + { + var orderLookup = new Dictionary(extensionOrder.Length, StringComparer.Ordinal); + for (var i = 0; i < extensionOrder.Length; i++) + { + orderLookup.TryAdd(extensionOrder[i], i); + } + + // Stable sort: items whose provider is in the order list get a low key; + // items not in the list keep their relative order after the ordered ones. + var ordered = new List(commands.Count); + var unordered = new List(commands.Count); + + foreach (var cmd in commands) + { + if (orderLookup.ContainsKey(cmd.CommandProviderId)) + { + ordered.Add(cmd); + } + else + { + unordered.Add(cmd); + } + } + + ordered.Sort((a, b) => orderLookup[a.CommandProviderId].CompareTo(orderLookup[b.CommandProviderId])); + ordered.AddRange(unordered); + return ordered; + } + + /// + /// Determines where to insert a new provider's commands in the existing list + /// based on . If the provider is in the order + /// list, it's placed after other ordered providers that precede it. Otherwise + /// it goes at the end. + /// + private static int FindInsertIndexByExtensionOrder(List items, string providerId, string[] extensionOrder) + { + var providerIndex = Array.IndexOf(extensionOrder, providerId); + if (providerIndex < 0) + { + return items.Count; + } + + // Find the last item in the list whose provider has a lower order index + for (var i = items.Count - 1; i >= 0; i--) + { + var existingIndex = Array.IndexOf(extensionOrder, items[i].CommandProviderId); + if (existingIndex >= 0 && existingIndex < providerIndex) + { + // Insert after the last command of this earlier-ordered provider + var insertAfterProvider = items[i].CommandProviderId; + for (var j = i + 1; j < items.Count; j++) + { + if (items[j].CommandProviderId != insertAfterProvider) + { + return j; + } + } + + return i + 1; + } + } + + // This provider has the lowest order index among those present — insert at the beginning + // (after built-in commands which are already in the list) + return 0; + } + private async Task TryStartExtensionAsync(IExtensionWrapper extension) { Logger.LogDebug($"Starting {extension.PackageFullName}"); diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml new file mode 100644 index 000000000000..42d6dc122e5a --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml @@ -0,0 +1,70 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml.cs new file mode 100644 index 000000000000..a5a4cc02a125 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRanker.xaml.cs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.CmdPal.UI.ViewModels; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.UI.Xaml.Controls; + +namespace Microsoft.CmdPal.UI.Controls; + +public sealed partial class ExtensionRanker : UserControl +{ + private readonly TaskScheduler _mainTaskScheduler = TaskScheduler.FromCurrentSynchronizationContext(); + private SettingsViewModel? viewModel; + + public ExtensionRanker() + { + this.InitializeComponent(); + + var topLevelCommandManager = App.Current.Services.GetService()!; + var themeService = App.Current.Services.GetService()!; + var settingsService = App.Current.Services.GetRequiredService(); + viewModel = new SettingsViewModel(topLevelCommandManager, _mainTaskScheduler, themeService, settingsService); + } + + private void ListView_DragItemsCompleted(ListViewBase sender, DragItemsCompletedEventArgs args) + { + viewModel?.ApplyExtensionOrder(); + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml new file mode 100644 index 000000000000..a1b2face91cd --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml @@ -0,0 +1,43 @@ + + + + + + + + 800 + + + + + + + + + + + + + diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml.cs new file mode 100644 index 000000000000..59d177d48807 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ExtensionRankerDialog.xaml.cs @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.UI.Xaml.Controls; +using Windows.Foundation; + +namespace Microsoft.CmdPal.UI.Controls; + +public sealed partial class ExtensionRankerDialog : UserControl +{ + public ExtensionRankerDialog() + { + InitializeComponent(); + } + + public IAsyncOperation ShowAsync() + { + return ExtensionRankerContentDialog!.ShowAsync()!; + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml index c8b11a88586d..aacb284a3a71 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml @@ -178,6 +178,11 @@ + + + + + + diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs index 2fb660db3d32..4a91687f4482 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs @@ -93,4 +93,16 @@ private async void MenuFlyoutItem_OnClick(object sender, RoutedEventArgs e) Logger.LogError("Error when showing FallbackRankerDialog", ex); } } + + private async void ReorderExtensions_OnClick(object sender, RoutedEventArgs e) + { + try + { + await ExtensionRankerDialog!.ShowAsync(); + } + catch (Exception ex) + { + Logger.LogError("Error when showing ExtensionRankerDialog", ex); + } + } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw b/src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw index 74aefbb26a4b..aaaf06afb016 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw @@ -799,6 +799,15 @@ Right-click to remove the key combination, thereby deactivating the shortcut. Manage fallback order + + Manage extension order + + + Manage extension order + + + Drag items to set which extensions appear first in the top-level list; extensions at the top take priority. + Version {0} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs new file mode 100644 index 000000000000..dc94c29794a5 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Text.Json; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +[TestClass] +public class ExtensionOrderTests +{ + [TestMethod] + public void ExtensionOrder_DefaultIsEmpty() + { + var settings = DeserializeSettings("{}"); + Assert.IsNotNull(settings.ExtensionOrder); + Assert.AreEqual(0, settings.ExtensionOrder.Length); + } + + [TestMethod] + public void ExtensionOrder_RoundTrips() + { + var order = new[] { "provider.b", "provider.a", "provider.c" }; + var settings = DeserializeSettings("{}") with { ExtensionOrder = order }; + + var json = JsonSerializer.Serialize(settings, JsonSerializationContext.Default.SettingsModel); + var deserialized = JsonSerializer.Deserialize(json, JsonSerializationContext.Default.SettingsModel)!; + + CollectionAssert.AreEqual(order, deserialized.ExtensionOrder); + } + + [TestMethod] + public void ExtensionOrder_NullDeserializesToEmpty() + { + var json = """{"ExtensionOrder": null}"""; + var settings = JsonSerializer.Deserialize(json, JsonSerializationContext.Default.SettingsModel)!; + Assert.IsNotNull(settings.ExtensionOrder); + Assert.AreEqual(0, settings.ExtensionOrder.Length); + } + + [TestMethod] + public void ExtensionOrder_PreservesOrderInJson() + { + var order = new[] { "z.ext", "a.ext", "m.ext" }; + var settings = DeserializeSettings("{}") with { ExtensionOrder = order }; + + var json = JsonSerializer.Serialize(settings, JsonSerializationContext.Default.SettingsModel); + var deserialized = JsonSerializer.Deserialize(json, JsonSerializationContext.Default.SettingsModel)!; + + Assert.AreEqual("z.ext", deserialized.ExtensionOrder[0]); + Assert.AreEqual("a.ext", deserialized.ExtensionOrder[1]); + Assert.AreEqual("m.ext", deserialized.ExtensionOrder[2]); + } + + private static SettingsModel DeserializeSettings(string json) + { + return JsonSerializer.Deserialize(json, JsonSerializationContext.Default.SettingsModel) ?? new SettingsModel(); + } +} From 4a77658c795f392d0a3339fabfb6072cf145fbc2 Mon Sep 17 00:00:00 2001 From: Michael Jolley Date: Wed, 24 Jun 2026 16:18:07 -0500 Subject: [PATCH 2/3] CmdPal: Add sort logic tests and differentiate menu icon - Extract SortByExtensionOrder and FindInsertIndex into a generic ExtensionOrderHelper class so the logic can be unit tested without constructing heavyweight TopLevelViewModel instances. - Add 13 unit tests covering edge cases: empty lists, all/none/mixed ordering, duplicate provider IDs, and insertion index calculations. - Change the Reorder Extensions menu icon glyph from E8CB (same as Manage Fallback Order) to E174 to visually differentiate the two menu items. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ExtensionOrderHelper.cs | 83 ++++++++++ .../TopLevelCommandManager.cs | 72 +-------- .../Settings/ExtensionsPage.xaml | 2 +- .../ExtensionOrderTests.cs | 143 ++++++++++++++++++ 4 files changed, 229 insertions(+), 71 deletions(-) create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs new file mode 100644 index 000000000000..3e429d04075b --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.CmdPal.UI.ViewModels; + +/// +/// Pure helper methods for sorting and inserting items according to a user-defined +/// extension order. Operates on provider ID strings so the logic is easily testable +/// without constructing heavyweight view-model instances. +/// +internal static class ExtensionOrderHelper +{ + /// + /// Returns a new list with items sorted so that those whose provider is in + /// appear first (in that order), followed by + /// the remaining items in their original order. + /// + internal static List SortByExtensionOrder(List items, string[] extensionOrder, Func providerIdSelector) + { + var orderLookup = new Dictionary(extensionOrder.Length, StringComparer.Ordinal); + for (var i = 0; i < extensionOrder.Length; i++) + { + orderLookup.TryAdd(extensionOrder[i], i); + } + + var ordered = new List(items.Count); + var unordered = new List(items.Count); + + foreach (var item in items) + { + if (orderLookup.ContainsKey(providerIdSelector(item))) + { + ordered.Add(item); + } + else + { + unordered.Add(item); + } + } + + ordered.Sort((a, b) => orderLookup[providerIdSelector(a)].CompareTo(orderLookup[providerIdSelector(b)])); + ordered.AddRange(unordered); + return ordered; + } + + /// + /// Determines where to insert a new provider's items in the existing list based on + /// . If the provider is in the order list, it's + /// placed after other ordered providers that precede it. Otherwise it goes at the end. + /// + internal static int FindInsertIndex(List items, string providerId, string[] extensionOrder, Func providerIdSelector) + { + var providerIndex = Array.IndexOf(extensionOrder, providerId); + if (providerIndex < 0) + { + return items.Count; + } + + // Find the last item in the list whose provider has a lower order index + for (var i = items.Count - 1; i >= 0; i--) + { + var existingIndex = Array.IndexOf(extensionOrder, providerIdSelector(items[i])); + if (existingIndex >= 0 && existingIndex < providerIndex) + { + // Insert after the last command of this earlier-ordered provider + var insertAfterProvider = providerIdSelector(items[i]); + for (var j = i + 1; j < items.Count; j++) + { + if (providerIdSelector(items[j]) != insertAfterProvider) + { + return j; + } + } + + return i + 1; + } + } + + // This provider has the lowest order index among those present — insert at the beginning + return 0; + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs index a1514c94bbaa..88d4a5f485ac 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs @@ -509,79 +509,11 @@ private async Task RegisterAndLoadCommandsAsync(IEnumera return new RegisterAndLoadSummary(totalCommands, totalDockBands); } - /// - /// Sorts commands so that providers listed in - /// appear first (in that order), followed by the remaining providers in their - /// original load order. - /// internal static List SortByExtensionOrder(List commands, string[] extensionOrder) - { - var orderLookup = new Dictionary(extensionOrder.Length, StringComparer.Ordinal); - for (var i = 0; i < extensionOrder.Length; i++) - { - orderLookup.TryAdd(extensionOrder[i], i); - } + => ExtensionOrderHelper.SortByExtensionOrder(commands, extensionOrder, c => c.CommandProviderId); - // Stable sort: items whose provider is in the order list get a low key; - // items not in the list keep their relative order after the ordered ones. - var ordered = new List(commands.Count); - var unordered = new List(commands.Count); - - foreach (var cmd in commands) - { - if (orderLookup.ContainsKey(cmd.CommandProviderId)) - { - ordered.Add(cmd); - } - else - { - unordered.Add(cmd); - } - } - - ordered.Sort((a, b) => orderLookup[a.CommandProviderId].CompareTo(orderLookup[b.CommandProviderId])); - ordered.AddRange(unordered); - return ordered; - } - - /// - /// Determines where to insert a new provider's commands in the existing list - /// based on . If the provider is in the order - /// list, it's placed after other ordered providers that precede it. Otherwise - /// it goes at the end. - /// private static int FindInsertIndexByExtensionOrder(List items, string providerId, string[] extensionOrder) - { - var providerIndex = Array.IndexOf(extensionOrder, providerId); - if (providerIndex < 0) - { - return items.Count; - } - - // Find the last item in the list whose provider has a lower order index - for (var i = items.Count - 1; i >= 0; i--) - { - var existingIndex = Array.IndexOf(extensionOrder, items[i].CommandProviderId); - if (existingIndex >= 0 && existingIndex < providerIndex) - { - // Insert after the last command of this earlier-ordered provider - var insertAfterProvider = items[i].CommandProviderId; - for (var j = i + 1; j < items.Count; j++) - { - if (items[j].CommandProviderId != insertAfterProvider) - { - return j; - } - } - - return i + 1; - } - } - - // This provider has the lowest order index among those present — insert at the beginning - // (after built-in commands which are already in the list) - return 0; - } + => ExtensionOrderHelper.FindInsertIndex(items, providerId, extensionOrder, c => c.CommandProviderId); private async Task TryLoadCommandsAsync(CommandProviderWrapper wrapper, CancellationToken ct) { diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml index aacb284a3a71..4b21bc940ec2 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml @@ -180,7 +180,7 @@ - + diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs index dc94c29794a5..99c159490647 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs @@ -2,6 +2,7 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; using System.Text.Json; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -10,6 +11,13 @@ namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; [TestClass] public class ExtensionOrderTests { + private static readonly string[] OrderAB = ["a", "b"]; + private static readonly string[] OrderABC = ["a", "b", "c"]; + private static readonly string[] OrderXY = ["x", "y"]; + private static readonly string[] ExpectedXYZ = ["x", "y", "z"]; + private static readonly string[] ExpectedABC = ["a", "b", "c"]; + private static readonly string[] ExpectedABXY = ["a", "b", "x", "y"]; + [TestMethod] public void ExtensionOrder_DefaultIsEmpty() { @@ -53,6 +61,141 @@ public void ExtensionOrder_PreservesOrderInJson() Assert.AreEqual("m.ext", deserialized.ExtensionOrder[2]); } + [TestMethod] + public void SortByExtensionOrder_EmptyList_ReturnsEmpty() + { + var result = ExtensionOrderHelper.SortByExtensionOrder( + new List(), + OrderAB, + s => s); + + Assert.AreEqual(0, result.Count); + } + + [TestMethod] + public void SortByExtensionOrder_EmptyOrder_PreservesOriginalOrder() + { + var items = new List { "x", "y", "z" }; + var result = ExtensionOrderHelper.SortByExtensionOrder(items, [], s => s); + + CollectionAssert.AreEqual(ExpectedXYZ, result); + } + + [TestMethod] + public void SortByExtensionOrder_AllInOrder_SortsAccordingly() + { + var items = new List { "c", "a", "b" }; + + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderABC, s => s); + + CollectionAssert.AreEqual(ExpectedABC, result); + } + + [TestMethod] + public void SortByExtensionOrder_NoneInOrder_PreservesOriginalOrder() + { + var items = new List { "x", "y", "z" }; + + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderABC, s => s); + + CollectionAssert.AreEqual(ExpectedXYZ, result); + } + + [TestMethod] + public void SortByExtensionOrder_Mixed_OrderedFirstThenUnordered() + { + var items = new List { "x", "b", "y", "a" }; + + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderAB, s => s); + + CollectionAssert.AreEqual(ExpectedABXY, result); + } + + [TestMethod] + public void SortByExtensionOrder_DuplicateProviderIds_AllGroupedInOrder() + { + var items = new List { "b", "a", "b", "c", "a" }; + + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderAB, s => s); + + Assert.AreEqual("a", result[0]); + Assert.AreEqual("a", result[1]); + Assert.AreEqual("b", result[2]); + Assert.AreEqual("b", result[3]); + Assert.AreEqual("c", result[4]); + } + + [TestMethod] + public void SortByExtensionOrder_SingleElement_ReturnsIt() + { + var items = new List { "a" }; + + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderAB, s => s); + + Assert.AreEqual(1, result.Count); + Assert.AreEqual("a", result[0]); + } + + [TestMethod] + public void FindInsertIndex_ProviderNotInOrder_ReturnsCount() + { + var items = new List { "a", "b", "c" }; + + var index = ExtensionOrderHelper.FindInsertIndex(items, "unknown", OrderXY, s => s); + + Assert.AreEqual(3, index); + } + + [TestMethod] + public void FindInsertIndex_EmptyItems_ReturnsZero() + { + var items = new List(); + + var index = ExtensionOrderHelper.FindInsertIndex(items, "a", OrderAB, s => s); + + Assert.AreEqual(0, index); + } + + [TestMethod] + public void FindInsertIndex_FirstInOrder_ReturnsZero() + { + var items = new List { "b", "c", "x" }; + + var index = ExtensionOrderHelper.FindInsertIndex(items, "a", OrderABC, s => s); + + Assert.AreEqual(0, index); + } + + [TestMethod] + public void FindInsertIndex_BetweenOrderedProviders_ReturnsCorrectPosition() + { + var items = new List { "a", "a", "c", "c" }; + + var index = ExtensionOrderHelper.FindInsertIndex(items, "b", OrderABC, s => s); + + Assert.AreEqual(2, index); + } + + [TestMethod] + public void FindInsertIndex_AfterLastOrderedProvider_ReturnsEnd() + { + var items = new List { "a", "b" }; + + var index = ExtensionOrderHelper.FindInsertIndex(items, "c", OrderABC, s => s); + + Assert.AreEqual(2, index); + } + + [TestMethod] + public void FindInsertIndex_WithUnorderedItemsAfterOrdered_InsertsCorrectly() + { + var items = new List { "a", "x", "y" }; + + var index = ExtensionOrderHelper.FindInsertIndex(items, "b", OrderAB, s => s); + + Assert.AreEqual(1, index); + } + private static SettingsModel DeserializeSettings(string json) { return JsonSerializer.Deserialize(json, JsonSerializationContext.Default.SettingsModel) ?? new SettingsModel(); From 0af71f19e954a1baf000055d15dd3851183ab50d Mon Sep 17 00:00:00 2001 From: Michael Jolley Date: Fri, 26 Jun 2026 16:47:23 -0500 Subject: [PATCH 3/3] CmdPal: make extension reordering WYSIWYG across load batches Sort the entire top-level command list by ExtensionOrder after every add path (initial load, dynamic provider update, late append) instead of only within each registration batch. Built-ins and external extensions load in separate batches, so the previous batch-scoped sort could not honor a reorder that crossed that boundary; the dialog let users arrange an order that silently would not stick. Also make ExtensionOrderHelper.SortByExtensionOrder a stable sort so a single provider's commands keep their relative order, remove the now-unused FindInsertIndex helper and the fragile startIndex>=Count heuristic, and update tests (drop FindInsertIndex cases, add stability and cross-batch coverage). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ExtensionOrderHelper.cs | 55 +++---------- .../TopLevelCommandManager.cs | 58 +++++++------ .../ExtensionOrderTests.cs | 81 +++++++++---------- 3 files changed, 81 insertions(+), 113 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs index 3e429d04075b..124d57751dc8 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionOrderHelper.cs @@ -5,16 +5,18 @@ namespace Microsoft.CmdPal.UI.ViewModels; /// -/// Pure helper methods for sorting and inserting items according to a user-defined -/// extension order. Operates on provider ID strings so the logic is easily testable -/// without constructing heavyweight view-model instances. +/// Pure helper methods for sorting items according to a user-defined extension +/// order. Operates on provider ID strings so the logic is easily testable without +/// constructing heavyweight view-model instances. /// internal static class ExtensionOrderHelper { /// /// Returns a new list with items sorted so that those whose provider is in /// appear first (in that order), followed by - /// the remaining items in their original order. + /// the remaining items in their original order. The sort is stable: items that + /// share a provider (and therefore the same order index) keep their original + /// relative order, so a single provider's commands are never shuffled. /// internal static List SortByExtensionOrder(List items, string[] extensionOrder, Func providerIdSelector) { @@ -39,45 +41,10 @@ internal static List SortByExtensionOrder(List items, string[] extensio } } - ordered.Sort((a, b) => orderLookup[providerIdSelector(a)].CompareTo(orderLookup[providerIdSelector(b)])); - ordered.AddRange(unordered); - return ordered; - } - - /// - /// Determines where to insert a new provider's items in the existing list based on - /// . If the provider is in the order list, it's - /// placed after other ordered providers that precede it. Otherwise it goes at the end. - /// - internal static int FindInsertIndex(List items, string providerId, string[] extensionOrder, Func providerIdSelector) - { - var providerIndex = Array.IndexOf(extensionOrder, providerId); - if (providerIndex < 0) - { - return items.Count; - } - - // Find the last item in the list whose provider has a lower order index - for (var i = items.Count - 1; i >= 0; i--) - { - var existingIndex = Array.IndexOf(extensionOrder, providerIdSelector(items[i])); - if (existingIndex >= 0 && existingIndex < providerIndex) - { - // Insert after the last command of this earlier-ordered provider - var insertAfterProvider = providerIdSelector(items[i]); - for (var j = i + 1; j < items.Count; j++) - { - if (providerIdSelector(items[j]) != insertAfterProvider) - { - return j; - } - } - - return i + 1; - } - } - - // This provider has the lowest order index among those present — insert at the beginning - return 0; + // OrderBy is a stable sort, so commands from the same provider keep the + // relative order in which they were loaded. + var result = ordered.OrderBy(item => orderLookup[providerIdSelector(item)]).ToList(); + result.AddRange(unordered); + return result; } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs index 88d4a5f485ac..41ac04ddc44b 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs @@ -192,16 +192,12 @@ private async Task UpdateCommandsForProvider(CommandProviderWrapper sender, IIte var startIndex = FindIndexForFirstProviderItem(clone, sender.ProviderId); clone.RemoveAll(item => item.CommandProviderId == sender.ProviderId); - if (startIndex >= clone.Count) - { - // Provider wasn't in the list yet — find position based on ExtensionOrder - var extensionOrder = _serviceProvider.GetRequiredService().Settings.ExtensionOrder; - startIndex = FindInsertIndexByExtensionOrder(clone, sender.ProviderId, extensionOrder); - } - - clone.InsertRange(startIndex, newItems); + // Insert the refreshed items where this provider previously sat (or at the + // end if it's brand new). RebuildTopLevelCommands then re-sorts the whole + // list so the provider lands in its user-configured position. + clone.InsertRange(Math.Min(startIndex, clone.Count), newItems); - ListHelpers.InPlaceUpdateList(TopLevelCommands, clone); + RebuildTopLevelCommands(clone); } lock (_dockBandsLock) @@ -477,16 +473,14 @@ private async Task RegisterAndLoadCommandsAsync(IEnumera lock (TopLevelCommands) { - var extensionOrder = _serviceProvider.GetRequiredService().Settings.ExtensionOrder; - if (extensionOrder.Length > 0) - { - commandsToAdd = SortByExtensionOrder(commandsToAdd, extensionOrder); - } + // Append the freshly loaded batch, then re-sort the entire top-level list + // so the user's configured extension order is honored across batches + // (built-ins and external extensions load in separate batches). + var clone = new List(TopLevelCommands.Count + commandsToAdd.Count); + clone.AddRange(TopLevelCommands); + clone.AddRange(commandsToAdd); - foreach (var c in commandsToAdd) - { - TopLevelCommands.Add(c); - } + RebuildTopLevelCommands(clone); } lock (_dockBandsLock) @@ -509,11 +503,22 @@ private async Task RegisterAndLoadCommandsAsync(IEnumera return new RegisterAndLoadSummary(totalCommands, totalDockBands); } - internal static List SortByExtensionOrder(List commands, string[] extensionOrder) - => ExtensionOrderHelper.SortByExtensionOrder(commands, extensionOrder, c => c.CommandProviderId); + /// + /// Replaces the contents of with , + /// first applying the user-configured extension order when one is set. Callers must hold the + /// lock. Built-in and external providers load in separate + /// batches, so re-sorting the whole list here is what lets the configured order span them. + /// + private void RebuildTopLevelCommands(List newCommands) + { + var extensionOrder = _serviceProvider.GetRequiredService().Settings.ExtensionOrder; + if (extensionOrder.Length > 0) + { + newCommands = ExtensionOrderHelper.SortByExtensionOrder(newCommands, extensionOrder, c => c.CommandProviderId); + } - private static int FindInsertIndexByExtensionOrder(List items, string providerId, string[] extensionOrder) - => ExtensionOrderHelper.FindInsertIndex(items, providerId, extensionOrder, c => c.CommandProviderId); + ListHelpers.InPlaceUpdateList(TopLevelCommands, newCommands); + } private async Task TryLoadCommandsAsync(CommandProviderWrapper wrapper, CancellationToken ct) { @@ -559,10 +564,11 @@ private async Task AppendCommandsWhenReadyAsync( { lock (TopLevelCommands) { - foreach (var c in commands) - { - TopLevelCommands.Add(c); - } + var clone = new List(TopLevelCommands.Count + commands.Count); + clone.AddRange(TopLevelCommands); + clone.AddRange(commands); + + RebuildTopLevelCommands(clone); } } diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs index 99c159490647..6c2f1634d294 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionOrderTests.cs @@ -13,10 +13,13 @@ public class ExtensionOrderTests { private static readonly string[] OrderAB = ["a", "b"]; private static readonly string[] OrderABC = ["a", "b", "c"]; - private static readonly string[] OrderXY = ["x", "y"]; private static readonly string[] ExpectedXYZ = ["x", "y", "z"]; private static readonly string[] ExpectedABC = ["a", "b", "c"]; private static readonly string[] ExpectedABXY = ["a", "b", "x", "y"]; + private static readonly string[] OrderExternalThenBuiltIns = ["external.foo", "builtin.apps", "builtin.calc"]; + private static readonly string[] OrderExternalThenApps = ["external.foo", "builtin.apps"]; + private static readonly string[] ExpectedExternalFirst = ["external.foo", "builtin.apps", "builtin.calc"]; + private static readonly string[] ExpectedNewProviderLast = ["external.foo", "builtin.apps", "newly.installed"]; [TestMethod] public void ExtensionOrder_DefaultIsEmpty() @@ -137,63 +140,55 @@ public void SortByExtensionOrder_SingleElement_ReturnsIt() } [TestMethod] - public void FindInsertIndex_ProviderNotInOrder_ReturnsCount() + public void SortByExtensionOrder_SameProviderItems_KeepRelativeOrder() { - var items = new List { "a", "b", "c" }; + // Two providers ("a" and "b"), each contributing several commands. The sort + // must be stable so a single provider's commands are never shuffled. + var items = new List<(string Provider, int Command)> + { + ("b", 0), + ("a", 0), + ("b", 1), + ("a", 1), + ("a", 2), + }; - var index = ExtensionOrderHelper.FindInsertIndex(items, "unknown", OrderXY, s => s); + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderAB, x => x.Provider); - Assert.AreEqual(3, index); + var expected = new List<(string, int)> + { + ("a", 0), + ("a", 1), + ("a", 2), + ("b", 0), + ("b", 1), + }; + CollectionAssert.AreEqual(expected, result); } [TestMethod] - public void FindInsertIndex_EmptyItems_ReturnsZero() + public void SortByExtensionOrder_ExternalCanOutrankBuiltIn_WhenBothInOrder() { - var items = new List(); + // Built-ins load before external extensions, but the configured order lists the + // external provider first. Sorting the full list lets the external outrank the + // built-in so the result matches what the reorder dialog showed (WYSIWYG). + var items = new List { "builtin.apps", "builtin.calc", "external.foo" }; - var index = ExtensionOrderHelper.FindInsertIndex(items, "a", OrderAB, s => s); + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderExternalThenBuiltIns, s => s); - Assert.AreEqual(0, index); + CollectionAssert.AreEqual(ExpectedExternalFirst, result); } [TestMethod] - public void FindInsertIndex_FirstInOrder_ReturnsZero() + public void SortByExtensionOrder_NewProviderNotInOrder_GoesToEnd() { - var items = new List { "b", "c", "x" }; + // A provider installed after the last reorder isn't in the saved order, so it + // keeps its natural load position at the end rather than jumping to the front. + var items = new List { "external.foo", "builtin.apps", "newly.installed" }; - var index = ExtensionOrderHelper.FindInsertIndex(items, "a", OrderABC, s => s); + var result = ExtensionOrderHelper.SortByExtensionOrder(items, OrderExternalThenApps, s => s); - Assert.AreEqual(0, index); - } - - [TestMethod] - public void FindInsertIndex_BetweenOrderedProviders_ReturnsCorrectPosition() - { - var items = new List { "a", "a", "c", "c" }; - - var index = ExtensionOrderHelper.FindInsertIndex(items, "b", OrderABC, s => s); - - Assert.AreEqual(2, index); - } - - [TestMethod] - public void FindInsertIndex_AfterLastOrderedProvider_ReturnsEnd() - { - var items = new List { "a", "b" }; - - var index = ExtensionOrderHelper.FindInsertIndex(items, "c", OrderABC, s => s); - - Assert.AreEqual(2, index); - } - - [TestMethod] - public void FindInsertIndex_WithUnorderedItemsAfterOrdered_InsertsCorrectly() - { - var items = new List { "a", "x", "y" }; - - var index = ExtensionOrderHelper.FindInsertIndex(items, "b", OrderAB, s => s); - - Assert.AreEqual(1, index); + CollectionAssert.AreEqual(ExpectedNewProviderLast, result); } private static SettingsModel DeserializeSettings(string json)