Skip to content

Commit acbd438

Browse files
zadjii-msftzadjii-msft
committed
PRE-MERGE #38844 
1 parent dd11184 commit acbd438

File tree

5 files changed

+152
-25
lines changed

5 files changed

+152
-25
lines changed

‎src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,10 @@ public CommandProviderWrapper(ICommandProvider provider, TaskScheduler mainThrea
6666
DisplayName = provider.DisplayName;
6767
Icon = new(provider.Icon);
6868
Icon.InitializeProperties();
69+
70+
// Note: explicitly not InitializeProperties()ing the settings here. If
71+
// we do that, then we'd regress GH #38321
6972
Settings = new(provider.Settings, this, _taskScheduler);
70-
Settings.InitializeProperties();
7173

7274
Logger.LogDebug($"Initialized command provider {ProviderId}");
7375
}
@@ -151,9 +153,11 @@ public async Task LoadTopLevelCommands(IServiceProvider serviceProvider, WeakRef
151153
Icon = new(model.Icon);
152154
Icon.InitializeProperties();
153155

156+
// Note: explicitly not InitializeProperties()ing the settings here. If
157+
// we do that, then we'd regress GH #38321
154158
Settings = new(model.Settings, this, _taskScheduler);
155-
Settings.InitializeProperties();
156159

160+
// We do need to explicitly initialize commands though
157161
InitializeCommands(commands, fallbacks, serviceProvider, pageContext);
158162

159163
Logger.LogDebug($"Loaded commands from {DisplayName} ({ProviderId})");
@@ -194,21 +198,6 @@ private void InitializeCommands(ICommandItem[] commands, IFallbackCommandItem[]
194198
}
195199
}
196200

197-
/* This is a View/ExtensionHost piece
198-
* public void AllowSetForeground(bool allow)
199-
{
200-
if (!IsExtension)
201-
{
202-
return;
203-
}
204-
205-
var iextn = extensionWrapper?.GetExtensionObject();
206-
unsafe
207-
{
208-
PInvoke.CoAllowSetForegroundWindow(iextn);
209-
}
210-
}*/
211-
212201
public override bool Equals(object? obj) => obj is CommandProviderWrapper wrapper && isValid == wrapper.isValid;
213202

214203
public override int GetHashCode() => _commandProvider.GetHashCode();

‎src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandSettingsViewModel.cs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,25 @@
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 ManagedCommon;
56
using Microsoft.CmdPal.UI.ViewModels.Models;
67
using Microsoft.CommandPalette.Extensions;
78

89
namespace Microsoft.CmdPal.UI.ViewModels;
910

10-
public partial class CommandSettingsViewModel(ICommandSettings _unsafeSettings, CommandProviderWrapper provider, TaskScheduler mainThread)
11+
public partial class CommandSettingsViewModel(ICommandSettings? _unsafeSettings, CommandProviderWrapper provider, TaskScheduler mainThread)
1112
{
1213
private readonly ExtensionObject<ICommandSettings> _model = new(_unsafeSettings);
1314

1415
public ContentPageViewModel? SettingsPage { get; private set; }
1516

16-
public void InitializeProperties()
17+
public bool Initialized { get; private set; }
18+
19+
public bool HasSettings =>
20+
_model.Unsafe != null && // We have a settings model AND
21+
(!Initialized || SettingsPage != null); // we weren't initialized, OR we were, and we do have a settings page
22+
23+
private void UnsafeInitializeProperties()
1724
{
1825
var model = _model.Unsafe;
1926
if (model == null)
@@ -27,4 +34,27 @@ public void InitializeProperties()
2734
SettingsPage.InitializeProperties();
2835
}
2936
}
37+
38+
public void SafeInitializeProperties()
39+
{
40+
try
41+
{
42+
UnsafeInitializeProperties();
43+
}
44+
catch (Exception ex)
45+
{
46+
Logger.LogError($"Failed to load settings page", ex: ex);
47+
}
48+
49+
Initialized = true;
50+
}
51+
52+
public void DoOnUiThread(Action action)
53+
{
54+
Task.Factory.StartNew(
55+
action,
56+
CancellationToken.None,
57+
TaskCreationOptions.None,
58+
mainThread);
59+
}
3060
}

