Skip to content

Fix editor commands to interrupt current prompt #1725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Fix editor commands to interrupt current prompt #1725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/PowerShellEditorServices/Extensions/Api/ExtensionCommandService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Microsoft.PowerShell.EditorServices.Services.Extension;
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.PowerShell.EditorServices.Extensions.Services
Expand Down Expand Up @@ -78,7 +79,7 @@ public ExtensionCommandService(ExtensionService extensionService)

public IReadOnlyList<EditorCommand> GetCommands() => _extensionService.GetCommands();

public Task InvokeCommandAsync(string commandName, EditorContext editorContext) => _extensionService.InvokeCommandAsync(commandName, editorContext);
public Task InvokeCommandAsync(string commandName, EditorContext editorContext) => _extensionService.InvokeCommandAsync(commandName, editorContext, CancellationToken.None);

public bool RegisterCommand(EditorCommand editorCommand) => _extensionService.RegisterCommand(editorCommand);

Expand Down
19 changes: 10 additions & 9 deletions src/PowerShellEditorServices/Services/Extension/ExtensionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ internal Task InitializeAsync()
/// <param name="editorContext">The context in which the command is being invoked.</param>
/// <returns>A Task that can be awaited for completion.</returns>
/// <exception cref="KeyNotFoundException">The command being invoked was not registered.</exception>
public async Task InvokeCommandAsync(string commandName, EditorContext editorContext)
public Task InvokeCommandAsync(string commandName, EditorContext editorContext, CancellationToken cancellationToken)
{
if (editorCommands.TryGetValue(commandName, out EditorCommand editorCommand))
{
Expand All @@ -128,20 +128,21 @@ public async Task InvokeCommandAsync(string commandName, EditorContext editorCon
.AddParameter("ScriptBlock", editorCommand.ScriptBlock)
.AddParameter("ArgumentList", new object[] { editorContext });

await ExecutionService.ExecutePSCommandAsync(
// This API is used for editor command execution, so it needs to interrupt the
// current prompt (or other foreground task).
return ExecutionService.ExecutePSCommandAsync(
executeCommand,
CancellationToken.None,
cancellationToken,
new PowerShellExecutionOptions
{
WriteOutputToHost = !editorCommand.SuppressOutput,
AddToHistory = !editorCommand.SuppressOutput,
ThrowOnError = false,
AddToHistory = !editorCommand.SuppressOutput
}).ConfigureAwait(false);
}
else
{
throw new KeyNotFoundException($"Editor command not found: '{commandName}'");
InterruptCurrentForeground = true
});
}

throw new KeyNotFoundException($"Editor command not found: '{commandName}'");
}

/// <summary>
Expand Down
20 changes: 4 additions & 16 deletions src/PowerShellEditorServices/Services/Extension/Handlers/InvokeExtensionCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,28 @@
using System.Threading;
using System.Threading.Tasks;
using MediatR;
using Microsoft.Extensions.Logging;
using Microsoft.PowerShell.EditorServices.Extensions;

namespace Microsoft.PowerShell.EditorServices.Services.Extension
{
internal class InvokeExtensionCommandHandler : IInvokeExtensionCommandHandler
{
private readonly ILogger<InvokeExtensionCommandHandler> _logger;
private readonly ExtensionService _extensionService;
private readonly EditorOperationsService _editorOperationsService;

public InvokeExtensionCommandHandler(
ILoggerFactory factory,
ExtensionService extensionService,
EditorOperationsService editorOperationsService
)
EditorOperationsService editorOperationsService)
{
_logger = factory.CreateLogger<InvokeExtensionCommandHandler>();
_extensionService = extensionService;
_editorOperationsService = editorOperationsService;
}

public async Task<Unit> Handle(InvokeExtensionCommandParams request, CancellationToken cancellationToken)
{
// We can now await here because we handle asynchronous message handling.
EditorContext editorContext =
_editorOperationsService.ConvertClientEditorContext(
request.Context);

await _extensionService.InvokeCommandAsync(
request.Name,
editorContext).ConfigureAwait(false);

return await Unit.Task.ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea why this was the old return type. Almost everything else using the same API returns Unit.Value (which seems the right thing to return).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, clarification on that "almost" it's that async Task<Unit> with an await needs to return Unit.Value, and if it's not async then it's a Unit.Task that needs to be returned.

EditorContext editorContext = _editorOperationsService.ConvertClientEditorContext(request.Context);
await _extensionService.InvokeCommandAsync(request.Name, editorContext, cancellationToken).ConfigureAwait(false);
return Unit.Value;
}
}
}
16 changes: 11 additions & 5 deletions src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,21 @@ public EvaluateHandler(

public async Task<EvaluateResponseBody> Handle(EvaluateRequestArguments request, CancellationToken cancellationToken)
{
// TODO: Understand why we currently handle this asynchronously and why we return a dummy result value
// instead of awaiting the execution and returing a real result of some kind

// This API is mostly used for F8 execution, so needs to interrupt the command prompt
// This API is mostly used for F8 execution, so it needs to interrupt the command prompt
// (or other foreground task).
await _executionService.ExecutePSCommandAsync(
new PSCommand().AddScript(request.Expression),
CancellationToken.None,
new PowerShellExecutionOptions { WriteInputToHost = true, WriteOutputToHost = true, AddToHistory = true, ThrowOnError = false, InterruptCurrentForeground = true }).ConfigureAwait(false);
new PowerShellExecutionOptions
{
WriteInputToHost = true,
WriteOutputToHost = true,
AddToHistory = true,
ThrowOnError = false,
InterruptCurrentForeground = true
}).ConfigureAwait(false);

// TODO: Should we return a more informative result?
return new EvaluateResponseBody
{
Result = "",
Expand Down