Skip to content

Commit

Permalink
This fixes the "uninstall&reload doesn't load extensions" thing....
Browse files Browse the repository at this point in the history 
But it's probably overkill. I think I really only need the things in ExtensionService.
The problem was that TopLevelCommandManager iterates over the installed extensions on a reload,
but SIMULTANEOUSLY, the ExtensionService gets the notification that the extension was uninstalled,
and it fuckin blows away the whole _installedExtensions cache
  • Loading branch information
@zadjii
zadjii committed Jan 28, 2025
1 parent bbc23b4 commit 9959d05
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 56 deletions.
94 changes: 54 additions & 40 deletions src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// 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.ObjectModel;
using System.Collections.Specialized;
using System.Diagnostics;
using CommunityToolkit.Mvvm.Messaging;
Expand All @@ -23,8 +22,8 @@ public partial class MainListPage : DynamicListPage,
{
private readonly IServiceProvider _serviceProvider;

private readonly ObservableCollection<TopLevelCommandItemWrapper> _commands;

// private readonly ObservableCollection<TopLevelCommandItemWrapper> _commands;
private readonly TopLevelCommandManager _tlcManager;
private IEnumerable<IListItem>? _filteredItems;

private bool _appsLoading = true;
Expand All @@ -36,12 +35,12 @@ public MainListPage(IServiceProvider serviceProvider)
Icon = new(Path.Combine(AppDomain.CurrentDomain.BaseDirectory.ToString(), "Assets\\StoreLogo.scale-200.png"));
_serviceProvider = serviceProvider;

var tlcManager = _serviceProvider.GetService<TopLevelCommandManager>()!;
tlcManager.PropertyChanged += TlcManager_PropertyChanged;
_tlcManager = _serviceProvider.GetService<TopLevelCommandManager>()!;
_tlcManager.PropertyChanged += TlcManager_PropertyChanged;

// reference the TLC collection directly... maybe? TODO is this a good idea ot a terrible one?
_commands = tlcManager.TopLevelCommands;
_commands.CollectionChanged += Commands_CollectionChanged;
// _commands = tlcManager.TopLevelCommands;
_tlcManager.TopLevelCommands.CollectionChanged += Commands_CollectionChanged;

WeakReferenceMessenger.Default.Register<ClearSearchMessage>(this);

Expand All @@ -60,7 +59,7 @@ private void TlcManager_PropertyChanged(object? sender, System.ComponentModel.Pr
}
}

private void Commands_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) => RaiseItemsChanged(_commands.Count);
private void Commands_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) => RaiseItemsChanged(_tlcManager.TopLevelCommands.Count);

public override IListItem[] GetItems()
{
Expand All @@ -70,9 +69,20 @@ public override IListItem[] GetItems()
_startedAppLoad = true;
}

return string.IsNullOrEmpty(SearchText)
? _commands.Select(tlc => tlc).Where(tlc => !string.IsNullOrEmpty(tlc.Title)).ToArray()
: _filteredItems?.ToArray() ?? [];
if (string.IsNullOrEmpty(SearchText))
{
lock (_tlcManager.TopLevelCommands)
{
return _tlcManager.TopLevelCommands.Select(tlc => tlc).Where(tlc => !string.IsNullOrEmpty(tlc.Title)).ToArray();
}
}
else
{
lock (_tlcManager.TopLevelCommands)
{
return _filteredItems?.ToArray() ?? [];
}
}
}

public override void UpdateSearchText(string oldSearch, string newSearch)
Expand All @@ -89,40 +99,44 @@ public override void UpdateSearchText(string oldSearch, string newSearch)
}
}

// This gets called on a background thread, because ListViewModel
// updates the .SearchText of all extensions on a BG thread.
foreach (var command in _commands)
var commands = _tlcManager.TopLevelCommands;
lock (commands)
{
command.TryUpdateFallbackText(newSearch);
}
// This gets called on a background thread, because ListViewModel
// updates the .SearchText of all extensions on a BG thread.
foreach (var command in commands)
{
command.TryUpdateFallbackText(newSearch);
}

// Cleared out the filter text? easy. Reset _filteredItems, and bail out.
if (string.IsNullOrEmpty(newSearch))
{
_filteredItems = null;
RaiseItemsChanged(_commands.Count);
return;
}
// Cleared out the filter text? easy. Reset _filteredItems, and bail out.
if (string.IsNullOrEmpty(newSearch))
{
_filteredItems = null;
RaiseItemsChanged(commands.Count);
return;
}

// If the new string doesn't start with the old string, then we can't
// re-use previous results. Reset _filteredItems, and keep er moving.
if (!newSearch.StartsWith(oldSearch, StringComparison.CurrentCultureIgnoreCase))
{
_filteredItems = null;
}
// If the new string doesn't start with the old string, then we can't
// re-use previous results. Reset _filteredItems, and keep er moving.
if (!newSearch.StartsWith(oldSearch, StringComparison.CurrentCultureIgnoreCase))
{
_filteredItems = null;
}