‎src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ProviderSettingsViewModel.cs

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ public partial class ProviderSettingsViewModel(
1818
IServiceProvider _serviceProvider) : ObservableObject
1919
{
2020
private readonly SettingsModel _settings = _serviceProvider.GetService<SettingsModel>()!;
21+
private readonly Lock _initializeSettingsLock = new();
22+
private Task? _initializeSettingsTask;
2123

2224
public string DisplayName => _provider.DisplayName;
2325

@@ -34,6 +36,9 @@ public partial class ProviderSettingsViewModel(
3436

3537
public IconInfoViewModel Icon => _provider.Icon;
3638

39+
[ObservableProperty]
40+
public partial bool LoadingSettings { get; set; } = _provider.Settings?.HasSettings ?? false;
41+
3742
public bool IsEnabled
3843
{
3944
get => _providerSettings.IsEnabled;
@@ -56,15 +61,60 @@ public bool IsEnabled
5661
}
5762
}
5863

59-
private void Provider_CommandsChanged(CommandProviderWrapper sender, CommandPalette.Extensions.IItemsChangedEventArgs args)
64+
/// <summary>
65+
/// Gets a value indicating whether returns true if we have a settings page
66+
/// that's initialized, or we are still working on initializing that
67+
/// settings page. If we don't have a settings object, or that settings
68+
/// object doesn't have a settings page, then we'll return false.
69+
/// </summary>
70+
public bool HasSettings
6071
{
61-
OnPropertyChanged(nameof(ExtensionSubtext));
62-
OnPropertyChanged(nameof(TopLevelCommands));
72+
get
73+
{
74+
if (_provider.Settings == null)
75+
{
76+
return false;
77+
}
78+
79+
if (_provider.Settings.Initialized)
80+
{
81+
return _provider.Settings.HasSettings;
82+
}
83+
84+
// settings still need to be loaded.
85+
return LoadingSettings;
86+
}
6387
}
6488

65-
public bool HasSettings => _provider.Settings != null && _provider.Settings.SettingsPage != null;
89+
/// <summary>
90+
/// Gets will return the settings page, if we have one, and have initialized it.
91+
/// If we haven't initialized it, this will kick off a thread to start
92+
/// initializing it.
93+
/// </summary>
94+
public ContentPageViewModel? SettingsPage
95+
{
96+
get
97+
{
98+
if (_provider.Settings == null)
99+
{
100+
return null;
101+
}
66102

67-
public ContentPageViewModel? SettingsPage => HasSettings ? _provider?.Settings?.SettingsPage : null;
103+
if (_provider.Settings.Initialized)
104+
{
105+
LoadingSettings = false;
106+
return _provider.Settings.SettingsPage;
107+
}
108+
109+
// Don't load the settings if we're already working on it
110+
lock (_initializeSettingsLock)
111+
{
112+
_initializeSettingsTask ??= Task.Run(InitializeSettingsPage);
113+
}
114+
115+
return null;
116+
}
117+
}
68118

69119
[field: AllowNull]
70120
public List<TopLevelViewModel> TopLevelCommands
@@ -90,4 +140,30 @@ private List<TopLevelViewModel> BuildTopLevelViewModels()
90140
}
91141

