Skip to content

Commit 800518c

Browse files
Merge pull request #1577 from rjmholt/review-fixes
Address review comments
2 parents aa263eb + 34836e8 commit 800518c

19 files changed

+172
-111
lines changed

src/PowerShellEditorServices/Server/PsesLanguageServer.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ public async Task StartAsync()
113113
// _Initialize_ request:
114114
// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize
115115
.OnInitialize(
116-
// TODO: Either fix or ignore "method lacks 'await'" warning.
117116
(languageServer, request, cancellationToken) =>
118117
{
119118
Log.Logger.Debug("Initializing OmniSharp Language Server");

src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ public static IServiceCollection AddPsesLanguageServices(
4747
provider.GetService<EditorOperationsService>(),
4848
provider.GetService<PowerShellExecutionService>());
4949

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

5257
return extensionService;

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution;
1818
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
1919
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging;
20-
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
2120

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

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

187186
if (dscBreakpoints == null || !dscBreakpoints.IsDscResourcePath(escapedScriptPath))
188187
{

src/PowerShellEditorServices/Services/PowerShell/Console/ColorConfiguration.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
using System.Collections.Generic;
33
using System.Text;
44

5-
namespace PowerShellEditorServices.Services.PowerShell.Console
5+
namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Console
66
{
77
internal class ColorConfiguration
88
{

src/PowerShellEditorServices/Services/PowerShell/Context/PowerShellVersionDetails.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,6 @@ public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerSh
103103

104104
try
105105
{
106-
var psVersionTableCommand = new PSCommand().AddScript("$PSVersionTable", useLocalScope: true);
107-
108106
Hashtable psVersionTable = pwsh
109107
.AddScript("$PSVersionTable", useLocalScope: true)
110108
.InvokeAndClear<Hashtable>()

src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,29 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging
1010
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
1111
using System.Threading.Tasks;
1212

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

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

Lines changed: 0 additions & 12 deletions
This file was deleted.

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,13 @@ private IReadOnlyList<TResult> ExecuteNormally(CancellationToken cancellationTok
7171
result = _pwsh.InvokeCommand<TResult>(_psCommand);
7272
cancellationToken.ThrowIfCancellationRequested();
7373
}
74+
// Test if we've been cancelled. If we're remoting, PSRemotingDataStructureException effectively means the pipeline was stopped.
7475
catch (Exception e) when (cancellationToken.IsCancellationRequested || e is PipelineStoppedException || e is PSRemotingDataStructureException)
7576
{
7677
throw new OperationCanceledException();
7778
}
79+
// We only catch RuntimeExceptions here in case writing errors to output was requested
80+
// Other errors are bubbled up to the caller
7881
catch (RuntimeException e)
7982
{
8083
Logger.LogWarning($"Runtime exception occurred while executing command:{Environment.NewLine}{Environment.NewLine}{e}");
@@ -107,6 +110,9 @@ private IReadOnlyList<TResult> ExecuteInDebugger(CancellationToken cancellationT
107110

108111
var outputCollection = new PSDataCollection<PSObject>();
109112

113+
// Out-Default doesn't work as needed in the debugger
114+
// Instead we add Out-String to the command and collect results in a PSDataCollection
115+
// and use the event handler to print output to the UI as its added to that collection
110116
if (_executionOptions.WriteOutputToHost)
111117
{
112118
_psCommand.AddDebugOutputCommand();
@@ -124,14 +130,21 @@ private IReadOnlyList<TResult> ExecuteInDebugger(CancellationToken cancellationT
124130
DebuggerCommandResults debuggerResult = null;
125131
try
126132
{
133+
// In the PowerShell debugger, extra debugger commands are made available, like "l", "s", "c", etc.
134+
// Executing those commands produces a result that needs to be set on the debugger stop event args.
135+
// So we use the Debugger.ProcessCommand() API to properly execute commands in the debugger
136+
// and then call DebugContext.ProcessDebuggerResult() later to handle the command appropriately
127137
debuggerResult = _pwsh.Runspace.Debugger.ProcessCommand(_psCommand, outputCollection);
128138
cancellationToken.ThrowIfCancellationRequested();
129139
}
140+
// Test if we've been cancelled. If we're remoting, PSRemotingDataStructureException effectively means the pipeline was stopped.
130141
catch (Exception e) when (cancellationToken.IsCancellationRequested || e is PipelineStoppedException || e is PSRemotingDataStructureException)
131142
{
132143
StopDebuggerIfRemoteDebugSessionFailed();
133144
throw new OperationCanceledException();
134145
}
146+
// We only catch RuntimeExceptions here in case writing errors to output was requested
147+
// Other errors are bubbled up to the caller
135148
catch (RuntimeException e)
136149
{
137150
Logger.LogWarning($"Runtime exception occurred while executing command:{Environment.NewLine}{Environment.NewLine}{e}");

src/PowerShellEditorServices/Services/PowerShell/Handlers/EvaluateHandler.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010

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

2630
public Task<EvaluateResponseBody> Handle(EvaluateRequestArguments request, CancellationToken cancellationToken)
2731
{
32+
// TODO: Understand why we currently handle this asynchronously and why we return a dummy result value
33+
// instead of awaiting the execution and returing a real result of some kind
34+
2835
_executionService.ExecutePSCommandAsync(
2936
new PSCommand().AddScript(request.Expression),
3037
new PowerShellExecutionOptions { WriteInputToHost = true, WriteOutputToHost = true, AddToHistory = true, InterruptCommandPrompt = true },

src/PowerShellEditorServices/Services/PowerShell/Runspace/SessionDetails.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,9 @@ internal class SessionDetails
2222
private const string Property_InstanceId = "instanceId";
2323

2424
/// <summary>
25-
/// Gets the PSCommand that gathers details from the
26-
/// current session.
25+
/// Runs a PowerShell command to gather details about the current session.
2726
/// </summary>
28-
/// <returns>A PSCommand used to gather session details.</returns>
27+
/// <returns>A data object containing details about the PowerShell session.</returns>
2928
public static SessionDetails GetFromPowerShell(PowerShell pwsh)
3029
{
3130
Hashtable detailsObject = pwsh

0 commit comments

Comments
 (0)