Skip to content

Commit 60bbf07

Browse files
authored
Update Fallback commands async, once (#38157)
The problem: > * we need to go update all the Fallback commands. (these are ones that extensions can use to react to the search text - basically, "what the user typed wasn't found immediately, but here's something they can fall back on" > * this is wacky, because the way I had it, I update each item, and if it "changes visibility", then we need to update the main list, because we've already removed it from the list. So we need to re-update the list to account for that > * you missed it reading that (and i missed it writing it) but that basically means we re-populate the list F={num fallbacks} times, because each one sends the "do it again" message > * That results in us basically creating (F+1)*(N=num items+apps) view models, initializing them, and not needing most of them The crux here being a single thread, to update all the fallback items, that then only raises _one_ items changed at the very end. I don't love this, one misbehaving fallback could stop all the others. In theory, we should do a parallel update of all these things, with a like, 1s timeout on each leg. But it has gotta be faster till we can do #38140 (or similar) Closes: (not sure I filed one). But the first typed character _felt_ slow.
1 parent d597bd2 commit 60bbf07

File tree

2 files changed

+58
-29
lines changed

2 files changed

+58
-29
lines changed

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs

+22-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Collections.Immutable;
56
using System.Collections.Specialized;
67
using CommunityToolkit.Mvvm.Messaging;
78
using Microsoft.CmdPal.Ext.Apps;
@@ -101,12 +102,7 @@ public override void UpdateSearchText(string oldSearch, string newSearch)
101102
var commands = _tlcManager.TopLevelCommands;
102103
lock (commands)
103104
{
104-
// This gets called on a background thread, because ListViewModel
105-
// updates the .SearchText of all extensions on a BG thread.
106-
foreach (var command in commands)
107-
{
108-
command.TryUpdateFallbackText(newSearch);
109-
}
105+
UpdateFallbacks(newSearch, commands.ToImmutableArray());
110106

111107
// Cleared out the filter text? easy. Reset _filteredItems, and bail out.
112108
if (string.IsNullOrEmpty(newSearch))
@@ -137,6 +133,26 @@ public override void UpdateSearchText(string oldSearch, string newSearch)
137133
}
138134
}
139135

136+
private void UpdateFallbacks(string newSearch, IReadOnlyList<TopLevelViewModel> commands)
137+
{
138+
// fire and forget
139+
_ = Task.Run(() =>
140+
{
141+
var needsToUpdate = false;
142+
143+
foreach (var command in commands)
144+
{
145+
var changedVisibility = command.SafeUpdateFallbackTextSynchronous(newSearch);
146+
needsToUpdate = needsToUpdate || changedVisibility;
147+
}
148+
149+
if (needsToUpdate)
150+
{
151+
RaiseItemsChanged();
152+
}
153+
});
154+
}
155+
140156
private bool ActuallyLoading()
141157
{
142158
var tlcManager = _serviceProvider.GetService<TopLevelCommandManager>()!;

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelViewModel.cs

+36-23
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44

55
using System.Collections.ObjectModel;
66
using CommunityToolkit.Mvvm.ComponentModel;
7-
using CommunityToolkit.Mvvm.Messaging;
8-
using Microsoft.CmdPal.UI.ViewModels.Messages;
7+
using ManagedCommon;
98
using Microsoft.CmdPal.UI.ViewModels.Settings;
109
using Microsoft.CommandPalette.Extensions;
1110
using Microsoft.CommandPalette.Extensions.Toolkit;
@@ -237,32 +236,46 @@ private void DoOnUiThread(Action action)
237236
}
238237
}
239238

240-
public void TryUpdateFallbackText(string newQuery)
239+
internal bool SafeUpdateFallbackTextSynchronous(string newQuery)
241240
{
242241
if (!IsFallback)
243242
{
244-
return;
243+
return false;
245244
}
246245

247-
_ = Task.Run(() =>
246+
try
248247
{
249-
try
250-
{
251-
var model = _commandItemViewModel.Model.Unsafe;
252-
if (model is IFallbackCommandItem fallback)
253-
{
254-
var wasEmpty = string.IsNullOrEmpty(Title);
255-
fallback.FallbackHandler.UpdateQuery(newQuery);
256-
var isEmpty = string.IsNullOrEmpty(Title);
257-
if (wasEmpty != isEmpty)
258-
{
259-
WeakReferenceMessenger.Default.Send<UpdateFallbackItemsMessage>();
260-
}
261-
}
262-
}
263-
catch (Exception)
264-
{
265-
}
266-
});
248+
return UnsafeUpdateFallbackSynchronous(newQuery);
249+
}
250+
catch (Exception ex)
251+
{
252+
Logger.LogError(ex.ToString());
253+
}
254+
255+
return false;
256+
}
257+
258+
/// <summary>
259+
/// Calls UpdateQuery on our command, if we're a fallback item. This does
260+
/// RPC work, so make sure you're calling it on a BG thread.
261+
/// </summary>
262+
/// <param name="newQuery">The new search text to pass to the extension</param>
263+
/// <returns>true if our Title changed across this call</returns>
264+
private bool UnsafeUpdateFallbackSynchronous(string newQuery)
265+
{
266+
var model = _commandItemViewModel.Model.Unsafe;
267+
268+
// RPC to check type
269+
if (model is IFallbackCommandItem fallback)
270+
{
271+
var wasEmpty = string.IsNullOrEmpty(Title);
272+
273+
// RPC for method
274+
fallback.FallbackHandler.UpdateQuery(newQuery);
275+
var isEmpty = string.IsNullOrEmpty(Title);
276+
return wasEmpty != isEmpty;
277+
}
278+
279+
return false;
267280
}
268281
}

0 commit comments

Comments
 (0)