Skip to content

Code cleanup of the start script and ESHost.cs file #796

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
Nov 25, 2018
Merged

Code cleanup of the start script and ESHost.cs file #796

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f0a54f
Code cleanup of the start script and ESHost.cs file
rkeithhill Nov 23, 2018
0eb17e8
Address PR feedback - remove redundant version checking, add comment
rkeithhill Nov 24, 2018
3743b05
Address PR feedback - use parameter labels and named parameters
rkeithhill Nov 24, 2018
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
98 changes: 53 additions & 45 deletions module/PowerShellEditorServices/Start-EditorServices.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
# PowerShell Editor Services Bootstrapper Script
# ----------------------------------------------
# This script contains startup logic for the PowerShell Editor Services
# module when launched by an editor. It handles the following tasks:
#
# - Verifying the existence of dependencies like PowerShellGet
# - Verifying that the expected version of the PowerShellEditorServices module is installed
# - Installing the PowerShellEditorServices module if confirmed by the user
# - Creating named pipes for the language and debug services to use (if using named pipes)
# - Starting the language and debug services from the PowerShellEditorServices module
#
# NOTE: If editor integration authors make modifications to this
# script, please consider contributing changes back to the
# canonical version of this script at the PowerShell Editor
# Services GitHub repository:
#
# https://github.com/PowerShell/PowerShellEditorServices/blob/master/module/PowerShellEditorServices/Start-EditorServices.ps1
<#
.SYNOPSIS
Starts the language and debug services from the PowerShellEditorServices module.
.DESCRIPTION
PowerShell Editor Services Bootstrapper Script
----------------------------------------------
This script contains startup logic for the PowerShell Editor Services
module when launched by an editor. It handles the following tasks:

- Verifying the existence of dependencies like PowerShellGet
- Verifying that the expected version of the PowerShellEditorServices module is installed
- Installing the PowerShellEditorServices module if confirmed by the user
- Creating named pipes for the language and debug services to use (if using named pipes)
- Starting the language and debug services from the PowerShellEditorServices module
.INPUTS
None
.OUTPUTS
None
.NOTES
If editor integration authors make modifications to this script, please
consider contributing changes back to the canonical version of this script
at the PowerShell Editor Services GitHub repository:
https://github.com/PowerShell/PowerShellEditorServices/blob/master/module/PowerShellEditorServices/Start-EditorServices.ps1'
#>
[CmdletBinding(DefaultParameterSetName="NamedPipe")]
param(
[Parameter(Mandatory=$true)]
Expand Down Expand Up @@ -65,7 +72,7 @@ param(
[switch]
$ConfirmInstall,

[Parameter(ParameterSetName="Stdio",Mandatory=$true)]
[Parameter(ParameterSetName="Stdio", Mandatory=$true)]
[switch]
$Stdio,

Expand Down Expand Up @@ -154,9 +161,6 @@ if ($host.Runspace.LanguageMode -eq 'ConstrainedLanguage') {
ExitWithError "PowerShell is configured with an unsupported LanguageMode (ConstrainedLanguage), language features are disabled."
}

# Are we running in PowerShell 5 or later?
$isPS5orLater = $PSVersionTable.PSVersion.Major -ge 5

# If PSReadline is present in the session, remove it so that runspace
# management is easier
if ((Microsoft.PowerShell.Core\Get-Module PSReadline).Count -gt 0) {
Expand All @@ -173,8 +177,8 @@ function Test-ModuleAvailable($ModuleName, $ModuleVersion) {
Log "Testing module availability $ModuleName $ModuleVersion"

$modules = Microsoft.PowerShell.Core\Get-Module -ListAvailable $moduleName
if ($modules -ne $null) {
if ($ModuleVersion -ne $null) {
if ($null -ne $modules) {
if ($null -ne $ModuleVersion) {
foreach ($module in $modules) {
if ($module.Version.Equals($moduleVersion)) {
Log "$ModuleName $ModuleVersion found"
Expand All @@ -193,7 +197,6 @@ function Test-ModuleAvailable($ModuleName, $ModuleVersion) {
}

function New-NamedPipeName {

# We try 10 times to find a valid pipe name
for ($i = 0; $i -lt 10; $i++) {
$PipeName = "PSES_$([System.IO.Path]::GetRandomFileName())"
Expand All @@ -202,6 +205,7 @@ function New-NamedPipeName {
return $PipeName
}
}

ExitWithError "Could not find valid a pipe name."
}

Expand All @@ -213,15 +217,14 @@ function Get-NamedPipePath {
$PipeName
)

if (-not $IsLinux -and -not $IsMacOS) {
if (($PSVersionTable.PSVersion.Major -le 5) -or $IsWindows) {
return "\\.\pipe\$PipeName";
}
else {
# Windows uses NamedPipes where non-Windows platforms use Unix Domain Sockets.
# the Unix Domain Sockets live in the tmp directory and are prefixed with "CoreFxPipe_"
return (Join-Path -Path ([System.IO.Path]::GetTempPath()) -ChildPath "CoreFxPipe_$PipeName")
}

}

# Returns True if it's a valid pipe name
Expand All @@ -236,7 +239,7 @@ function Test-NamedPipeName {
)

$path = Get-NamedPipePath -PipeName $PipeName
return -not (Test-Path $path)
return !(Test-Path $path)
}

function Set-NamedPipeMode {
Expand All @@ -247,16 +250,16 @@ function Set-NamedPipeMode {
$PipeFile
)

if ($IsWindows) {
if (($PSVersionTable.PSVersion.Major -le 5) -or $IsWindows) {
return
}

chmod $DEFAULT_USER_MODE $PipeFile

if ($IsLinux) {
if (($PSVersionTable.PSVersion.Major -ge 6) -and $IsLinux) {
$mode = /usr/bin/stat -c "%a" $PipeFile
}
elseif ($IsMacOS) {
elseif (($PSVersionTable.PSVersion.Major -ge 6) -and $IsMacOS) {
$mode = /usr/bin/stat -f "%A" $PipeFile
}

Expand All @@ -268,33 +271,36 @@ function Set-NamedPipeMode {
LogSection "Console Encoding"
Log $OutputEncoding

function Test-NamedPipeName-OrCreate-IfNull {
function Get-ValidatedNamedPipeName {
Copy link
Contributor

Choose a reason for hiding this comment

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

So much nicer!

param(
[string]
$PipeName
)
if (-not $PipeName) {

if (!$PipeName) {
$PipeName = New-NamedPipeName
}
else {
if (-not (Test-NamedPipeName -PipeName $PipeName)) {
ExitWithError "Pipe name supplied is already taken: $PipeName"
}
elseif (!(Test-NamedPipeName -PipeName $PipeName)) {
ExitWithError "Pipe name supplied is already in use: $PipeName"
}

return $PipeName
}

function Set-PipeFileResult {
param (
[Hashtable]
$ResultTable,

[string]
$PipeNameKey,

[string]
$PipeNameValue
)

$ResultTable[$PipeNameKey] = Get-NamedPipePath -PipeName $PipeNameValue
if ($IsLinux -or $IsMacOS) {
if (($PSVersionTable.PSVersion.Major -ge 6) -and ($IsLinux -or $IsMacOS)) {
Set-NamedPipeMode -PipeFile $ResultTable[$PipeNameKey]
}
}
Expand Down Expand Up @@ -349,11 +355,12 @@ try {
-WaitForDebugger:$WaitForDebugger.IsPresent
break
}

"NamedPipeSimplex" {
$LanguageServiceInPipeName = Test-NamedPipeName-OrCreate-IfNull $LanguageServiceInPipeName
$LanguageServiceOutPipeName = Test-NamedPipeName-OrCreate-IfNull $LanguageServiceOutPipeName
$DebugServiceInPipeName = Test-NamedPipeName-OrCreate-IfNull $DebugServiceInPipeName
$DebugServiceOutPipeName = Test-NamedPipeName-OrCreate-IfNull $DebugServiceOutPipeName
$LanguageServiceInPipeName = Get-ValidatedNamedPipeName $LanguageServiceInPipeName
$LanguageServiceOutPipeName = Get-ValidatedNamedPipeName $LanguageServiceOutPipeName
$DebugServiceInPipeName = Get-ValidatedNamedPipeName $DebugServiceInPipeName
$DebugServiceOutPipeName = Get-ValidatedNamedPipeName $DebugServiceOutPipeName

$editorServicesHost = Start-EditorServicesHost `
-HostName $HostName `
Expand All @@ -377,9 +384,10 @@ try {
Set-PipeFileResult $resultDetails "debugServiceWritePipeName" $DebugServiceOutPipeName
break
}

Default {
$LanguageServicePipeName = Test-NamedPipeName-OrCreate-IfNull $LanguageServicePipeName
$DebugServicePipeName = Test-NamedPipeName-OrCreate-IfNull $DebugServicePipeName
$LanguageServicePipeName = Get-ValidatedNamedPipeName $LanguageServicePipeName
$DebugServicePipeName = Get-ValidatedNamedPipeName $DebugServicePipeName

$editorServicesHost = Start-EditorServicesHost `
-HostName $HostName `
Expand Down Expand Up @@ -417,7 +425,7 @@ catch [System.Exception] {

Log "ERRORS caught starting up EditorServicesHost"

while ($e -ne $null) {
while ($null -ne $e) {
$errorString = $errorString + ($e.Message + "`r`n" + $e.StackTrace + "`r`n")
$e = $e.InnerException;
Log $errorString
Expand All @@ -438,7 +446,7 @@ catch [System.Exception] {

Log "ERRORS caught while waiting for EditorServicesHost to complete execution"

while ($e -ne $null) {
while ($null -ne $e) {
$errorString = $errorString + ($e.Message + "`r`n" + $e.StackTrace + "`r`n")
$e = $e.InnerException;
Log $errorString
Expand Down
8 changes: 6 additions & 2 deletions src/PowerShellEditorServices.Host/EditorServicesHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ public class EditorServiceTransportConfig
/// For NamedPipe it's the pipe name.
/// </summary>
public string InOutPipeName { get; set; }

public string OutPipeName { get; set; }

public string InPipeName { get; set; }

internal string Endpoint => OutPipeName != null && InPipeName != null ? $"In pipe: {InPipeName} Out pipe: {OutPipeName}" : $" InOut pipe: {InOutPipeName}";
}

Expand Down Expand Up @@ -243,7 +246,7 @@ await this.editorSession.PowerShellContext.ImportCommandsModule(
foreach (string module in this.additionalModules)
{
await this.editorSession.PowerShellContext.ExecuteCommand<System.Management.Automation.PSObject>(
new System.Management.Automation.PSCommand().AddCommand("Import-Module").AddArgument(module),
new System.Management.Automation.PSCommand().AddCommand("Microsoft.PowerShell.Core\\Import-Module").AddArgument(module),
false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a good time to label these arguments too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean .AddCommand("Microsoft.PowerShell.Core\\Import-Module").AddParameter("Name", module),? If so, yeah, makes senses to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I meant the true and false in C#. But I'm in favour of all of it :)

true);
}
Expand Down Expand Up @@ -455,6 +458,7 @@ private void CurrentDomain_UnhandledException(
e.ExceptionObject.ToString()));
}
#endif

private IServerListener CreateServiceListener(MessageProtocolType protocol, EditorServiceTransportConfig config)
{
switch (config.TransportType)
Expand All @@ -466,7 +470,7 @@ private IServerListener CreateServiceListener(MessageProtocolType protocol, Edit

case EditorServiceTransportType.NamedPipe:
{
if (config.OutPipeName !=null && config.InPipeName !=null)
if ((config.OutPipeName != null) && (config.InPipeName != null))
{
this.logger.Write(LogLevel.Verbose, $"Creating NamedPipeServerListener for ${protocol} protocol with two pipes: In: '{config.InPipeName}'. Out: '{config.OutPipeName}'");
return new NamedPipeServerListener(protocol, config.InPipeName, config.OutPipeName, this.logger);
Expand Down