From 03d8989e1d1f5a934bb32c802e6d8aa0066321fa Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 28 Apr 2022 11:22:05 -0400 Subject: [PATCH 1/2] Revert "Consolidate `InterruptCurrentForeground` and `MustRunInForeground` (#1777)" This reverts commit 39c9ed458d0df24c26a672bcbfc6e7d8721f7bb5. --- .../Handlers/ConfigurationDoneHandler.cs | 4 +- .../Services/Extension/ExtensionService.cs | 5 +- .../PowerShell/Console/PsrlReadLine.cs | 6 +- .../Execution/BlockingConcurrentDeque.cs | 2 +- .../PowerShell/Execution/ExecutionOptions.cs | 6 +- .../PowerShell/Handlers/EvaluateHandler.cs | 5 +- .../PowerShell/Host/PsesInternalHost.cs | 64 ++++++++----------- .../Services/Template/TemplateService.cs | 16 ++--- 8 files changed, 43 insertions(+), 65 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs index d22dde4ee..74595831e 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs @@ -25,11 +25,9 @@ internal class ConfigurationDoneHandler : IConfigurationDoneHandler // TODO: We currently set `WriteInputToHost` as true, which writes our debugged commands' // `GetInvocationText` and that reveals some obscure implementation details we should // instead hide from the user with pretty strings (or perhaps not write out at all). - // - // This API is mostly used for F5 execution so it requires the foreground. private static readonly PowerShellExecutionOptions s_debuggerExecutionOptions = new() { - RequiresForeground = true, + MustRunInForeground = true, WriteInputToHost = true, WriteOutputToHost = true, ThrowOnError = false, diff --git a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs index f4284f934..eb8bd2859 100644 --- a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs +++ b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs @@ -129,16 +129,17 @@ public Task InvokeCommandAsync(string commandName, EditorContext editorContext, .AddParameter("ScriptBlock", editorCommand.ScriptBlock) .AddParameter("ArgumentList", new object[] { editorContext }); - // This API is used for editor command execution so it requires the foreground. + // This API is used for editor command execution, so it needs to interrupt the + // current prompt (or other foreground task). return ExecutionService.ExecutePSCommandAsync( executeCommand, cancellationToken, new PowerShellExecutionOptions { - RequiresForeground = true, WriteOutputToHost = !editorCommand.SuppressOutput, AddToHistory = !editorCommand.SuppressOutput, ThrowOnError = false, + InterruptCurrentForeground = true }); } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs b/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs index cda3af925..64e3627fa 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs @@ -32,11 +32,7 @@ public PsrlReadLine( _psrlProxy.OverrideIdleHandler(onIdleAction); } - public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate( - representation: "ReadLine", - new ExecutionOptions { RequiresForeground = true }, - InvokePSReadLine, - cancellationToken); + public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate(representation: "ReadLine", new ExecutionOptions { MustRunInForeground = true }, InvokePSReadLine, cancellationToken); protected override ConsoleKeyInfo ReadKey(CancellationToken cancellationToken) => _psesHost.ReadKey(intercept: true, cancellationToken); diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs index 2e5b4f733..035437dd3 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs @@ -65,7 +65,7 @@ public bool TryTake(out T item) public IDisposable BlockConsumers() => PriorityQueueBlockLifetime.StartBlocking(_blockConsumersEvent); - public void Dispose() => _blockConsumersEvent.Dispose(); + public void Dispose() => ((IDisposable)_blockConsumersEvent).Dispose(); private class PriorityQueueBlockLifetime : IDisposable { diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs index 83de5301c..27bb8935a 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs @@ -16,7 +16,8 @@ public enum ExecutionPriority public record ExecutionOptions { public ExecutionPriority Priority { get; init; } = ExecutionPriority.Normal; - public bool RequiresForeground { get; init; } + public bool MustRunInForeground { get; init; } + public bool InterruptCurrentForeground { get; init; } } public record PowerShellExecutionOptions : ExecutionOptions @@ -24,7 +25,8 @@ public record PowerShellExecutionOptions : ExecutionOptions internal static PowerShellExecutionOptions ImmediateInteractive = new() { Priority = ExecutionPriority.Next, - RequiresForeground = true, + MustRunInForeground = true, + InterruptCurrentForeground = true, }; public bool WriteOutputToHost { get; init; } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs b/src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs index 55a60be28..ac617b094 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs @@ -21,17 +21,18 @@ internal class EvaluateHandler : IEvaluateHandler public async Task Handle(EvaluateRequestArguments request, CancellationToken cancellationToken) { - // This API is mostly used for F8 execution so it requires the foreground. + // This API is mostly used for F8 execution, so it needs to interrupt the command prompt + // (or other foreground task). await _executionService.ExecutePSCommandAsync( new PSCommand().AddScript(request.Expression), CancellationToken.None, new PowerShellExecutionOptions { - RequiresForeground = true, WriteInputToHost = true, WriteOutputToHost = true, AddToHistory = true, ThrowOnError = false, + InterruptCurrentForeground = true }).ConfigureAwait(false); // TODO: Should we return a more informative result? diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 8b2fd79ad..1140f5e65 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -277,7 +277,7 @@ public void SetExit() public Task InvokeTaskOnPipelineThreadAsync( SynchronousTask task) { - if (task.ExecutionOptions.RequiresForeground) + if (task.ExecutionOptions.InterruptCurrentForeground) { // When a task must displace the current foreground command, // we must: @@ -404,14 +404,9 @@ public void InvokePSDelegate(string representation, ExecutionOptions executionOp internal Task LoadHostProfilesAsync(CancellationToken cancellationToken) { - // TODO: Why exactly does loading profiles require the foreground? return ExecuteDelegateAsync( "LoadProfiles", - new PowerShellExecutionOptions - { - RequiresForeground = true, - ThrowOnError = false - }, + new PowerShellExecutionOptions { MustRunInForeground = true, ThrowOnError = false }, (pwsh, _) => pwsh.LoadProfiles(_hostInfo.ProfilePaths), cancellationToken); } @@ -855,15 +850,7 @@ private string InvokeReadLine(CancellationToken cancellationToken) private void InvokeInput(string input, CancellationToken cancellationToken) { PSCommand command = new PSCommand().AddScript(input, useLocalScope: false); - InvokePSCommand( - command, - new PowerShellExecutionOptions - { - AddToHistory = true, - ThrowOnError = false, - WriteOutputToHost = true - }, - cancellationToken); + InvokePSCommand(command, new PowerShellExecutionOptions { AddToHistory = true, ThrowOnError = false, WriteOutputToHost = true }, cancellationToken); } private void AddRunspaceEventHandlers(Runspace runspace) @@ -957,7 +944,6 @@ private Runspace CreateInitialRunspace(InitialSessionState initialSessionState) return runspace; } - // NOTE: This token is received from PSReadLine, and it _is_ the ReadKey cancellation token! private void OnPowerShellIdle(CancellationToken idleCancellationToken) { IReadOnlyList eventSubscribers = _mainRunspaceEngineIntrinsics.Events.Subscribers; @@ -990,7 +976,7 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken) while (!cancellationScope.CancellationToken.IsCancellationRequested && _taskQueue.TryTake(out ISynchronousTask task)) { - if (task.ExecutionOptions.RequiresForeground) + if (task.ExecutionOptions.MustRunInForeground) { // If we have a task that is queued, but cannot be run under readline // we place it back at the front of the queue, and cancel the readline task @@ -1111,27 +1097,27 @@ private void OnDebuggerStopped(object sender, DebuggerStopEventArgs debuggerStop void OnDebuggerStoppedImpl(object sender, DebuggerStopEventArgs debuggerStopEventArgs) { - // If the debug server is NOT active, we need to synchronize state and start it. - if (!DebugContext.IsDebugServerActive) - { - _languageServer?.SendNotification("powerShell/startDebugger"); - } + // If the debug server is NOT active, we need to synchronize state and start it. + if (!DebugContext.IsDebugServerActive) + { + _languageServer?.SendNotification("powerShell/startDebugger"); + } - DebugContext.SetDebuggerStopped(debuggerStopEventArgs); + DebugContext.SetDebuggerStopped(debuggerStopEventArgs); - try - { - CurrentPowerShell.WaitForRemoteOutputIfNeeded(); - PowerShellFrameType frameBase = CurrentFrame.FrameType & PowerShellFrameType.Remote; - PushPowerShellAndRunLoop( - CreateNestedPowerShell(CurrentRunspace), - frameBase | PowerShellFrameType.Debug | PowerShellFrameType.Nested | PowerShellFrameType.Repl); - CurrentPowerShell.ResumeRemoteOutputIfNeeded(); - } - finally - { - DebugContext.SetDebuggerResumed(); - } + try + { + CurrentPowerShell.WaitForRemoteOutputIfNeeded(); + PowerShellFrameType frameBase = CurrentFrame.FrameType & PowerShellFrameType.Remote; + PushPowerShellAndRunLoop( + CreateNestedPowerShell(CurrentRunspace), + frameBase | PowerShellFrameType.Debug | PowerShellFrameType.Nested | PowerShellFrameType.Repl); + CurrentPowerShell.ResumeRemoteOutputIfNeeded(); + } + finally + { + DebugContext.SetDebuggerResumed(); + } } } @@ -1155,7 +1141,7 @@ private Task PopOrReinitializeRunspaceAsync() // we simply run this on its thread, guaranteeing that no other action can occur return ExecuteDelegateAsync( nameof(PopOrReinitializeRunspaceAsync), - new ExecutionOptions { RequiresForeground = true }, + new ExecutionOptions { InterruptCurrentForeground = true }, (_) => { while (_psFrameStack.Count > 0 diff --git a/src/PowerShellEditorServices/Services/Template/TemplateService.cs b/src/PowerShellEditorServices/Services/Template/TemplateService.cs index f6dc76fa6..1427ba174 100644 --- a/src/PowerShellEditorServices/Services/Template/TemplateService.cs +++ b/src/PowerShellEditorServices/Services/Template/TemplateService.cs @@ -158,21 +158,15 @@ public async Task CreateFromTemplateAsync( _logger.LogTrace( $"Invoking Plaster...\n\n TemplatePath: {templatePath}\n DestinationPath: {destinationPath}"); - PSCommand command = new PSCommand() - .AddCommand("Invoke-Plaster") - .AddParameter("TemplatePath", templatePath) - .AddParameter("DestinationPath", destinationPath); + PSCommand command = new(); + command.AddCommand("Invoke-Plaster"); + command.AddParameter("TemplatePath", templatePath); + command.AddParameter("DestinationPath", destinationPath); - // This command is interactive so it requires the foreground. await _executionService.ExecutePSCommandAsync( command, CancellationToken.None, - new PowerShellExecutionOptions - { - RequiresForeground = true, - WriteOutputToHost = true, - ThrowOnError = false - }).ConfigureAwait(false); + new PowerShellExecutionOptions { WriteOutputToHost = true, InterruptCurrentForeground = true, ThrowOnError = false }).ConfigureAwait(false); // If any errors were written out, creation was not successful return true; From 89035b2617a5edbb335c3f4beb00e0b43dabd869 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 28 Apr 2022 11:54:05 -0400 Subject: [PATCH 2/2] Add back unrelated changes --- .../DebugAdapter/Handlers/ConfigurationDoneHandler.cs | 2 ++ .../PowerShell/Execution/BlockingConcurrentDeque.cs | 2 +- .../Services/PowerShell/Host/PsesInternalHost.cs | 1 + .../Services/Template/TemplateService.cs | 9 +++++---- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs index 74595831e..083c5dd0e 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs @@ -25,6 +25,8 @@ internal class ConfigurationDoneHandler : IConfigurationDoneHandler // TODO: We currently set `WriteInputToHost` as true, which writes our debugged commands' // `GetInvocationText` and that reveals some obscure implementation details we should // instead hide from the user with pretty strings (or perhaps not write out at all). + // + // This API is mostly used for F5 execution so it requires the foreground. private static readonly PowerShellExecutionOptions s_debuggerExecutionOptions = new() { MustRunInForeground = true, diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs index 035437dd3..2e5b4f733 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs @@ -65,7 +65,7 @@ public bool TryTake(out T item) public IDisposable BlockConsumers() => PriorityQueueBlockLifetime.StartBlocking(_blockConsumersEvent); - public void Dispose() => ((IDisposable)_blockConsumersEvent).Dispose(); + public void Dispose() => _blockConsumersEvent.Dispose(); private class PriorityQueueBlockLifetime : IDisposable { diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 1140f5e65..db332aab0 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -944,6 +944,7 @@ private Runspace CreateInitialRunspace(InitialSessionState initialSessionState) return runspace; } + // NOTE: This token is received from PSReadLine, and it _is_ the ReadKey cancellation token! private void OnPowerShellIdle(CancellationToken idleCancellationToken) { IReadOnlyList eventSubscribers = _mainRunspaceEngineIntrinsics.Events.Subscribers; diff --git a/src/PowerShellEditorServices/Services/Template/TemplateService.cs b/src/PowerShellEditorServices/Services/Template/TemplateService.cs index 1427ba174..39c01040a 100644 --- a/src/PowerShellEditorServices/Services/Template/TemplateService.cs +++ b/src/PowerShellEditorServices/Services/Template/TemplateService.cs @@ -158,11 +158,12 @@ public async Task CreateFromTemplateAsync( _logger.LogTrace( $"Invoking Plaster...\n\n TemplatePath: {templatePath}\n DestinationPath: {destinationPath}"); - PSCommand command = new(); - command.AddCommand("Invoke-Plaster"); - command.AddParameter("TemplatePath", templatePath); - command.AddParameter("DestinationPath", destinationPath); + PSCommand command = new PSCommand() + .AddCommand("Invoke-Plaster") + .AddParameter("TemplatePath", templatePath) + .AddParameter("DestinationPath", destinationPath); + // This command is interactive so it requires the foreground. await _executionService.ExecutePSCommandAsync( command, CancellationToken.None,