Skip to content

Commit 78f8fb7

Browse files
andyleejordanandyleejordan
committed
Handle host initialization in Initialize request delegate
This fixes a whole confusing mess of problems. Previously our initialize delegate just set some options, but didn’t actually initialize the host (which is required for the server to work). Instead, we relied on a particular behavior of VS Code’s client to immediately send us an `OnDidChangeConfiguration` request, where we then actually started the host’s command loop. The logic was there because that’s where we received the client’s configuration on loading profiles and initial working directory. However, not all clients immediately sent this request, resulting in PSES v3 not working in Vim or Emacs etc. This behavior was also confusing! Instead, we now request that the client send initialization configuration via the initialize request’s parameters (in fact, the spec has an `initializationOptions` field just for this). Based on these settings, we can now start the host at the correct time. We also no longer respect `rootUri` as it has been long-since deprecated. Note that we can “no longer” (but we weren’t currently) respect a change to `EnableProfileLoading` nor `InitialWorkingDirectory` after the server has initialized. But this makes sense, you must restart it if you want to disable/enable profile loading or change the initial working directory.
1 parent 846045e commit 78f8fb7

File tree

3 files changed

+20
-96
lines changed

3 files changed

+20
-96
lines changed

‎src/PowerShellEditorServices/Server/PsesLanguageServer.cs

+19-19
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
using System;
54
using System.IO;
65
using System.Threading.Tasks;
76
using Microsoft.Extensions.DependencyInjection;
@@ -12,6 +11,8 @@
1211
using Microsoft.PowerShell.EditorServices.Services.Extension;
1312
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
1413
using Microsoft.PowerShell.EditorServices.Services.Template;
14+
using Newtonsoft.Json.Linq;
15+
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
1516
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
1617
using OmniSharp.Extensions.LanguageServer.Server;
1718
using Serilog;
@@ -114,33 +115,32 @@ public async Task StartAsync()
114115
// _Initialize_ request:
115116
// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize
116117
.OnInitialize(
117-
(languageServer, request, _) =>
118+
(languageServer, initializeParams, cancellationToken) =>
118119
{
119-
Log.Logger.Debug("Initializing OmniSharp Language Server");
120-
121-
IServiceProvider serviceProvider = languageServer.Services;
122-
123-
_psesHost = serviceProvider.GetService<PsesInternalHost>();
124-
125-
WorkspaceService workspaceService = serviceProvider.GetService<WorkspaceService>();
126-
127-
// Grab the workspace path from the parameters
128-
if (request.RootUri != null)
120+
// Set the workspace path from the parameters.
121+
WorkspaceService workspaceService = languageServer.Services.GetService<WorkspaceService>();
122+
if (initializeParams.WorkspaceFolders is not null)
129123
{
130-
workspaceService.WorkspacePath = request.RootUri.GetFileSystemPath();
131-
}
132-
else if (request.WorkspaceFolders != null)
133-
{
134-
// If RootUri isn't set, try to use the first WorkspaceFolder.
135124
// TODO: Support multi-workspace.
136-
foreach (OmniSharp.Extensions.LanguageServer.Protocol.Models.WorkspaceFolder workspaceFolder in request.WorkspaceFolders)
125+
foreach (WorkspaceFolder workspaceFolder in initializeParams.WorkspaceFolders)
137126
{
138127
workspaceService.WorkspacePath = workspaceFolder.Uri.GetFileSystemPath();
139128
break;
140129
}
141130
}
142131

143-
return Task.CompletedTask;
132+
// Parse initialization options.
133+
JObject initializationOptions = initializeParams.InitializationOptions as JObject;
134+
HostStartOptions hostStartOptions = new()
135+
{
136+
LoadProfiles = initializationOptions?.GetValue("EnableProfileLoading")?.Value<bool>() ?? false,
137+
// TODO: Consider deprecating the setting which sets this and
138+
// instead use WorkspacePath exclusively.
139+
InitialWorkingDirectory = initializationOptions?.GetValue("InitialWorkingDirectory")?.Value<string>() ?? workspaceService.WorkspacePath
140+
};
141+
142+
_psesHost = languageServer.Services.GetService<PsesInternalHost>();
143+
return _psesHost.TryStartAsync(hostStartOptions, cancellationToken);
144144
});
145145
}).ConfigureAwait(false);
146146

‎src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs

-4
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,6 @@ public PsesInternalHost(
169169

170170
public bool IsRunning => _isRunningLatch.IsSignaled;
171171

172-
public string InitialWorkingDirectory { get; private set; }
173-
174172
public Task Shutdown => _stopped.Task;
175173

176174
IRunspaceInfo IRunspaceContext.CurrentRunspace => CurrentRunspace;
@@ -442,8 +440,6 @@ internal Task LoadHostProfilesAsync(CancellationToken cancellationToken)
442440

443441
public Task SetInitialWorkingDirectoryAsync(string path, CancellationToken cancellationToken)
444442
{
445-
InitialWorkingDirectory = path;
446-
447443
return ExecutePSCommandAsync(
448444
new PSCommand().AddCommand("Set-Location").AddParameter("LiteralPath", path),
449445
cancellationToken);

‎src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs

+1-73
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,13 @@
33

44
using System;
55
using System.Collections.Generic;
6-
using System.IO;
76
using System.Threading;
87
using System.Threading.Tasks;
98
using MediatR;
109
using Microsoft.Extensions.Logging;
1110
using Microsoft.PowerShell.EditorServices.Logging;
1211
using Microsoft.PowerShell.EditorServices.Services;
1312
using Microsoft.PowerShell.EditorServices.Services.Configuration;
14-
using Microsoft.PowerShell.EditorServices.Services.Extension;
15-
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
1613
using Newtonsoft.Json.Linq;
1714
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
1815
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
@@ -26,27 +23,19 @@ internal class PsesConfigurationHandler : DidChangeConfigurationHandlerBase
2623
private readonly ILogger _logger;
2724
private readonly WorkspaceService _workspaceService;
2825
private readonly ConfigurationService _configurationService;
29-
private readonly ExtensionService _extensionService;
30-
private readonly PsesInternalHost _psesHost;
3126
private readonly ILanguageServerFacade _languageServer;
32-
private bool _profilesLoaded;
33-
private bool _cwdSet;
3427

3528
public PsesConfigurationHandler(
3629
ILoggerFactory factory,
3730
WorkspaceService workspaceService,
3831
AnalysisService analysisService,
3932
ConfigurationService configurationService,
40-
ILanguageServerFacade languageServer,
41-
ExtensionService extensionService,
42-
PsesInternalHost psesHost)
33+
ILanguageServerFacade languageServer)
4334
{
4435
_logger = factory.CreateLogger<PsesConfigurationHandler>();
4536
_workspaceService = workspaceService;
4637
_configurationService = configurationService;
4738
_languageServer = languageServer;
48-
_extensionService = extensionService;
49-
_psesHost = psesHost;
5039

5140
ConfigurationUpdated += analysisService.OnConfigurationUpdated;
5241
}
@@ -63,7 +52,6 @@ public override async Task<Unit> Handle(DidChangeConfigurationParams request, Ca
6352

6453
SendFeatureChangesTelemetry(incomingSettings);
6554

66-
bool profileLoadingPreviouslyEnabled = _configurationService.CurrentSettings.EnableProfileLoading;
6755
bool oldScriptAnalysisEnabled = _configurationService.CurrentSettings.ScriptAnalysis.Enable;
6856
string oldScriptAnalysisSettingsPath = _configurationService.CurrentSettings.ScriptAnalysis?.SettingsPath;
6957

@@ -72,66 +60,6 @@ public override async Task<Unit> Handle(DidChangeConfigurationParams request, Ca
7260
_workspaceService.WorkspacePath,
7361
_logger);
7462

75-
// We need to load the profiles if:
76-
// - Profile loading is configured, AND
77-
// - Profiles haven't been loaded before, OR
78-
// - The profile loading configuration just changed
79-
bool loadProfiles = _configurationService.CurrentSettings.EnableProfileLoading
80-
&& (!_profilesLoaded || !profileLoadingPreviouslyEnabled);
81-
82-
if (!_psesHost.IsRunning)
83-
{
84-
_logger.LogTrace("Starting command loop");
85-
86-
if (loadProfiles)
87-
{
88-
_logger.LogTrace("Loading profiles...");
89-
}
90-
91-
await _psesHost.TryStartAsync(new HostStartOptions { LoadProfiles = loadProfiles }, CancellationToken.None).ConfigureAwait(false);
92-
93-
if (loadProfiles)
94-
{
95-
_profilesLoaded = true;
96-
_logger.LogTrace("Loaded!");
97-
}
98-
}
99-
100-
// TODO: Load profiles when the host is already running? Note that this might mess up
101-
// the ordering and require the foreground.
102-
if (!_cwdSet)
103-
{
104-
if (!string.IsNullOrEmpty(_configurationService.CurrentSettings.Cwd)
105-
&& Directory.Exists(_configurationService.CurrentSettings.Cwd))
106-
{
107-
_logger.LogTrace($"Setting CWD (from config) to {_configurationService.CurrentSettings.Cwd}");
108-
await _psesHost.SetInitialWorkingDirectoryAsync(
109-
_configurationService.CurrentSettings.Cwd,
110-
CancellationToken.None).ConfigureAwait(false);
111-
}
112-
else if (_workspaceService.WorkspacePath is not null
113-
&& Directory.Exists(_workspaceService.WorkspacePath))
114-
{
115-
_logger.LogTrace($"Setting CWD (from workspace) to {_workspaceService.WorkspacePath}");
116-
await _psesHost.SetInitialWorkingDirectoryAsync(
117-
_workspaceService.WorkspacePath,
118-
CancellationToken.None).ConfigureAwait(false);
119-
}
120-
else
121-
{
122-
_logger.LogTrace("Tried to set CWD but in bad state");
123-
}
124-
125-
_cwdSet = true;
126-
}
127-
128-
// This is another place we call this to setup $psEditor, which really needs to be done
129-
// _before_ profiles. In current testing, this has already been done by the call to
130-
// InitializeAsync when the ExtensionService class is injected.
131-
//
132-
// TODO: Remove this.
133-
await _extensionService.InitializeAsync().ConfigureAwait(false);
134-
13563
// Run any events subscribed to configuration updates
13664
_logger.LogTrace("Running configuration update event handlers");
13765
ConfigurationUpdated?.Invoke(this, _configurationService.CurrentSettings);

0 commit comments

Comments
 (0)