Skip to content

Address review comments #1577

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.

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 21 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion src/PowerShellEditorServices/Server/PsesLanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ public async Task StartAsync()
// _Initialize_ request:
// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize
.OnInitialize(
// TODO: Either fix or ignore "method lacks 'await'" warning.
(languageServer, request, cancellationToken) =>
{
Log.Logger.Debug("Initializing OmniSharp Language Server");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ public static IServiceCollection AddPsesLanguageServices(
provider.GetService<EditorOperationsService>(),
provider.GetService<PowerShellExecutionService>());

// This is where we create the $psEditor variable
// so that when the console is ready, it will be available
// TODO: Improve the sequencing here so that:
// - The variable is guaranteed to be initialized when the console first appears
// - Any errors that occur are handled rather than lost by the unawaited task
extensionService.InitializeAsync();

return extensionService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;

namespace Microsoft.PowerShell.EditorServices.Services
{
Expand Down Expand Up @@ -182,7 +181,7 @@ public async Task<BreakpointDetails[]> SetLineBreakpointsAsync(

// Fix for issue #123 - file paths that contain wildcard chars [ and ] need to
// quoted and have those wildcard chars escaped.
string escapedScriptPath = PathUtils.WildcardEscape(scriptPath);
string escapedScriptPath = PathUtils.WildcardEscapePath(scriptPath);

if (dscBreakpoints == null || !dscBreakpoints.IsDscResourcePath(escapedScriptPath))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
using System.Collections.Generic;
using System.Text;

namespace PowerShellEditorServices.Services.PowerShell.Console
namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Console
{
internal class ColorConfiguration
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerSh

try
{
var psVersionTableCommand = new PSCommand().AddScript("$PSVersionTable", useLocalScope: true);

Hashtable psVersionTable = pwsh
.AddScript("$PSVersionTable", useLocalScope: true)
.InvokeAndClear<Hashtable>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,29 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
using System.Threading.Tasks;

/// <summary>
/// Handles the state of the PowerShell debugger.
/// </summary>
/// <remarks>
/// <para>
/// Debugging through a PowerShell Host is implemented by registering a handler
/// for the <see cref="System.Management.Automation.Debugger.DebuggerStop"/> event.
/// Registering that handler causes debug actions in PowerShell like Set-PSBreakpoint
/// and Wait-Debugger to drop into the debugger and trigger the handler.
/// The handler is passed a mutable <see cref="System.Management.Automation.DebuggerStopEventArgs"/> object
/// and the debugger stop lasts for the duration of the handler call.
/// The handler sets the <see cref="System.Management.Automation.DebuggerStopEventArgs.ResumeAction"/> property
/// when after it returns, the PowerShell debugger uses that as the direction on how to proceed.
/// </para>
/// <para>
/// When we handle the <see cref="System.Management.Automation.Debugger.DebuggerStop"/> event,
/// we drop into a nested debug prompt and execute things in the debugger with <see cref="System.Management.Automation.Debugger.ProcessCommand(PSCommand, PSDataCollection{PSObject})"/>,
/// which enables debugger commands like <c>l</c>, <c>c</c>, <c>s</c>, etc.
/// <see cref="Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging.PowerShellDebugContext"/> saves the event args object in its state,
/// and when one of the debugger commands is used, the result returned is used to set <see cref="System.Management.Automation.DebuggerStopEventArgs.ResumeAction"/>
/// on the saved event args object so that when the event handler returns, the PowerShell debugger takes the correct action.
/// </para>
/// </remarks>
internal class PowerShellDebugContext : IPowerShellDebugContext
{
private readonly ILogger _logger;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ private IReadOnlyList<TResult> ExecuteNormally(CancellationToken cancellationTok
result = _pwsh.InvokeCommand<TResult>(_psCommand);
cancellationToken.ThrowIfCancellationRequested();
}
// Test if we've been cancelled. If we're remoting, PSRemotingDataStructureException effectively means the pipeline was stopped.
catch (Exception e) when (cancellationToken.IsCancellationRequested || e is PipelineStoppedException || e is PSRemotingDataStructureException)
{
throw new OperationCanceledException();
}
// We only catch RuntimeExceptions here in case writing errors to output was requested
// Other errors are bubbled up to the caller
catch (RuntimeException e)
{
Logger.LogWarning($"Runtime exception occurred while executing command:{Environment.NewLine}{Environment.NewLine}{e}");
Expand Down Expand Up @@ -107,6 +110,9 @@ private IReadOnlyList<TResult> ExecuteInDebugger(CancellationToken cancellationT

var outputCollection = new PSDataCollection<PSObject>();

// Out-Default doesn't work as needed in the debugger
// Instead we add Out-String to the command and collect results in a PSDataCollection
// and use the event handler to print output to the UI as its added to that collection
if (_executionOptions.WriteOutputToHost)
{
_psCommand.AddDebugOutputCommand();
Expand All @@ -124,14 +130,21 @@ private IReadOnlyList<TResult> ExecuteInDebugger(CancellationToken cancellationT
DebuggerCommandResults debuggerResult = null;
try
{
// In the PowerShell debugger, extra debugger commands are made available, like "l", "s", "c", etc.
// Executing those commands produces a result that needs to be set on the debugger stop event args.
// So we use the Debugger.ProcessCommand() API to properly execute commands in the debugger
// and then call DebugContext.ProcessDebuggerResult() later to handle the command appropriately
debuggerResult = _pwsh.Runspace.Debugger.ProcessCommand(_psCommand, outputCollection);
cancellationToken.ThrowIfCancellationRequested();
}
// Test if we've been cancelled. If we're remoting, PSRemotingDataStructureException effectively means the pipeline was stopped.
catch (Exception e) when (cancellationToken.IsCancellationRequested || e is PipelineStoppedException || e is PSRemotingDataStructureException)
{
StopDebuggerIfRemoteDebugSessionFailed();
throw new OperationCanceledException();
}
// We only catch RuntimeExceptions here in case writing errors to output was requested
// Other errors are bubbled up to the caller
catch (RuntimeException e)
{
Logger.LogWarning($"Runtime exception occurred while executing command:{Environment.NewLine}{Environment.NewLine}{e}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

namespace Microsoft.PowerShell.EditorServices.Handlers
{
/// <summary>
/// Handler for a custom request type for evaluating PowerShell.
/// This is generally for F8 support, to allow execution of a highlighted code snippet in the console as if it were copy-pasted.
/// </summary>
internal class EvaluateHandler : IEvaluateHandler
{
private readonly ILogger _logger;
Expand All @@ -25,6 +29,9 @@ public EvaluateHandler(

public 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

_executionService.ExecutePSCommandAsync(
new PSCommand().AddScript(request.Expression),
new PowerShellExecutionOptions { WriteInputToHost = true, WriteOutputToHost = true, AddToHistory = true, InterruptCommandPrompt = true },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ internal class SessionDetails
private const string Property_InstanceId = "instanceId";

/// <summary>
/// Gets the PSCommand that gathers details from the
/// current session.
/// Runs a PowerShell command to gather details about the current session.
/// </summary>
/// <returns>A PSCommand used to gather session details.</returns>
/// <returns>A data object containing details about the PowerShell session.</returns>
public static SessionDetails GetFromPowerShell(PowerShell pwsh)
{
Hashtable detailsObject = pwsh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@

namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility
{
/// <summary>
/// Encapsulates the scoping logic for cancellation tokens.
/// As PowerShell commands nest, this class maintains a stack of cancellation scopes
/// that allow each scope of logic to be cancelled at its own level.
/// Implicitly handles the merging and cleanup of cancellation token sources.
/// </summary>
/// <example>
/// The <see cref="Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility.CancellationContext"/> class
/// and the <see cref="Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility.CancellationScope"/> struct
/// are intended to be used with a <c>using</c> block so you can do this:
/// <code>
/// using (CancellationScope cancellationScope = _cancellationContext.EnterScope(_globalCancellationSource.CancellationToken, localCancellationToken))
/// {
/// ExecuteCommandAsync(command, cancellationScope.CancellationToken);
/// }
/// </code>
/// </example>
internal class CancellationContext
{
private readonly ConcurrentStack<CancellationTokenSource> _cancellationSourceStack;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,7 @@ public static async Task<CommandInfo> GetCommandInfoAsync(
.AddArgument(commandName)
.AddParameter("ErrorAction", "Ignore");

CommandInfo commandInfo = null;
foreach (CommandInfo result in await executionService.ExecutePSCommandAsync<CommandInfo>(command, new PowerShellExecutionOptions(), CancellationToken.None).ConfigureAwait(false))
{
commandInfo = result;
break;
}
CommandInfo commandInfo = (await executionService.ExecutePSCommandAsync<CommandInfo>(command, new PowerShellExecutionOptions(), CancellationToken.None).ConfigureAwait(false)).FirstOrDefault();

// Only cache CmdletInfos since they're exposed in binaries they are likely to not change throughout the session.
if (commandInfo?.CommandType == CommandTypes.Cmdlet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,15 @@ static ErrorRecordExtensions()

var errorObjectParameter = Expression.Parameter(typeof(PSObject));

// Generates a call like:
// $errorPSObject.WriteStream = [System.Management.Automation.WriteStreamType]::Error
// So that error record PSObjects will be rendered in the console properly
// See https://github.com/PowerShell/PowerShell/blob/946341b2ebe6a61f081f4c9143668dc7be1f9119/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs#L2088-L2091
s_setWriteStreamProperty = Expression.Lambda<Action<PSObject>>(
Expression.Call(
errorObjectParameter,
writeStreamProperty.GetSetMethod(),
Expression.Constant(errorStreamType)),
Expression.Constant(errorStreamType)),
errorObjectParameter)
.Compile();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,26 @@ public static PSCommand AddDebugOutputCommand(this PSCommand psCommand)

public static PSCommand MergePipelineResults(this PSCommand psCommand)
{
// We need to do merge errors and output before rendering with an Out- cmdlet
Command lastCommand = psCommand.Commands[psCommand.Commands.Count - 1];
lastCommand.MergeMyResults(PipelineResultTypes.Error, PipelineResultTypes.Output);
lastCommand.MergeMyResults(PipelineResultTypes.Information, PipelineResultTypes.Output);
return psCommand;
}

/// <summary>
/// Get a representation of the PSCommand, for logging purposes.
/// </summary>
public static string GetInvocationText(this PSCommand command)
{
Command lastCommand = command.Commands[0];
Command currentCommand = command.Commands[0];
var sb = new StringBuilder().AddCommandText(command.Commands[0]);

for (int i = 1; i < command.Commands.Count; i++)
{
sb.Append(lastCommand.IsEndOfStatement ? "; " : " | ");
lastCommand = command.Commands[i];
sb.AddCommandText(lastCommand);
sb.Append(currentCommand.IsEndOfStatement ? "; " : " | ");
currentCommand = command.Commands[i];
sb.AddCommandText(currentCommand);
}

return sb.ToString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,21 @@ public static void InvokeCommand(this PowerShell pwsh, PSCommand psCommand)
pwsh.InvokeAndClear();
}

/// <summary>
/// When running a remote session, waits for remote processing and output to complete.
/// </summary>
public static void WaitForRemoteOutputIfNeeded(this PowerShell pwsh)
{
if (!pwsh.Runspace.RunspaceIsRemote)
{
return;
}

// These methods are required when running commands remotely.
// Remote rendering from command output is done asynchronously.
// So to ensure we wait for output to be rendered,
// we need these methods to wait for rendering.
// PowerShell does this in its own implementation: https://github.com/PowerShell/PowerShell/blob/883ca98dd74ea13b3d8c0dd62d301963a40483d6/src/System.Management.Automation/engine/debugger/debugger.cs#L4628-L4652
s_waitForServicingComplete(pwsh);
s_suspendIncomingData(pwsh);
}
Expand Down Expand Up @@ -155,10 +163,10 @@ public static void LoadProfiles(this PowerShell pwsh, ProfilePathInfo profilePat
{
var profileVariable = new PSObject();

pwsh.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.AllUsersAllHosts), profilePaths.AllUsersAllHosts);
pwsh.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.AllUsersCurrentHost), profilePaths.AllUsersCurrentHost);
pwsh.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.CurrentUserAllHosts), profilePaths.CurrentUserAllHosts);
pwsh.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.CurrentUserCurrentHost), profilePaths.CurrentUserCurrentHost);
pwsh.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.AllUsersAllHosts), profilePaths.AllUsersAllHosts)
.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.AllUsersCurrentHost), profilePaths.AllUsersCurrentHost)
.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.CurrentUserAllHosts), profilePaths.CurrentUserAllHosts)
.AddProfileMemberAndLoadIfExists(profileVariable, nameof(profilePaths.CurrentUserCurrentHost), profilePaths.CurrentUserCurrentHost);

pwsh.Runspace.SessionStateProxy.SetVariable("PROFILE", profileVariable);
}
Expand Down Expand Up @@ -189,7 +197,7 @@ public static string GetErrorString(this PowerShell pwsh)
return sb.ToString();
}

private static void AddProfileMemberAndLoadIfExists(this PowerShell pwsh, PSObject profileVariable, string profileName, string profilePath)
private static PowerShell AddProfileMemberAndLoadIfExists(this PowerShell pwsh, PSObject profileVariable, string profileName, string profilePath)
{
profileVariable.Members.Add(new PSNoteProperty(profileName, profilePath));

Expand All @@ -201,6 +209,8 @@ private static void AddProfileMemberAndLoadIfExists(this PowerShell pwsh, PSObje

pwsh.InvokeCommand(psCommand);
}

return pwsh;
}

private static StringBuilder AddErrorString(this StringBuilder sb, ErrorRecord error, int errorIndex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,9 @@ internal static class RunspaceExtensions
static RunspaceExtensions()
{
// PowerShell ApartmentState APIs aren't available in PSStandard, so we need to use reflection.
if (!VersionUtils.IsNetCore || VersionUtils.IsPS7OrGreater)
{
MethodInfo setterInfo = typeof(Runspace).GetProperty("ApartmentState").GetSetMethod();
Delegate setter = Delegate.CreateDelegate(typeof(Action<Runspace, ApartmentState>), firstArgument: null, method: setterInfo);
s_runspaceApartmentStateSetter = (Action<Runspace, ApartmentState>)setter;
}
MethodInfo setterInfo = typeof(Runspace).GetProperty("ApartmentState").GetSetMethod();
Delegate setter = Delegate.CreateDelegate(typeof(Action<Runspace, ApartmentState>), firstArgument: null, method: setterInfo);
s_runspaceApartmentStateSetter = (Action<Runspace, ApartmentState>)setter;

MethodInfo getRemotePromptMethod = typeof(HostUtilities).GetMethod("GetRemotePrompt", BindingFlags.NonPublic | BindingFlags.Static);
ParameterExpression runspaceParam = Expression.Parameter(typeof(Runspace));
Expand All @@ -45,6 +42,13 @@ public static void SetApartmentStateToSta(this Runspace runspace)
s_runspaceApartmentStateSetter?.Invoke(runspace, ApartmentState.STA);
}

/// <summary>
/// Augment a given prompt string with a remote decoration.
/// This is an internal method on <c>Runspace</c> in PowerShell that we reuse via reflection.
/// </summary>
/// <param name="runspace">The runspace the prompt is for.</param>
/// <param name="basePrompt">The base prompt to decorate.</param>
/// <returns>A prompt string decorated with remote connection details.</returns>
public static string GetRemotePrompt(this Runspace runspace, string basePrompt)
{
return s_getRemotePromptFunc(runspace, basePrompt);
Expand Down
Loading