// If we don't have any previous filter results to work with, start
// with a list of all our commands & apps.
if (_filteredItems == null)
{
IEnumerable<IListItem> commands = _commands;
IEnumerable<IListItem> apps = AllAppsCommandProvider.Page.GetItems();
_filteredItems = commands.Concat(apps);
}
// If we don't have any previous filter results to work with, start
// with a list of all our commands & apps.
if (_filteredItems == null)
{
// IEnumerable<IListItem> commands = _commands;
IEnumerable<IListItem> apps = AllAppsCommandProvider.Page.GetItems();
_filteredItems = commands.Concat(apps);
}

// Produce a list of everything that matches the current filter.
_filteredItems = ListHelpers.FilterList<IListItem>(_filteredItems, SearchText, ScoreTopLevelItem);
RaiseItemsChanged(_filteredItems.Count());
// Produce a list of everything that matches the current filter.
_filteredItems = ListHelpers.FilterList<IListItem>(_filteredItems, SearchText, ScoreTopLevelItem);
RaiseItemsChanged(_filteredItems.Count());
}
}

private bool ActuallyLoading()
Expand Down
31 changes: 20 additions & 11 deletions src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public class ExtensionService : IExtensionService, IDisposable

public event TypedEventHandler<IExtensionService, IEnumerable<IExtensionWrapper>>? OnExtensionAdded;

public event TypedEventHandler<IExtensionService, IEnumerable<IExtensionWrapper>>? OnExtensionRemoved;

private static readonly PackageCatalog _catalog = PackageCatalog.OpenForCurrentUser();
private static readonly Lock _lock = new();
private readonly SemaphoreSlim _getInstalledExtensionsLock = new(1, 1);
Expand Down Expand Up @@ -75,7 +77,7 @@ private void Catalog_PackageInstalling(PackageCatalog sender, PackageInstallingE
}
});

OnPackageChange(args.Package);
// OnPackageChange(args.Package);
}

// if (isCmdPalExtension)
Expand All @@ -92,14 +94,22 @@ private void Catalog_PackageUninstalling(PackageCatalog sender, PackageUninstall
{
lock (_lock)
{
List<IExtensionWrapper> removedExtensions = [];
foreach (var extension in _installedExtensions)
{
if (extension.PackageFullName == args.Package.Id.FullName)
{
OnPackageChange(args.Package);
break;
// OnPackageChange(args.Package);
removedExtensions.Add(extension);

// _installedExtensions.Remove(extension);

// break;
}
}

OnExtensionRemoved?.Invoke(this, removedExtensions);
_installedExtensions.RemoveAll(i => removedExtensions.Contains(i));
}
}
}
Expand All @@ -118,19 +128,18 @@ private void Catalog_PackageUpdating(PackageCatalog sender, PackageUpdatingEvent
var isExtension = isCmdPalExtensionResult.IsExtension;
if (isExtension)
{
OnPackageChange(args.TargetPackage);
// OnPackageChange(args.TargetPackage);
}
}
}
}

private void OnPackageChange(Package package)
{
_installedExtensions.Clear();
_enabledExtensions.Clear();
OnExtensionsChanged.Invoke(this, EventArgs.Empty);
}

// private void OnPackageChange(Package package)
// {
// _installedExtensions.Clear();
// _enabledExtensions.Clear();
// OnExtensionsChanged.Invoke(this, EventArgs.Empty);
// }
private static async Task<IsExtensionResult> IsValidCmdPalExtension(Package package)
{
var extensions = await AppExtensionCatalog.Open("com.microsoft.windows.commandpalette").FindAllAsync();
Expand Down
23 changes: 18 additions & 5 deletions src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ private async Task LoadTopLevelCommandsFromProvider(CommandProviderWrapper comma
{
ExtensionHost = commandProvider.ExtensionHost,
};
TopLevelCommands.Add(wrapper);
lock (TopLevelCommands)
{
TopLevelCommands.Add(wrapper);
}
};

await Task.Factory.StartNew(
Expand Down Expand Up @@ -174,7 +177,11 @@ public async Task ReloadAllCommandsAsync()
IsLoading = true;
var extensionService = _serviceProvider.GetService<IExtensionService>()!;
await extensionService.SignalStopExtensionsAsync();
TopLevelCommands.Clear();
lock (TopLevelCommands)
{
TopLevelCommands.Clear();
}

await LoadBuiltinsAsync();
_ = Task.Run(LoadExtensionsAsync);
}
Expand All @@ -190,6 +197,9 @@ public async Task ReloadAllCommandsAsync()
public async Task<bool> LoadExtensionsAsync()
{
var extensionService = _serviceProvider.GetService<IExtensionService>()!;

extensionService.OnExtensionAdded -= ExtensionService_OnExtensionAddedAsync;

var extensions = await extensionService.GetInstalledExtensionsAsync();
_extensionCommandProviders.Clear();
foreach (var extension in extensions)
Expand Down Expand Up @@ -237,11 +247,14 @@ private void ExtensionService_OnExtensionAddedAsync(IExtensionService sender, IE

public TopLevelCommandItemWrapper? LookupCommand(string id)
{
foreach (var command in TopLevelCommands)
lock (TopLevelCommands)
{
if (command.Id == id)
foreach (var command in TopLevelCommands)
{
return command;
if (command.Id == id)
{
return command;
}
}
}

Expand Down

0 comments on commit 9959d05

Please sign in to comment.