diff --git a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs index 4405a4ce7..028fe0f19 100644 --- a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs +++ b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs @@ -14,37 +14,6 @@ namespace Microsoft.PowerShell.EditorServices.Services.Extension { - /// Enumerates the possible execution results that can occur after - /// executing a command or script. - /// - internal enum ExecutionStatus - { - /// - /// Indicates that execution has not yet started. - /// - Pending, - - /// - /// Indicates that the command is executing. - /// - Running, - - /// - /// Indicates that execution has failed. - /// - Failed, - - /// - /// Indicates that execution was aborted by the user. - /// - Aborted, - - /// - /// Indicates that execution completed successfully. - /// - Completed - } - /// /// Provides a high-level service which enables PowerShell scripts /// and modules to extend the behavior of the host editor. @@ -154,7 +123,6 @@ internal Task InitializeAsync() /// The command being invoked was not registered. public Task InvokeCommandAsync(string commandName, EditorContext editorContext, CancellationToken cancellationToken) { - _languageServer?.SendNotification("powerShell/executionStatusChanged", ExecutionStatus.Pending); if (editorCommands.TryGetValue(commandName, out EditorCommand editorCommand)) { PSCommand executeCommand = new PSCommand() @@ -163,7 +131,6 @@ public Task InvokeCommandAsync(string commandName, EditorContext editorContext, .AddParameter("ArgumentList", new object[] { editorContext }); // This API is used for editor command execution so it requires the foreground. - _languageServer?.SendNotification("powerShell/executionStatusChanged", ExecutionStatus.Running); return ExecutionService.ExecutePSCommandAsync( executeCommand, cancellationToken, @@ -173,27 +140,9 @@ public Task InvokeCommandAsync(string commandName, EditorContext editorContext, WriteOutputToHost = !editorCommand.SuppressOutput, AddToHistory = !editorCommand.SuppressOutput, ThrowOnError = false, - }).ContinueWith((Task executeTask) => - { - ExecutionStatus status = ExecutionStatus.Failed; - if (executeTask.IsCompleted) - { - status = ExecutionStatus.Completed; - } - else if (executeTask.IsCanceled) - { - status = ExecutionStatus.Aborted; - } - else if (executeTask.IsFaulted) - { - status = ExecutionStatus.Failed; - } - - _languageServer?.SendNotification("powerShell/executionStatusChanged", status); - }, TaskScheduler.Default); + }); } - _languageServer?.SendNotification("powerShell/executionStatusChanged", ExecutionStatus.Failed); throw new KeyNotFoundException($"Editor command not found: '{commandName}'"); } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs index 2fdefbfdc..5256f5ad9 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs @@ -16,7 +16,14 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution { - internal class SynchronousPowerShellTask : SynchronousTask> + internal interface ISynchronousPowerShellTask + { + PowerShellExecutionOptions PowerShellExecutionOptions { get; } + + void MaybeAddToHistory(); + } + + internal class SynchronousPowerShellTask : SynchronousTask>, ISynchronousPowerShellTask { private static readonly PowerShellExecutionOptions s_defaultPowerShellExecutionOptions = new(); @@ -353,7 +360,7 @@ private void CancelNormalExecution() } } - internal void MaybeAddToHistory() + public 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. diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 499757faa..12f746baf 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -295,6 +295,8 @@ public void SetExit() internal void ForceSetExit() => _shouldExit = true; + private void SetBusy(bool busy) => _languageServer?.SendNotification("powerShell/executionBusyStatus", busy); + private bool CancelForegroundAndPrepend(ISynchronousTask task, bool isIdle = false) { // NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`. @@ -313,9 +315,9 @@ private bool CancelForegroundAndPrepend(ISynchronousTask task, bool isIdle = fal _skipNextPrompt = true; - if (task is SynchronousPowerShellTask psTask) + if (task is ISynchronousPowerShellTask t) { - psTask.MaybeAddToHistory(); + t.MaybeAddToHistory(); } using (_taskQueue.BlockConsumers()) @@ -334,6 +336,32 @@ private bool CancelForegroundAndPrepend(ISynchronousTask task, bool isIdle = fal return true; } + // This handles executing the task while also notifying the client that the pipeline is + // currently busy with a PowerShell task. The extension indicates this with a spinner. + private void ExecuteTaskSynchronously(ISynchronousTask task, CancellationToken cancellationToken) + { + // TODO: Simplify this logic. + bool busy = false; + if (task is ISynchronousPowerShellTask t + && (t.PowerShellExecutionOptions.AddToHistory + || t.PowerShellExecutionOptions.FromRepl)) + { + busy = true; + SetBusy(true); + } + try + { + task.ExecuteSynchronously(cancellationToken); + } + finally + { + if (busy) + { + SetBusy(false); + } + } + } + public Task InvokeTaskOnPipelineThreadAsync(SynchronousTask task) { if (CancelForegroundAndPrepend(task)) @@ -769,8 +797,13 @@ private void RunExecutionLoop(bool isForDebug = false) { try { - task.ExecuteSynchronously(cancellationScope.CancellationToken); + ExecuteTaskSynchronously(task, cancellationScope.CancellationToken); } + // Our flaky extension command test seems to be such because sometimes another + // task gets queued, and since it runs in the foreground it cancels that task. + // Interactively, this happens in the first loop (with DoOneRepl) which catches + // the cancellation exception, but when under test that is a no-op, so it + // happens in this second loop. Hence we need to catch it here too. catch (OperationCanceledException e) { _logger.LogDebug(e, "Task {Task} was canceled!", task); @@ -935,19 +968,27 @@ private string InvokeReadLine(CancellationToken cancellationToken) } } + // TODO: Should we actually be directly invoking input versus queueing it as a task like everything else? 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, - FromRepl = true, - }, - cancellationToken); + SetBusy(true); + try + { + InvokePSCommand( + new PSCommand().AddScript(input, useLocalScope: false), + new PowerShellExecutionOptions + { + AddToHistory = true, + ThrowOnError = false, + WriteOutputToHost = true, + FromRepl = true, + }, + cancellationToken); + } + finally + { + SetBusy(false); + } } private void AddRunspaceEventHandlers(Runspace runspace) @@ -1076,16 +1117,18 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken) while (!cancellationScope.CancellationToken.IsCancellationRequested && _taskQueue.TryTake(out ISynchronousTask task)) { + // Tasks which require the foreground cannot run under this idle handler, so the + // current foreground tasks gets canceled, the new task gets prepended, and this + // handler returns. if (CancelForegroundAndPrepend(task, isIdle: true)) { return; } - // If we're executing a task, we don't need to run an extra pipeline later for events - // TODO: This may not be a PowerShell task, so ideally we can differentiate that here. - // For now it's mostly true and an easy assumption to make. - runPipelineForEventProcessing = false; - task.ExecuteSynchronously(cancellationScope.CancellationToken); + // If we're executing a PowerShell task, we don't need to run an extra pipeline + // later for events. + runPipelineForEventProcessing = task is not ISynchronousPowerShellTask; + ExecuteTaskSynchronously(task, cancellationScope.CancellationToken); } }