From 81c0b834f955ad66185fab83aa2dd2fe86229161 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 23 Jun 2022 14:08:51 -0700 Subject: [PATCH] Add command to PSReadLine history before cancellation Otherwise PSReadLine thinks it's supposed to re-insert it in the buffer after execution. --- .../Execution/SynchronousPowerShellTask.cs | 12 ++-- .../PowerShell/Host/PsesInternalHost.cs | 68 +++++++++++-------- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs index 6c3f3092c..0661ce962 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs @@ -95,7 +95,6 @@ private static bool IsPromptCommand(PSCommand command) private IReadOnlyList ExecuteNormally(CancellationToken cancellationToken) { _frame = _psesHost.CurrentFrame; - MaybeAddToHistory(_psCommand); if (PowerShellExecutionOptions.WriteOutputToHost) { _psCommand.AddOutputCommand(); @@ -188,7 +187,6 @@ private IReadOnlyList ExecuteInDebugger(CancellationToken cancellationT cancellationToken.Register(CancelDebugExecution); PSDataCollection outputCollection = new(); - MaybeAddToHistory(_psCommand); // Out-Default doesn't work as needed in the debugger // Instead we add Out-String to the command and collect results in a PSDataCollection @@ -355,7 +353,7 @@ private void CancelNormalExecution() } } - private void MaybeAddToHistory(PSCommand command) + internal void MaybeAddToHistory() { // Do not add PSES internal commands to history. Also exclude input that came from the // REPL (e.g. PSReadLine) as it handles history itself in that scenario. @@ -365,19 +363,19 @@ private void MaybeAddToHistory(PSCommand command) } // Only add pure script commands with no arguments to interactive history. - if (command.Commands is { Count: not 1 } - || command.Commands[0] is { Parameters.Count: not 0 } or { IsScript: false }) + if (_psCommand.Commands is { Count: not 1 } + || _psCommand.Commands[0] is { Parameters.Count: not 0 } or { IsScript: false }) { return; } try { - _psesHost.AddToHistory(command.Commands[0].CommandText); + _psesHost.AddToHistory(_psCommand.Commands[0].CommandText); } catch { - // Ignore exceptions as the user can register a scriptblock predicate that + // Ignore exceptions as the user can register a script-block predicate that // determines if the command should be added to history. } } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 69b9f0d72..828cbf890 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -314,31 +314,52 @@ public void SetExit() internal void ForceSetExit() => _shouldExit = true; - public Task InvokeTaskOnPipelineThreadAsync( - SynchronousTask task) + private bool CancelForegroundAndPrepend(ISynchronousTask task, bool isIdle = false) { // NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`. - // TODO: Deduplicate this. - if (task.ExecutionOptions.RequiresForeground) - { - // When a task must displace the current foreground command, - // we must: - // - block the consumer thread from mutating the queue - // - cancel any running task on the consumer thread - // - place our task on the front of the queue - // - skip the next prompt so the task runs instead - // - unblock the consumer thread - using (_taskQueue.BlockConsumers()) + // + // When a task must displace the current foreground command, + // we must: + // - block the consumer thread from mutating the queue + // - cancel any running task on the consumer thread + // - place our task on the front of the queue + // - skip the next prompt so the task runs instead + // - unblock the consumer thread + if (!task.ExecutionOptions.RequiresForeground) + { + return false; + } + + _skipNextPrompt = true; + + if (task is SynchronousPowerShellTask psTask) + { + psTask.MaybeAddToHistory(); + } + + using (_taskQueue.BlockConsumers()) + { + _taskQueue.Prepend(task); + if (isIdle) + { + CancelIdleParentTask(); + } + else { CancelCurrentTask(); - _taskQueue.Prepend(task); - _skipNextPrompt = true; } + } + return true; + } + + public Task InvokeTaskOnPipelineThreadAsync(SynchronousTask task) + { + if (CancelForegroundAndPrepend(task)) + { return task.Task; } - // TODO: Apply stashed `QueueTask` function. switch (task.ExecutionOptions.Priority) { case ExecutionPriority.Next: @@ -819,7 +840,7 @@ private void DoOneRepl(CancellationToken cancellationToken) { UI.WriteLine(); } - // Propogate cancellation if that's what happened, since ReadLine won't. + // Propagate cancellation if that's what happened, since ReadLine won't. // TODO: We may not need to do this at all. cancellationToken.ThrowIfCancellationRequested(); return; // Task wasn't canceled but there was no input. @@ -1047,15 +1068,8 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken) while (!cancellationScope.CancellationToken.IsCancellationRequested && _taskQueue.TryTake(out ISynchronousTask task)) { - // NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`. - // TODO: Deduplicate this. - if (task.ExecutionOptions.RequiresForeground) + if (CancelForegroundAndPrepend(task, isIdle: true)) { - // 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 - _taskQueue.Prepend(task); - _skipNextPrompt = true; - _cancellationContext.CancelIdleParentTask(); return; } @@ -1082,7 +1096,7 @@ private void OnCancelKeyPress(object sender, ConsoleCancelEventArgs args) _cancellationContext.CancelCurrentTask(); // If the current task was running under the debugger, we need to synchronize the - // cancelation with our debug context (and likely the debug server). Note that if we're + // cancellation with our debug context (and likely the debug server). Note that if we're // currently stopped in a breakpoint, that means the task is _not_ under the debugger. if (!CurrentRunspace.Runspace.Debugger.InBreakpoint) { @@ -1166,7 +1180,7 @@ private void OnDebuggerStopped(object sender, DebuggerStopEventArgs debuggerStop // selection and terminating the debugger. Without this, if the "Stop" button is pressed // then we hit this repeatedly. // - // This info is publically accessible via `PSDebugContext` but we'd need to access it + // This info is publicly accessible via `PSDebugContext` but we'd need to access it // via a script. At this point in the call I'd prefer this to be as light as possible so // we can escape ASAP but we may want to consider switching to that at some point. if (!Runspace.RunspaceIsRemote && s_scriptDebuggerTriggerObjectProperty is not null)