Skip to content

Commit

Permalink
WIP: Generalized busy indicator
Browse files Browse the repository at this point in the history
  • Loading branch information
@andyleejordan
andyleejordan committed Sep 30, 2022
1 parent bbdf98d commit 5816987
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 73 deletions.
53 changes: 1 addition & 52 deletions src/PowerShellEditorServices/Services/Extension/ExtensionService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,6 @@

namespace Microsoft.PowerShell.EditorServices.Services.Extension
{
/// Enumerates the possible execution results that can occur after
/// executing a command or script.
/// </summary>
internal enum ExecutionStatus
{
/// <summary>
/// Indicates that execution has not yet started.
/// </summary>
Pending,

/// <summary>
/// Indicates that the command is executing.
/// </summary>
Running,

/// <summary>
/// Indicates that execution has failed.
/// </summary>
Failed,

/// <summary>
/// Indicates that execution was aborted by the user.
/// </summary>
Aborted,

/// <summary>
/// Indicates that execution completed successfully.
/// </summary>
Completed
}

/// <summary>
/// Provides a high-level service which enables PowerShell scripts
/// and modules to extend the behavior of the host editor.
Expand Down Expand Up @@ -154,7 +123,6 @@ internal Task InitializeAsync()
/// <exception cref="KeyNotFoundException">The command being invoked was not registered.</exception>
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()
Expand All @@ -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,
Expand All @@ -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}'");
}

Expand Down
11 changes: 9 additions & 2 deletions src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@

namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution
{
internal class SynchronousPowerShellTask<TResult> : SynchronousTask<IReadOnlyList<TResult>>
internal interface ISynchronousPowerShellTask
{
PowerShellExecutionOptions PowerShellExecutionOptions { get; }

void MaybeAddToHistory();
}

internal class SynchronousPowerShellTask<TResult> : SynchronousTask<IReadOnlyList<TResult>>, ISynchronousPowerShellTask
{
private static readonly PowerShellExecutionOptions s_defaultPowerShellExecutionOptions = new();

Expand Down Expand Up @@ -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.
Expand Down
73 changes: 54 additions & 19 deletions src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,9 @@ private bool CancelForegroundAndPrepend(ISynchronousTask task, bool isIdle = fal

_skipNextPrompt = true;

if (task is SynchronousPowerShellTask<PSObject> psTask)
if (task is ISynchronousPowerShellTask t)
{
psTask.MaybeAddToHistory();
t.MaybeAddToHistory();
}

using (_taskQueue.BlockConsumers())
Expand All @@ -334,6 +334,26 @@ 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)
{
if (task is ISynchronousPowerShellTask t
&& (t.PowerShellExecutionOptions.AddToHistory
|| t.PowerShellExecutionOptions.FromRepl))
{
_languageServer?.SendNotification("powerShell/executionBusyStatus", true);
}
try
{
task.ExecuteSynchronously(cancellationToken);
}
finally
{
_languageServer?.SendNotification("powerShell/executionBusyStatus", false);
}
}

public Task<T> InvokeTaskOnPipelineThreadAsync<T>(SynchronousTask<T> task)
{
if (CancelForegroundAndPrepend(task))
Expand Down Expand Up @@ -769,8 +789,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);
Expand Down Expand Up @@ -935,19 +960,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);
_languageServer?.SendNotification("powerShell/executionBusyStatus", true);
try
{
InvokePSCommand(
new PSCommand().AddScript(input, useLocalScope: false),
new PowerShellExecutionOptions
{
AddToHistory = true,
ThrowOnError = false,
WriteOutputToHost = true,
FromRepl = true,
},
cancellationToken);
}
finally
{
_languageServer?.SendNotification("powerShell/executionBusyStatus", false);
}
}

private void AddRunspaceEventHandlers(Runspace runspace)
Expand Down Expand Up @@ -1076,16 +1109,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);
}
}

Expand Down

0 comments on commit 5816987

Please sign in to comment.