Skip to content

NamedPipeConnectionInfo <= Enter-PSHostProcess #881

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 3 commits into from
Mar 20, 2019
Merged

NamedPipeConnectionInfo <= Enter-PSHostProcess #881

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0cbeef
NamedPipeConnectionInfo <= Enter-PSHostProcess
TylerLeonhardt Mar 18, 2019
225a371
var
TylerLeonhardt Mar 18, 2019
fba3022
address comments
TylerLeonhardt Mar 20, 2019
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
61 changes: 34 additions & 27 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Management.Automation;
using System.Management.Automation.Language;
using System.Management.Automation.Runspaces;
using System.Text;
Expand All @@ -28,6 +27,8 @@

namespace Microsoft.PowerShell.EditorServices.Protocol.Server
{
using System.Management.Automation;

public class LanguageServer
{
private static CancellationTokenSource s_existingRequestCancellation;
Expand Down Expand Up @@ -1225,44 +1226,50 @@ protected async Task HandleGetRunspaceRequestAsync(
string processId,
RequestContext<GetRunspaceResponse[]> requestContext)
{
var runspaceResponses = new List<GetRunspaceResponse>();
IEnumerable<PSObject> runspaces = null;

if (this.editorSession.PowerShellContext.LocalPowerShellVersion.Version.Major >= 5)
{
if (processId == null) {
processId = "current";
}

var isNotCurrentProcess = processId != null && processId != "current";

var psCommand = new PSCommand();

if (isNotCurrentProcess) {
psCommand.AddCommand("Enter-PSHostProcess").AddParameter("Id", processId).AddStatement();
}

psCommand.AddCommand("Get-Runspace");

StringBuilder sb = new StringBuilder();
IEnumerable<Runspace> runspaces = await editorSession.PowerShellContext.ExecuteCommandAsync<Runspace>(psCommand, sb);
if (runspaces != null)
// If the processId is a valid int, we need to run Get-Runspace within that process
// otherwise just use the current runspace
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// otherwise just use the current runspace
// otherwise just use the current runspace.

if (int.TryParse(processId, out int pid))
{
foreach (var p in runspaces)
// Create a remote runspace that we will invoke Get-Runspace in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Create a remote runspace that we will invoke Get-Runspace in
// Create a remote runspace that we will invoke Get-Runspace in.

using(var rs = RunspaceFactory.CreateRunspace(new NamedPipeConnectionInfo(pid)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work with all supported versions of PowerShell?

Copy link
Member Author

@TylerLeonhardt TylerLeonhardt Mar 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch... this use to be a runtime check using Enter-PSHostProcess but now it's an assembly load check...

This will work in PowerShell 5.1+ so it's not a problem in this PR.

But when I backport I need to ifdef this properly...

using(var ps = PowerShell.Create())
{
runspaceResponses.Add(
new GetRunspaceResponse
{
Id = p.Id,
Name = p.Name,
Availability = p.RunspaceAvailability.ToString()
});
rs.Open();
ps.Runspace = rs;
// returns deserialized Runspaces. For simpler code, we use PSObject and rely on dynamic later.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// returns deserialized Runspaces. For simpler code, we use PSObject and rely on dynamic later.
// Returns deserialized Runspaces. For simpler code, we use PSObject and rely on dynamic later.

runspaces = ps.AddCommand("Get-Runspace").Invoke<PSObject>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
runspaces = ps.AddCommand("Get-Runspace").Invoke<PSObject>();
runspaces = ps.AddCommand("Microsoft.PowerShell.Utility\\Get-Runspace").Invoke<PSObject>();

}
}
else
{
var psCommand = new PSCommand().AddCommand("Get-Runspace");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
var psCommand = new PSCommand().AddCommand("Get-Runspace");
var psCommand = new PSCommand().AddCommand("Microsoft.PowerShell.Utility\\Get-Runspace");

StringBuilder sb = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use var here - channeling my inner @rjmholt. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

got me!

// returns (not deserialized) Runspaces. For simpler code, we use PSObject and rely on dynamic later.
runspaces = await editorSession.PowerShellContext.ExecuteCommandAsync<PSObject>(psCommand, sb);
}
}

var runspaceResponses = new List<GetRunspaceResponse>();

if (isNotCurrentProcess) {
var exitCommand = new PSCommand();
exitCommand.AddCommand("Exit-PSHostProcess");
await editorSession.PowerShellContext.ExecuteCommandAsync(exitCommand);
if (runspaces != null)
{
foreach (dynamic p in runspaces)
Copy link
Collaborator

@SeeminglyScience SeeminglyScience Mar 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Consider using a more descriptive variable name like runspace or even pso.

Edit: didn't realize this was existing code, but maybe still worth fixing.

{
runspaceResponses.Add(
new GetRunspaceResponse
{
Id = p.Id,
Name = p.Name,
Availability = p.RunspaceAvailability.ToString()
});
}
}

Expand Down