Skip to content

Commit 854665f

Browse files
Add command to PSReadLine history before cancellation (#1841)
Otherwise PSReadLine thinks it's supposed to re-insert it in the buffer after execution.
1 parent 066faec commit 854665f

File tree

2 files changed

+46
-34
lines changed

2 files changed

+46
-34
lines changed

src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs

+5-7
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ private static bool IsPromptCommand(PSCommand command)
9595
private IReadOnlyList<TResult> ExecuteNormally(CancellationToken cancellationToken)
9696
{
9797
_frame = _psesHost.CurrentFrame;
98-
MaybeAddToHistory(_psCommand);
9998
if (PowerShellExecutionOptions.WriteOutputToHost)
10099
{
101100
_psCommand.AddOutputCommand();
@@ -188,7 +187,6 @@ private IReadOnlyList<TResult> ExecuteInDebugger(CancellationToken cancellationT
188187
cancellationToken.Register(CancelDebugExecution);
189188

190189
PSDataCollection<PSObject> outputCollection = new();
191-
MaybeAddToHistory(_psCommand);
192190

193191
// Out-Default doesn't work as needed in the debugger
194192
// Instead we add Out-String to the command and collect results in a PSDataCollection
@@ -355,7 +353,7 @@ private void CancelNormalExecution()
355353
}
356354
}
357355

358-
private void MaybeAddToHistory(PSCommand command)
356+
internal void MaybeAddToHistory()
359357
{
360358
// Do not add PSES internal commands to history. Also exclude input that came from the
361359
// REPL (e.g. PSReadLine) as it handles history itself in that scenario.
@@ -365,19 +363,19 @@ private void MaybeAddToHistory(PSCommand command)
365363
}
366364

367365
// Only add pure script commands with no arguments to interactive history.
368-
if (command.Commands is { Count: not 1 }
369-
|| command.Commands[0] is { Parameters.Count: not 0 } or { IsScript: false })
366+
if (_psCommand.Commands is { Count: not 1 }
367+
|| _psCommand.Commands[0] is { Parameters.Count: not 0 } or { IsScript: false })
370368
{
371369
return;
372370
}
373371

374372
try
375373
{
376-
_psesHost.AddToHistory(command.Commands[0].CommandText);
374+
_psesHost.AddToHistory(_psCommand.Commands[0].CommandText);
377375
}
378376
catch
379377
{
380-
// Ignore exceptions as the user can register a scriptblock predicate that
378+
// Ignore exceptions as the user can register a script-block predicate that
381379
// determines if the command should be added to history.
382380
}
383381
}

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

+41-27
Original file line numberDiff line numberDiff line change
@@ -314,31 +314,52 @@ public void SetExit()
314314

315315
internal void ForceSetExit() => _shouldExit = true;
316316

317-
public Task<T> InvokeTaskOnPipelineThreadAsync<T>(
318-
SynchronousTask<T> task)
317+
private bool CancelForegroundAndPrepend(ISynchronousTask task, bool isIdle = false)
319318
{
320319
// NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`.
321-
// TODO: Deduplicate this.
322-
if (task.ExecutionOptions.RequiresForeground)
323-
{
324-
// When a task must displace the current foreground command,
325-
// we must:
326-
// - block the consumer thread from mutating the queue
327-
// - cancel any running task on the consumer thread
328-
// - place our task on the front of the queue
329-
// - skip the next prompt so the task runs instead
330-
// - unblock the consumer thread
331-
using (_taskQueue.BlockConsumers())
320+
//
321+
// When a task must displace the current foreground command,
322+
// we must:
323+
// - block the consumer thread from mutating the queue
324+
// - cancel any running task on the consumer thread
325+
// - place our task on the front of the queue
326+
// - skip the next prompt so the task runs instead
327+
// - unblock the consumer thread
328+
if (!task.ExecutionOptions.RequiresForeground)
329+
{
330+
return false;
331+
}
332+
333+
_skipNextPrompt = true;
334+
335+
if (task is SynchronousPowerShellTask<PSObject> psTask)
336+
{
337+
psTask.MaybeAddToHistory();
338+
}
339+
340+
using (_taskQueue.BlockConsumers())
341+
{
342+
_taskQueue.Prepend(task);
343+
if (isIdle)
344+
{
345+
CancelIdleParentTask();
346+
}
347+
else
332348
{
333349
CancelCurrentTask();
334-
_taskQueue.Prepend(task);
335-
_skipNextPrompt = true;
336350
}
351+
}
337352

353+
return true;
354+
}
355+
356+
public Task<T> InvokeTaskOnPipelineThreadAsync<T>(SynchronousTask<T> task)
357+
{
358+
if (CancelForegroundAndPrepend(task))
359+
{
338360
return task.Task;
339361
}
340362

341-
// TODO: Apply stashed `QueueTask` function.
342363
switch (task.ExecutionOptions.Priority)
343364
{
344365
case ExecutionPriority.Next:
@@ -819,7 +840,7 @@ private void DoOneRepl(CancellationToken cancellationToken)
819840
{
820841
UI.WriteLine();
821842
}
822-
// Propogate cancellation if that's what happened, since ReadLine won't.
843+
// Propagate cancellation if that's what happened, since ReadLine won't.
823844
// TODO: We may not need to do this at all.
824845
cancellationToken.ThrowIfCancellationRequested();
825846
return; // Task wasn't canceled but there was no input.
@@ -1047,15 +1068,8 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken)
10471068
while (!cancellationScope.CancellationToken.IsCancellationRequested
10481069
&& _taskQueue.TryTake(out ISynchronousTask task))
10491070
{
1050-
// NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`.
1051-
// TODO: Deduplicate this.
1052-
if (task.ExecutionOptions.RequiresForeground)
1071+
if (CancelForegroundAndPrepend(task, isIdle: true))
10531072
{
1054-
// If we have a task that is queued, but cannot be run under readline
1055-
// we place it back at the front of the queue, and cancel the readline task
1056-
_taskQueue.Prepend(task);
1057-
_skipNextPrompt = true;
1058-
_cancellationContext.CancelIdleParentTask();
10591073
return;
10601074
}
10611075

@@ -1082,7 +1096,7 @@ private void OnCancelKeyPress(object sender, ConsoleCancelEventArgs args)
10821096
_cancellationContext.CancelCurrentTask();
10831097

10841098
// If the current task was running under the debugger, we need to synchronize the
1085-
// cancelation with our debug context (and likely the debug server). Note that if we're
1099+
// cancellation with our debug context (and likely the debug server). Note that if we're
10861100
// currently stopped in a breakpoint, that means the task is _not_ under the debugger.
10871101
if (!CurrentRunspace.Runspace.Debugger.InBreakpoint)
10881102
{
@@ -1166,7 +1180,7 @@ private void OnDebuggerStopped(object sender, DebuggerStopEventArgs debuggerStop
11661180
// selection and terminating the debugger. Without this, if the "Stop" button is pressed
11671181
// then we hit this repeatedly.
11681182
//
1169-
// This info is publically accessible via `PSDebugContext` but we'd need to access it
1183+
// This info is publicly accessible via `PSDebugContext` but we'd need to access it
11701184
// via a script. At this point in the call I'd prefer this to be as light as possible so
11711185
// we can escape ASAP but we may want to consider switching to that at some point.
11721186
if (!Runspace.RunspaceIsRemote && s_scriptDebuggerTriggerObjectProperty is not null)

0 commit comments

Comments
 (0)