92142
private void Save() => SettingsModel.SaveSettings(_settings);
143+
144+
private void InitializeSettingsPage()
145+
{
146+
if (_provider.Settings == null)
147+
{
148+
return;
149+
}
150+
151+
_provider.Settings.SafeInitializeProperties();
152+
_provider.Settings.DoOnUiThread(() =>
153+
{
154+
// Changing these properties will try to update XAML, and that has
155+
// to be handled on the UI thread, so we need to raise them on the
156+
// UI thread
157+
LoadingSettings = false;
158+
OnPropertyChanged(nameof(HasSettings));
159+
OnPropertyChanged(nameof(LoadingSettings));
160+
OnPropertyChanged(nameof(SettingsPage));
161+
});
162+
}
163+
164+
private void Provider_CommandsChanged(CommandProviderWrapper sender, CommandPalette.Extensions.IItemsChangedEventArgs args)
165+
{
166+
OnPropertyChanged(nameof(ExtensionSubtext));
167+
OnPropertyChanged(nameof(TopLevelCommands));
168+
}
93169
}

‎src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
using System.Collections.Immutable;
66
using System.Collections.ObjectModel;
7+
using System.Diagnostics;
78
using CommunityToolkit.Mvvm.ComponentModel;
89
using CommunityToolkit.Mvvm.Input;
910
using CommunityToolkit.Mvvm.Messaging;
@@ -44,6 +45,9 @@ public TopLevelCommandManager(IServiceProvider serviceProvider)
4445

4546
public async Task<bool> LoadBuiltinsAsync()
4647
{
48+
var s = new Stopwatch();
49+
s.Start();
50+
4751
_builtInCommands.Clear();
4852

4953
// Load built-In commands first. These are all in-proc, and
@@ -56,6 +60,10 @@ public async Task<bool> LoadBuiltinsAsync()
5660
await LoadTopLevelCommandsFromProvider(wrapper);
5761
}
5862

63+
s.Stop();
64+
65+
Logger.LogDebug($"Loading built-ins took {s.ElapsedMilliseconds}ms");
66+
5967
return true;
6068
}
6169

@@ -239,6 +247,9 @@ private void ExtensionService_OnExtensionAdded(IExtensionService sender, IEnumer
239247

240248
private async Task StartExtensionsAndGetCommands(IEnumerable<IExtensionWrapper> extensions)
241249
{
250+
var s = new Stopwatch();
251+
s.Start();
252+
242253
// TODO This most definitely needs a lock
243254
foreach (var extension in extensions)
244255
{
@@ -258,6 +269,10 @@ private async Task StartExtensionsAndGetCommands(IEnumerable<IExtensionWrapper>
258269
Logger.LogError(ex.ToString());
259270
}
260271
}
272+
273+
s.Stop();
274+
275+
Logger.LogDebug($"Loading extensions took {s.ElapsedMilliseconds}ms");
261276
}
262277

263278
private void ExtensionService_OnExtensionRemoved(IExtensionService sender, IEnumerable<IExtensionWrapper> extensions)

‎src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionPage.xaml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,24 @@
113113
Visibility="{x:Bind ViewModel.HasSettings}" />
114114

115115
<Frame x:Name="SettingsFrame" Visibility="{x:Bind ViewModel.HasSettings}">
116-
<cmdpalUI:ContentPage ViewModel="{x:Bind ViewModel.SettingsPage, Mode=OneWay}" />
116+
117+
<controls:SwitchPresenter
118+
HorizontalAlignment="Stretch"
119+
TargetType="x:Boolean"
120+
Value="{x:Bind ViewModel.LoadingSettings, Mode=OneWay}">
121+
<controls:Case Value="True">
122+
<ProgressRing
123+
Width="36"
124+
Height="36"
125+
HorizontalAlignment="Center"
126+
VerticalAlignment="Center"
127+
IsIndeterminate="True" />
128+
</controls:Case>
129+
<controls:Case Value="False">
130+
<cmdpalUI:ContentPage ViewModel="{x:Bind ViewModel.SettingsPage, Mode=OneWay}" />
131+
</controls:Case>
132+
</controls:SwitchPresenter>
133+
117134
</Frame>
118135

119136
<TextBlock

0 commit comments

Comments
 (0)