Skip to content

Add attach to local runspace. #875

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 11 commits into from
Mar 13, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class AttachRequestArguments

public string ProcessId { get; set; }

public int RunspaceId { get; set; }
public string RunspaceId { get; set; }

public string CustomPipeName { get; set; }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;

namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
{
public class GetRunspaceRequest
{
public static readonly
RequestType<object, GetRunspaceResponse[], object, object> Type =
RequestType<object, GetRunspaceResponse[], object, object>.Create("powerShell/getRunspace");
}

public class GetRunspaceResponse
{
public int Id { get; set; }

public string Name { get; set; }

public string Availability { get; set; }
}
}
35 changes: 27 additions & 8 deletions src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -417,16 +417,18 @@ await requestContext.SendErrorAsync(
return;
}

await _editorSession.PowerShellContext.ExecuteScriptStringAsync(
if (attachParams.ProcessId != "current") {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't need this if because the outer if tries to parse the ProcessId as an integer (L411):

if (processIdIsSet && int.TryParse(attachParams.ProcessId, out int processId) && (processId > 0))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updated the PR.

await _editorSession.PowerShellContext.ExecuteScriptStringAsync(
$"Enter-PSHostProcess -Id {processId}",
errorMessages);

if (errorMessages.Length > 0)
{
await requestContext.SendErrorAsync(
$"Could not attach to process '{processId}'");
if (errorMessages.Length > 0)
{
await requestContext.SendErrorAsync(
$"Could not attach to process '{processId}'");

return;
return;
}
}
}
else if (customPipeNameIsSet)
Expand All @@ -450,7 +452,7 @@ await requestContext.SendErrorAsync(
return;
}
}
else
else if (attachParams.ProcessId != "current")
{
Logger.Write(
LogLevel.Error,
Expand All @@ -469,7 +471,24 @@ await requestContext.SendErrorAsync(
// will block the debug adapter initialization process. The
// InitializedEvent will be sent as soon as the RunspaceChanged
// event gets fired with the attached runspace.
int runspaceId = attachParams.RunspaceId > 0 ? attachParams.RunspaceId : 1;

int runspaceId = 1;
if (int.TryParse(attachParams.RunspaceId, out runspaceId))
{
runspaceId = runspaceId > 0 ? runspaceId : 1;
Copy link
Member

Choose a reason for hiding this comment

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

I would maybe factor this into the if

if (!int.TryParse(attachParams.RunspaceId, out runspaceId) || runspaceId <= 0)
{
    Logger.Write(
        LogLevel.Error,
        $"Attach request failed, '{attachParams.RunspaceId}' is an invalid value for the processId.");

    await requestContext.SendErrorAsync(
        "A positive integer must be specified for the RunspaceId field.");

    return;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

}
else
{
Logger.Write(
LogLevel.Error,
$"Attach request failed, '{attachParams.RunspaceId}' is an invalid value for the processId.");

await requestContext.SendErrorAsync(
"A positive integer must be specified for the RunspaceId field.");

return;
}

_waitingForAttach = true;
Task nonAwaitedTask = _editorSession.PowerShellContext
.ExecuteScriptStringAsync($"\nDebug-Runspace -Id {runspaceId}")
Expand Down
33 changes: 33 additions & 0 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System.Linq;
using System.Management.Automation;
using System.Management.Automation.Language;
using System.Management.Automation.Runspaces;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
Expand Down Expand Up @@ -161,6 +162,8 @@ public void Start()
this.messageHandlers.SetRequestHandler(GetPSHostProcessesRequest.Type, this.HandleGetPSHostProcessesRequestAsync);
this.messageHandlers.SetRequestHandler(CommentHelpRequest.Type, this.HandleCommentHelpRequestAsync);

this.messageHandlers.SetRequestHandler(GetRunspaceRequest.Type, this.HandleGetRunspaceRequestAsync);

// Initialize the extension service
// TODO: This should be made awaited once Initialize is async!
this.editorSession.ExtensionService.InitializeAsync(
Expand Down Expand Up @@ -1218,6 +1221,36 @@ protected async Task HandleCommentHelpRequestAsync(
await requestContext.SendResultAsync(result);
}

protected async Task HandleGetRunspaceRequestAsync(
object noParams,
RequestContext<GetRunspaceResponse[]> requestContext)
{
var runspaceResponses = new List<GetRunspaceResponse>();

if (this.editorSession.PowerShellContext.LocalPowerShellVersion.Version.Major >= 5)
Copy link
Member

Choose a reason for hiding this comment

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

Was Get-Runspace added in 5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a really good question. I just took that check from the process picker but I'm not sure when Debug-Runspace\Get-Runspace was added. I can try and find out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

Well looks like that answers our question :)

{
var psCommand = new PSCommand();
psCommand.AddCommand("Get-Runspace");

var runspaces = await editorSession.PowerShellContext.ExecuteCommandAsync<PSObject>(psCommand);
Copy link
Member

Choose a reason for hiding this comment

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

can you replace var with the type since this is a tad more complicated than new Blah()

Copy link
Member

Choose a reason for hiding this comment

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

Also, instead of PSObject you should be able to do System.Management.Automation.Runspaces.Runspace and then you don't have to use dynamic below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

if (runspaces != null)
{
foreach (dynamic p in runspaces)
{
runspaceResponses.Add(
new GetRunspaceResponse
{
Id = p.Id,
Name = p.Name,
Availability = p.RunspaceAvailability.ToString()
});
}
}
}

await requestContext.SendResultAsync(runspaceResponses.ToArray());
}

private bool IsQueryMatch(string query, string symbolName)
{
return symbolName.IndexOf(query, StringComparison.OrdinalIgnoreCase) >= 0;
Expand Down