From d1c4ebafc4d4b8e9e79b932520958c35e8a08832 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 12 May 2022 14:33:51 -0700 Subject: [PATCH 1/5] Add `ASSEMBLY_LOAD_STACKTRACE` guard Otherwise this is incredibly noisy with diagnostic logging. --- .../EditorServicesLoader.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index bc2130076..781da68ef 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -12,7 +12,7 @@ using System.Management.Automation; using System.Management.Automation.Runspaces; -#if DEBUG +#if ASSEMBLY_LOAD_STACKTRACE using System.Diagnostics; #endif @@ -98,7 +98,7 @@ public static EditorServicesLoader Create( AssemblyLoadContext.Default.Resolving += (AssemblyLoadContext _, AssemblyName asmName) => { -#if DEBUG +#if ASSEMBLY_LOAD_STACKTRACE logger.Log(PsesLogLevel.Diagnostic, $"Assembly resolve event fired for {asmName}. Stacktrace:\n{new StackTrace()}"); #else logger.Log(PsesLogLevel.Diagnostic, $"Assembly resolve event fired for {asmName}"); @@ -138,7 +138,7 @@ public static EditorServicesLoader Create( // Unlike in .NET Core, we need to be look for all dependencies in .NET Framework, not just PSES.dll AppDomain.CurrentDomain.AssemblyResolve += (object sender, ResolveEventArgs args) => { -#if DEBUG +#if ASSEMBLY_LOAD_STACKTRACE logger.Log(PsesLogLevel.Diagnostic, $"Assembly resolve event fired for {args.Name}. Stacktrace:\n{new StackTrace()}"); #else logger.Log(PsesLogLevel.Diagnostic, $"Assembly resolve event fired for {args.Name}"); From a430513665ccf6f3b9d595018648f5072cd50da3 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 12 May 2022 14:48:52 -0700 Subject: [PATCH 2/5] Clean up `NullPSHost` classes --- .../PowerShell/Host/NullPSHostRawUI.cs | 42 ++++++------- .../Services/PowerShell/Host/NullPSHostUI.cs | 62 +++++++------------ 2 files changed, 41 insertions(+), 63 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostRawUI.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostRawUI.cs index 8d5774780..4bfb94fa7 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostRawUI.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostRawUI.cs @@ -12,44 +12,38 @@ internal class NullPSHostRawUI : PSHostRawUserInterface public NullPSHostRawUI() => _buffer = new BufferCell[0, 0]; - public override ConsoleColor BackgroundColor { get; set; } - public override Size BufferSize { get; set; } - public override Coordinates CursorPosition { get; set; } - public override int CursorSize { get; set; } - public override ConsoleColor ForegroundColor { get; set; } + public override Coordinates WindowPosition { get; set; } - public override bool KeyAvailable => false; + public override Size MaxWindowSize => new() { Width = _buffer.GetLength(0), Height = _buffer.GetLength(1) }; public override Size MaxPhysicalWindowSize => MaxWindowSize; - public override Size MaxWindowSize => new() { Width = _buffer.GetLength(0), Height = _buffer.GetLength(1) }; + public override bool KeyAvailable => false; + + public override ConsoleColor ForegroundColor { get; set; } + + public override int CursorSize { get; set; } + + public override Coordinates CursorPosition { get; set; } + + public override Size BufferSize { get; set; } + + public override ConsoleColor BackgroundColor { get; set; } - public override Coordinates WindowPosition { get; set; } public override Size WindowSize { get; set; } + public override string WindowTitle { get; set; } - public override void FlushInputBuffer() - { - // Do nothing - } + public override void FlushInputBuffer() { } public override BufferCell[,] GetBufferContents(Rectangle rectangle) => _buffer; public override KeyInfo ReadKey(ReadKeyOptions options) => default; - public override void ScrollBufferContents(Rectangle source, Coordinates destination, Rectangle clip, BufferCell fill) - { - // Do nothing - } + public override void ScrollBufferContents(Rectangle source, Coordinates destination, Rectangle clip, BufferCell fill) { } - public override void SetBufferContents(Coordinates origin, BufferCell[,] contents) - { - // Do nothing - } + public override void SetBufferContents(Coordinates origin, BufferCell[,] contents) { } - public override void SetBufferContents(Rectangle rectangle, BufferCell fill) - { - // Do nothing - } + public override void SetBufferContents(Rectangle rectangle, BufferCell fill) { } } } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostUI.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostUI.cs index 2b8fb7f50..2d3af9eb0 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostUI.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/NullPSHostUI.cs @@ -14,6 +14,8 @@ internal class NullPSHostUI : PSHostUserInterface { public NullPSHostUI() => RawUI = new NullPSHostRawUI(); + public override bool SupportsVirtualTerminal => false; + public override PSHostRawUserInterface RawUI { get; } public override Dictionary Prompt(string caption, string message, Collection descriptions) => new(); @@ -29,44 +31,26 @@ public override PSCredential PromptForCredential(string caption, string message, public override SecureString ReadLineAsSecureString() => new(); - public override void Write(ConsoleColor foregroundColor, ConsoleColor backgroundColor, string value) - { - // Do nothing - } - - public override void Write(string value) - { - // Do nothing - } - - public override void WriteDebugLine(string message) - { - // Do nothing - } - - public override void WriteErrorLine(string value) - { - // Do nothing - } - - public override void WriteLine(string value) - { - // Do nothing - } - - public override void WriteProgress(long sourceId, ProgressRecord record) - { - // Do nothing - } - - public override void WriteVerboseLine(string message) - { - // Do nothing - } - - public override void WriteWarningLine(string message) - { - // Do nothing - } + public override void Write(ConsoleColor foregroundColor, ConsoleColor backgroundColor, string value) { } + + public override void Write(string value) { } + + public override void WriteDebugLine(string message) { } + + public override void WriteErrorLine(string value) { } + + public override void WriteInformation(InformationRecord record) { } + + public override void WriteLine() { } + + public override void WriteLine(ConsoleColor foregroundColor, ConsoleColor backgroundColor, string value) { } + + public override void WriteLine(string value) { } + + public override void WriteProgress(long sourceId, ProgressRecord record) { } + + public override void WriteVerboseLine(string message) { } + + public override void WriteWarningLine(string message) { } } } From 846045ee70b9980324a0047f512f0601971e30d7 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 12 May 2022 14:37:48 -0700 Subject: [PATCH 3/5] Fix miscellaneous logging, typos, cancellation tokens, and notes --- .../EditorServicesLoader.cs | 1 + .../Hosting/EditorServicesServerFactory.cs | 1 + .../Server/PsesDebugServer.cs | 19 ++++++------------- .../Server/PsesLanguageServer.cs | 4 ---- .../Server/PsesServiceCollectionExtensions.cs | 9 ++++----- .../PowerShell/Host/PsesInternalHost.cs | 14 +++++++++----- .../Services/Symbols/Vistors/AstOperations.cs | 9 ++------- .../TextDocument/Handlers/CodeLensHandlers.cs | 2 +- .../Handlers/CompletionHandler.cs | 1 + .../Handlers/DocumentHighlightHandler.cs | 10 +++++----- 10 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index 781da68ef..924619283 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -208,6 +208,7 @@ public Task LoadAndRunEditorServicesAsync() #endif // Add the bundled modules to the PSModulePath + // TODO: Why do we do this in addition to passing the bundled module path to the host? UpdatePSModulePath(); // Check to see if the configuration we have is valid diff --git a/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs b/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs index e9704e8cd..6a8d07f17 100644 --- a/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs +++ b/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs @@ -152,6 +152,7 @@ public PsesDebugServer CreateDebugServerForTempSession( .AddSerilog() .SetMinimumLevel(LogLevel.Trace)) // TODO: Why randomly set to trace? .AddSingleton(_ => null) + // TODO: Why add these for a debug server?! .AddPsesLanguageServices(hostStartupInfo) // For a Temp session, there is no LanguageServer so just set it to null .AddSingleton( diff --git a/src/PowerShellEditorServices/Server/PsesDebugServer.cs b/src/PowerShellEditorServices/Server/PsesDebugServer.cs index b84c2cbf6..0e79946ef 100644 --- a/src/PowerShellEditorServices/Server/PsesDebugServer.cs +++ b/src/PowerShellEditorServices/Server/PsesDebugServer.cs @@ -3,7 +3,6 @@ using System; using System.IO; -using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -23,15 +22,10 @@ internal class PsesDebugServer : IDisposable private readonly Stream _inputStream; private readonly Stream _outputStream; private readonly TaskCompletionSource _serverStopped; - private DebugAdapterServer _debugAdapterServer; - private PsesInternalHost _psesHost; - private bool _startedPses; - private readonly bool _isTemp; - protected readonly ILoggerFactory _loggerFactory; public PsesDebugServer( @@ -91,10 +85,12 @@ public async Task StartAsync() .WithHandler() // The OnInitialize delegate gets run when we first receive the _Initialize_ request: // https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Initialize - .OnInitialize(async (server, _, _) => + .OnInitialize(async (server, _, cancellationToken) => { - // We need to make sure the host has been started - _startedPses = !await _psesHost.TryStartAsync(new HostStartOptions(), CancellationToken.None).ConfigureAwait(false); + // Start the host if not already started, and enable debug mode (required + // for remote debugging). + _startedPses = !await _psesHost.TryStartAsync(new HostStartOptions(), cancellationToken).ConfigureAwait(false); + _psesHost.DebugContext.EnableDebugMode(); // We need to give the host a handle to the DAP so it can register // notifications (specifically for sendKeyPress). @@ -103,11 +99,8 @@ public async Task StartAsync() _psesHost.DebugServer = server; } - // Ensure the debugger mode is set correctly - this is required for remote debugging to work - _psesHost.DebugContext.EnableDebugMode(); - + // Clear any existing breakpoints before proceeding. BreakpointService breakpointService = server.GetService(); - // Clear any existing breakpoints before proceeding await breakpointService.RemoveAllBreakpointsAsync().ConfigureAwait(false); }) // The OnInitialized delegate gets run right before the server responds to the _Initialize_ request: diff --git a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs index 204ae88d9..946c0b823 100644 --- a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs +++ b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs @@ -24,15 +24,12 @@ namespace Microsoft.PowerShell.EditorServices.Server internal class PsesLanguageServer { internal ILoggerFactory LoggerFactory { get; } - internal ILanguageServer LanguageServer { get; private set; } - private readonly LogLevel _minimumLogLevel; private readonly Stream _inputStream; private readonly Stream _outputStream; private readonly HostStartupInfo _hostDetails; private readonly TaskCompletionSource _serverStart; - private PsesInternalHost _psesHost; /// @@ -156,7 +153,6 @@ public async Task StartAsync() /// A task that completes when the server is shut down. public async Task WaitForShutdown() { - Log.Logger.Debug("Shutting down OmniSharp Language Server"); await _serverStart.Task.ConfigureAwait(false); await LanguageServer.WaitForExit.ConfigureAwait(false); diff --git a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs index 2284b9444..c552515de 100644 --- a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs +++ b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs @@ -44,11 +44,10 @@ public static IServiceCollection AddPsesLanguageServices( provider.GetService(), provider.GetService()); - // This is where we create the $psEditor variable - // so that when the console is ready, it will be available - // TODO: Improve the sequencing here so that: - // - The variable is guaranteed to be initialized when the console first appears - // - Any errors that occur are handled rather than lost by the unawaited task + // This is where we create the $psEditor variable so that when the console + // is ready, it will be available. NOTE: We cannot await this because it + // uses a lazy initialization to avoid a race with the dependency injection + // framework, see the EditorObject class for that! extensionService.InitializeAsync(); return extensionService; diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 3467b80aa..1d0d1a7ce 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -102,7 +102,7 @@ public PsesInternalHost( // Respect a user provided bundled module path. if (Directory.Exists(hostInfo.BundledModulePath)) { - _logger.LogTrace("Using new bundled module path: {}", hostInfo.BundledModulePath); + _logger.LogTrace($"Using new bundled module path: {hostInfo.BundledModulePath}"); s_bundledModulePath = hostInfo.BundledModulePath; } @@ -236,7 +236,7 @@ public override void SetShouldExit(int exitCode) /// A task that resolves when the host has finished startup, with the value true if the caller started the host, and false otherwise. public async Task TryStartAsync(HostStartOptions startOptions, CancellationToken cancellationToken) { - _logger.LogInformation("Host starting"); + _logger.LogDebug("Starting host..."); if (!_isRunningLatch.TryEnter()) { _logger.LogDebug("Host start requested after already started."); @@ -248,13 +248,16 @@ public async Task TryStartAsync(HostStartOptions startOptions, Cancellatio if (startOptions.LoadProfiles) { + _logger.LogDebug("Loading profiles..."); await LoadHostProfilesAsync(cancellationToken).ConfigureAwait(false); - _logger.LogInformation("Profiles loaded"); + _logger.LogDebug("Profiles loaded!"); } if (startOptions.InitialWorkingDirectory is not null) { - await SetInitialWorkingDirectoryAsync(startOptions.InitialWorkingDirectory, CancellationToken.None).ConfigureAwait(false); + _logger.LogDebug($"Setting InitialWorkingDirectory to {startOptions.InitialWorkingDirectory}..."); + await SetInitialWorkingDirectoryAsync(startOptions.InitialWorkingDirectory, cancellationToken).ConfigureAwait(false); + _logger.LogDebug("InitialWorkingDirectory set!"); } await _started.Task.ConfigureAwait(false); @@ -269,6 +272,7 @@ public Task StopAsync() public void TriggerShutdown() { + _logger.LogDebug("Shutting down host..."); if (Interlocked.Exchange(ref _shuttingDown, 1) == 0) { _cancellationContext.CancelCurrentTaskStack(); @@ -680,7 +684,7 @@ private void RunTopLevelExecutionLoop() return; } - _logger.LogInformation("PSES pipeline thread loop shutting down"); + _logger.LogDebug("PSES pipeline thread loop shutting down"); _stopped.SetResult(true); } diff --git a/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs b/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs index ee364107c..0a26b2eaa 100644 --- a/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs +++ b/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs @@ -76,12 +76,7 @@ public static async Task GetCompletionsAsync( { IScriptPosition cursorPosition = s_clonePositionWithNewOffset(scriptAst.Extent.StartScriptPosition, fileOffset); - logger.LogTrace( - string.Format( - "Getting completions at offset {0} (line: {1}, column: {2})", - fileOffset, - cursorPosition.LineNumber, - cursorPosition.ColumnNumber)); + logger.LogTrace($"Getting completions at offset {fileOffset} (line: {cursorPosition.LineNumber}, column: {cursorPosition.ColumnNumber})"); Stopwatch stopwatch = new(); @@ -103,7 +98,7 @@ await executionService.ExecuteDelegateAsync( .ConfigureAwait(false); stopwatch.Stop(); - logger.LogTrace($"IntelliSense completed in {stopwatch.ElapsedMilliseconds}ms."); + logger.LogTrace($"IntelliSense completed in {stopwatch.ElapsedMilliseconds}ms: {commandCompletion}"); return commandCompletion; } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs index 108dace6c..b0a1717dc 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs @@ -47,7 +47,7 @@ public override Task Handle(CodeLensParams request, Cancellat public override Task Handle(CodeLens request, CancellationToken cancellationToken) { - // TODO: Catch deserializtion exception on bad object + // TODO: Catch deserialization exception on bad object CodeLensData codeLensData = request.Data.ToObject(); ICodeLensProvider originalProvider = _symbolsService diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 4bf174cac..6c2c4db0d 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -144,6 +144,7 @@ internal async Task> GetCompletionsInFileAsync( for (int i = 0; i < result.CompletionMatches.Count; i++) { completionItems[i] = CreateCompletionItem(result.CompletionMatches[i], replacedRange, i + 1); + _logger.LogTrace("Created completion item: " + completionItems[i] + " with " + completionItems[i].TextEdit); } return completionItems; } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentHighlightHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentHighlightHandler.cs index 99fe542e4..8e318f3bc 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentHighlightHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentHighlightHandler.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services; using Microsoft.PowerShell.EditorServices.Services.Symbols; @@ -9,9 +12,6 @@ using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities; using OmniSharp.Extensions.LanguageServer.Protocol.Document; using OmniSharp.Extensions.LanguageServer.Protocol.Models; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Handlers { @@ -27,7 +27,6 @@ public PsesDocumentHighlightHandler( { _logger = loggerFactory.CreateLogger(); _workspaceService = workspaceService; - _logger.LogInformation("highlight handler loaded"); } protected override DocumentHighlightRegistrationOptions CreateRegistrationOptions(DocumentHighlightCapability capability, ClientCapabilities clientCapabilities) => new() @@ -46,7 +45,7 @@ public override Task Handle( request.Position.Line + 1, request.Position.Character + 1); - if (symbolOccurrences == null) + if (symbolOccurrences is null) { return Task.FromResult(s_emptyHighlightContainer); } @@ -60,6 +59,7 @@ public override Task Handle( Range = symbolOccurrences[i].ScriptRegion.ToRange() }; } + _logger.LogDebug("Highlights: " + highlights); return Task.FromResult(new DocumentHighlightContainer(highlights)); } From 78f8fb7643c2b6eae773f66f6ae46192193ff2c8 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 13 May 2022 10:49:02 -0700 Subject: [PATCH 4/5] Handle host initialization in `Initialize` request delegate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Server/PsesLanguageServer.cs | 38 +++++----- .../PowerShell/Host/PsesInternalHost.cs | 4 - .../Handlers/ConfigurationHandler.cs | 74 +------------------ 3 files changed, 20 insertions(+), 96 deletions(-) diff --git a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs index 946c0b823..e9aae314a 100644 --- a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs +++ b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.IO; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; @@ -12,6 +11,8 @@ using Microsoft.PowerShell.EditorServices.Services.Extension; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; using Microsoft.PowerShell.EditorServices.Services.Template; +using Newtonsoft.Json.Linq; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; using OmniSharp.Extensions.LanguageServer.Protocol.Server; using OmniSharp.Extensions.LanguageServer.Server; using Serilog; @@ -114,33 +115,32 @@ public async Task StartAsync() // _Initialize_ request: // https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize .OnInitialize( - (languageServer, request, _) => + (languageServer, initializeParams, cancellationToken) => { - Log.Logger.Debug("Initializing OmniSharp Language Server"); - - IServiceProvider serviceProvider = languageServer.Services; - - _psesHost = serviceProvider.GetService(); - - WorkspaceService workspaceService = serviceProvider.GetService(); - - // Grab the workspace path from the parameters - if (request.RootUri != null) + // Set the workspace path from the parameters. + WorkspaceService workspaceService = languageServer.Services.GetService(); + if (initializeParams.WorkspaceFolders is not null) { - workspaceService.WorkspacePath = request.RootUri.GetFileSystemPath(); - } - else if (request.WorkspaceFolders != null) - { - // If RootUri isn't set, try to use the first WorkspaceFolder. // TODO: Support multi-workspace. - foreach (OmniSharp.Extensions.LanguageServer.Protocol.Models.WorkspaceFolder workspaceFolder in request.WorkspaceFolders) + foreach (WorkspaceFolder workspaceFolder in initializeParams.WorkspaceFolders) { workspaceService.WorkspacePath = workspaceFolder.Uri.GetFileSystemPath(); break; } } - return Task.CompletedTask; + // Parse initialization options. + JObject initializationOptions = initializeParams.InitializationOptions as JObject; + HostStartOptions hostStartOptions = new() + { + LoadProfiles = initializationOptions?.GetValue("EnableProfileLoading")?.Value() ?? false, + // TODO: Consider deprecating the setting which sets this and + // instead use WorkspacePath exclusively. + InitialWorkingDirectory = initializationOptions?.GetValue("InitialWorkingDirectory")?.Value() ?? workspaceService.WorkspacePath + }; + + _psesHost = languageServer.Services.GetService(); + return _psesHost.TryStartAsync(hostStartOptions, cancellationToken); }); }).ConfigureAwait(false); diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 1d0d1a7ce..aa87675e6 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -169,8 +169,6 @@ public PsesInternalHost( public bool IsRunning => _isRunningLatch.IsSignaled; - public string InitialWorkingDirectory { get; private set; } - public Task Shutdown => _stopped.Task; IRunspaceInfo IRunspaceContext.CurrentRunspace => CurrentRunspace; @@ -442,8 +440,6 @@ internal Task LoadHostProfilesAsync(CancellationToken cancellationToken) public Task SetInitialWorkingDirectoryAsync(string path, CancellationToken cancellationToken) { - InitialWorkingDirectory = path; - return ExecutePSCommandAsync( new PSCommand().AddCommand("Set-Location").AddParameter("LiteralPath", path), cancellationToken); diff --git a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs index f0020f122..ca0361a89 100644 --- a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs +++ b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.IO; using System.Threading; using System.Threading.Tasks; using MediatR; @@ -11,8 +10,6 @@ using Microsoft.PowerShell.EditorServices.Logging; using Microsoft.PowerShell.EditorServices.Services; using Microsoft.PowerShell.EditorServices.Services.Configuration; -using Microsoft.PowerShell.EditorServices.Services.Extension; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; using Newtonsoft.Json.Linq; using OmniSharp.Extensions.LanguageServer.Protocol.Models; using OmniSharp.Extensions.LanguageServer.Protocol.Server; @@ -26,27 +23,19 @@ internal class PsesConfigurationHandler : DidChangeConfigurationHandlerBase private readonly ILogger _logger; private readonly WorkspaceService _workspaceService; private readonly ConfigurationService _configurationService; - private readonly ExtensionService _extensionService; - private readonly PsesInternalHost _psesHost; private readonly ILanguageServerFacade _languageServer; - private bool _profilesLoaded; - private bool _cwdSet; public PsesConfigurationHandler( ILoggerFactory factory, WorkspaceService workspaceService, AnalysisService analysisService, ConfigurationService configurationService, - ILanguageServerFacade languageServer, - ExtensionService extensionService, - PsesInternalHost psesHost) + ILanguageServerFacade languageServer) { _logger = factory.CreateLogger(); _workspaceService = workspaceService; _configurationService = configurationService; _languageServer = languageServer; - _extensionService = extensionService; - _psesHost = psesHost; ConfigurationUpdated += analysisService.OnConfigurationUpdated; } @@ -63,7 +52,6 @@ public override async Task Handle(DidChangeConfigurationParams request, Ca SendFeatureChangesTelemetry(incomingSettings); - bool profileLoadingPreviouslyEnabled = _configurationService.CurrentSettings.EnableProfileLoading; bool oldScriptAnalysisEnabled = _configurationService.CurrentSettings.ScriptAnalysis.Enable; string oldScriptAnalysisSettingsPath = _configurationService.CurrentSettings.ScriptAnalysis?.SettingsPath; @@ -72,66 +60,6 @@ public override async Task Handle(DidChangeConfigurationParams request, Ca _workspaceService.WorkspacePath, _logger); - // We need to load the profiles if: - // - Profile loading is configured, AND - // - Profiles haven't been loaded before, OR - // - The profile loading configuration just changed - bool loadProfiles = _configurationService.CurrentSettings.EnableProfileLoading - && (!_profilesLoaded || !profileLoadingPreviouslyEnabled); - - if (!_psesHost.IsRunning) - { - _logger.LogTrace("Starting command loop"); - - if (loadProfiles) - { - _logger.LogTrace("Loading profiles..."); - } - - await _psesHost.TryStartAsync(new HostStartOptions { LoadProfiles = loadProfiles }, CancellationToken.None).ConfigureAwait(false); - - if (loadProfiles) - { - _profilesLoaded = true; - _logger.LogTrace("Loaded!"); - } - } - - // TODO: Load profiles when the host is already running? Note that this might mess up - // the ordering and require the foreground. - if (!_cwdSet) - { - if (!string.IsNullOrEmpty(_configurationService.CurrentSettings.Cwd) - && Directory.Exists(_configurationService.CurrentSettings.Cwd)) - { - _logger.LogTrace($"Setting CWD (from config) to {_configurationService.CurrentSettings.Cwd}"); - await _psesHost.SetInitialWorkingDirectoryAsync( - _configurationService.CurrentSettings.Cwd, - CancellationToken.None).ConfigureAwait(false); - } - else if (_workspaceService.WorkspacePath is not null - && Directory.Exists(_workspaceService.WorkspacePath)) - { - _logger.LogTrace($"Setting CWD (from workspace) to {_workspaceService.WorkspacePath}"); - await _psesHost.SetInitialWorkingDirectoryAsync( - _workspaceService.WorkspacePath, - CancellationToken.None).ConfigureAwait(false); - } - else - { - _logger.LogTrace("Tried to set CWD but in bad state"); - } - - _cwdSet = true; - } - - // This is another place we call this to setup $psEditor, which really needs to be done - // _before_ profiles. In current testing, this has already been done by the call to - // InitializeAsync when the ExtensionService class is injected. - // - // TODO: Remove this. - await _extensionService.InitializeAsync().ConfigureAwait(false); - // Run any events subscribed to configuration updates _logger.LogTrace("Running configuration update event handlers"); ConfigurationUpdated?.Invoke(this, _configurationService.CurrentSettings); From c1dd1dbc5784dce44f5637769103cf8e97ec764c Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 13 May 2022 13:08:02 -0700 Subject: [PATCH 5/5] =?UTF-8?q?Fix=20completion=20text=20for=20clients=20w?= =?UTF-8?q?hich=20don=E2=80=99t=20support=20`TextEdit`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We could set the `Label` to `CompletionText` as the fallback, but that then makes the list of completions in fancier clients not as accurate (since PowerShell explicitly provides `ListItemText` for this purpose). Instead, we set usually unused field `InsertText` with the same information we provided for more advanced clients in `TextEdit`. --- .../Services/TextDocument/Handlers/CompletionHandler.cs | 5 ++++- .../Completion/CompleteAttributeValue.cs | 3 +++ .../Completion/CompleteCommandFromModule.cs | 1 + .../Completion/CompleteCommandInFile.cs | 1 + .../Completion/CompleteNamespace.cs | 1 + .../Completion/CompleteTypeName.cs | 1 + .../Completion/CompleteVariableInFile.cs | 1 + 7 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 6c2c4db0d..dfe0d82d5 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -185,7 +185,10 @@ internal static CompletionItem CreateCompletionItem( // Retain PowerShell's sort order with the given index. SortText = $"{sortIndex:D4}{result.ListItemText}", FilterText = result.CompletionText, - TextEdit = textEdit // Used instead of InsertText. + // Used instead of Label when TextEdit is unsupported + InsertText = result.CompletionText, + // Used instead of InsertText when possible + TextEdit = textEdit }; return result.ResultType switch diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs index 317cc6277..d38c430a4 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs @@ -23,6 +23,7 @@ internal static class CompleteAttributeValue Kind = CompletionItemKind.Property, Detail = "System.Boolean ValueFromPipeline", FilterText = "ValueFromPipeline", + InsertText = "ValueFromPipeline", Label = "ValueFromPipeline", SortText = "0001ValueFromPipeline", TextEdit = new TextEdit @@ -41,6 +42,7 @@ internal static class CompleteAttributeValue Kind = CompletionItemKind.Property, Detail = "System.Boolean ValueFromPipelineByPropertyName", FilterText = "ValueFromPipelineByPropertyName", + InsertText = "ValueFromPipelineByPropertyName", Label = "ValueFromPipelineByPropertyName", SortText = "0002ValueFromPipelineByPropertyName", TextEdit = new TextEdit @@ -59,6 +61,7 @@ internal static class CompleteAttributeValue Kind = CompletionItemKind.Property, Detail = "System.Boolean ValueFromRemainingArguments", FilterText = "ValueFromRemainingArguments", + InsertText = "ValueFromRemainingArguments", Label = "ValueFromRemainingArguments", SortText = "0003ValueFromRemainingArguments", TextEdit = new TextEdit diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs index 709d66563..dff679717 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs @@ -26,6 +26,7 @@ internal static class CompleteCommandFromModule Kind = CompletionItemKind.Function, Detail = "", // OS-dependent, checked separately. FilterText = "Get-Random", + InsertText = "Get-Random", Label = "Get-Random", SortText = "0001Get-Random", TextEdit = new TextEdit diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs index cf8bb6855..80a1cc906 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs @@ -23,6 +23,7 @@ internal static class CompleteCommandInFile Kind = CompletionItemKind.Function, Detail = "", FilterText = "Get-Something", + InsertText = "Get-Something", Label = "Get-Something", SortText = "0001Get-Something", TextEdit = new TextEdit diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs index 66c111871..8d24377c3 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs @@ -23,6 +23,7 @@ internal static class CompleteNamespace Kind = CompletionItemKind.Module, Detail = "Namespace System.Collections", FilterText = "System.Collections", + InsertText = "System.Collections", Label = "Collections", SortText = "0001Collections", TextEdit = new TextEdit diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs index 867f067b4..38bba7c0b 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs @@ -23,6 +23,7 @@ internal static class CompleteTypeName Kind = CompletionItemKind.TypeParameter, Detail = "System.Collections.ArrayList", FilterText = "System.Collections.ArrayList", + InsertText = "System.Collections.ArrayList", Label = "ArrayList", SortText = "0001ArrayList", TextEdit = new TextEdit diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs index 394962379..361bec108 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs @@ -23,6 +23,7 @@ internal static class CompleteVariableInFile Kind = CompletionItemKind.Variable, Detail = "", // Same as label, so not shown. FilterText = "$testVar1", + InsertText = "$testVar1", Label = "testVar1", SortText = "0001testVar1", TextEdit = new TextEdit