From 8121b52c21e3e935d541c389558f841e2b5a24d9 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 26 Apr 2022 12:21:14 -0700 Subject: [PATCH] Consolidate `InterruptCurrentForeground` and `MustRunInForeground` And set them consistently. If the task needs to interrupt or run in the foreground, it really needs to do both. --- .../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, 65 insertions(+), 43 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs index 74595831e..d22dde4ee 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs @@ -25,9 +25,11 @@ 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, + RequiresForeground = true, WriteInputToHost = true, WriteOutputToHost = true, ThrowOnError = false, diff --git a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs index eb8bd2859..f4284f934 100644 --- a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs +++ b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs @@ -129,17 +129,16 @@ 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 needs to interrupt the - // current prompt (or other foreground task). + // This API is used for editor command execution so it requires the foreground. 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 64e3627fa..cda3af925 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs @@ -32,7 +32,11 @@ public PsrlReadLine( _psrlProxy.OverrideIdleHandler(onIdleAction); } - public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate(representation: "ReadLine", new ExecutionOptions { MustRunInForeground = true }, InvokePSReadLine, cancellationToken); + public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate( + representation: "ReadLine", + new ExecutionOptions { RequiresForeground = 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 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/Execution/ExecutionOptions.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs index 27bb8935a..83de5301c 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs @@ -16,8 +16,7 @@ public enum ExecutionPriority public record ExecutionOptions { public ExecutionPriority Priority { get; init; } = ExecutionPriority.Normal; - public bool MustRunInForeground { get; init; } - public bool InterruptCurrentForeground { get; init; } + public bool RequiresForeground { get; init; } } public record PowerShellExecutionOptions : ExecutionOptions @@ -25,8 +24,7 @@ public record PowerShellExecutionOptions : ExecutionOptions internal static PowerShellExecutionOptions ImmediateInteractive = new() { Priority = ExecutionPriority.Next, - MustRunInForeground = true, - InterruptCurrentForeground = true, + RequiresForeground = 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 ac617b094..55a60be28 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs @@ -21,18 +21,17 @@ internal class EvaluateHandler : IEvaluateHandler public async Task Handle(EvaluateRequestArguments request, CancellationToken cancellationToken) { - // This API is mostly used for F8 execution, so it needs to interrupt the command prompt - // (or other foreground task). + // This API is mostly used for F8 execution so it requires the foreground. 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 8697f45ab..3b4281248 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.InterruptCurrentForeground) + if (task.ExecutionOptions.RequiresForeground) { // When a task must displace the current foreground command, // we must: @@ -404,9 +404,14 @@ 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 { MustRunInForeground = true, ThrowOnError = false }, + new PowerShellExecutionOptions + { + RequiresForeground = true, + ThrowOnError = false + }, (pwsh, _) => pwsh.LoadProfiles(_hostInfo.ProfilePaths), cancellationToken); } @@ -855,7 +860,15 @@ 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) @@ -949,6 +962,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; @@ -981,7 +995,7 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken) while (!cancellationScope.CancellationToken.IsCancellationRequested && _taskQueue.TryTake(out ISynchronousTask task)) { - if (task.ExecutionOptions.MustRunInForeground) + if (task.ExecutionOptions.RequiresForeground) { // 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 @@ -1102,27 +1116,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(); + } } } @@ -1146,7 +1160,7 @@ private Task PopOrReinitializeRunspaceAsync() // we simply run this on its thread, guaranteeing that no other action can occur return ExecuteDelegateAsync( nameof(PopOrReinitializeRunspaceAsync), - new ExecutionOptions { InterruptCurrentForeground = true }, + new ExecutionOptions { RequiresForeground = true }, (_) => { while (_psFrameStack.Count > 0 diff --git a/src/PowerShellEditorServices/Services/Template/TemplateService.cs b/src/PowerShellEditorServices/Services/Template/TemplateService.cs index 1427ba174..f6dc76fa6 100644 --- a/src/PowerShellEditorServices/Services/Template/TemplateService.cs +++ b/src/PowerShellEditorServices/Services/Template/TemplateService.cs @@ -158,15 +158,21 @@ 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, - new PowerShellExecutionOptions { WriteOutputToHost = true, InterruptCurrentForeground = true, ThrowOnError = false }).ConfigureAwait(false); + new PowerShellExecutionOptions + { + RequiresForeground = true, + WriteOutputToHost = true, + ThrowOnError = false + }).ConfigureAwait(false); // If any errors were written out, creation was not successful return true;