-
Notifications
You must be signed in to change notification settings - Fork 238
EditorServiceHost: allow Tcp/NamedPipe/Stdio listeners #629
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
Changes from 5 commits
318dbb7
ae39b16
941ab40
ed6b722
efdf05a
8de969d
17ce8b7
66da3fe
179cf80
2da1168
f73d784
592d29d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,16 +31,21 @@ function Start-EditorServicesHost { | |
[string] | ||
$HostVersion, | ||
|
||
[Parameter(Mandatory=$true)] | ||
[ValidateNotNullOrEmpty()] | ||
[int] | ||
$LanguageServicePort, | ||
|
||
[Parameter(Mandatory=$true)] | ||
[ValidateNotNullOrEmpty()] | ||
[int] | ||
$DebugServicePort, | ||
|
||
[bool] | ||
$Stdio, | ||
|
||
[string] | ||
$LanguageServiceNamedPipe, | ||
|
||
[string] | ||
$DebugServiceNamedPipe, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the different connection types be separate parameter sets? For example
|
||
|
||
[ValidateNotNullOrEmpty()] | ||
[string] | ||
$BundledModulesPath, | ||
|
@@ -89,12 +94,39 @@ function Start-EditorServicesHost { | |
|
||
$editorServicesHost.StartLogging($LogPath, $LogLevel); | ||
|
||
if ($DebugServiceOnly.IsPresent) { | ||
$editorServicesHost.StartDebugService($DebugServicePort, $profilePaths, $false); | ||
$languageServiceConfig = New-Object Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportConfig | ||
$debugServiceConfig = New-Object Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportConfig | ||
|
||
if ($Stdio) { | ||
$languageServiceConfig.TransportType = [Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::Stdio | ||
$debugServiceConfig.TransportType = [Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::Stdio | ||
} | ||
else { | ||
$editorServicesHost.StartLanguageService($LanguageServicePort, $profilePaths); | ||
$editorServicesHost.StartDebugService($DebugServicePort, $profilePaths, $true); | ||
|
||
if ($LanguageServicePort) { | ||
$languageServiceConfig.TransportType = [Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::Tcp | ||
$languageServiceConfig.Endpoint = "$LanguageServicePort" | ||
} | ||
|
||
if ($DebugServicePort) { | ||
$debugServiceConfig.TransportType = [Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::Tcp | ||
$debugServiceConfig.Endpoint = "$DebugServicePort" | ||
} | ||
|
||
if ($LanguageServiceNamedPipe) { | ||
$languageServiceConfig.TransportType = [Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::NamedPipe | ||
$languageServiceConfig.Endpoint = "$LanguageServiceNamedPipe" | ||
} | ||
|
||
if ($DebugServiceNamedPipe) { | ||
$debugServiceConfig.TransportType = [Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::NamedPipe | ||
$debugServiceConfig.Endpoint = "$DebugServiceNamedPipe" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With parameter sets this would work nicely as a switch statement switch ($PSCmdlet.ParameterSetName) {
ByTcp {
$languageServiceConfig.TransportType = $debugServiceConfig.TransportType =
Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::Tcp
$languageServiceConfig.Endpoint = $LanguageServicePort
$debugServiceConfig.Endpoint = $DebugServicePort
}
ByNamedPipe {
$languageServiceConfig.TransportType = $debugServiceConfig.TransportType =
[Microsoft.PowerShell.EditorServices.Host.EditorServiceTransportType]::NamedPipe
$languageServiceConfig.Endpoint = $LanguageServiceNamedPipe
$debugServiceConfig.Endpoint = $DebugServiceNamedPipe
}
# etc...
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ooo that's nice 😍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @SeeminglyScience but if you'd prefer we can add the parameter sets after this gets merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah this is totally cool! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just thought about this for a while.. A few days ago I was debugging a problem when both language service and debug service are talking to stdio and the channel gets too crowded. In fact these two should never sit on the same stdio train. :) |
||
|
||
if ($DebugServiceOnly.IsPresent) { | ||
$editorServicesHost.StartDebugService($debugServiceConfig, $profilePaths, $false); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge thing but the script style for this file doesn't use cuddled else. Again, something we can tweak after merging. |
||
$editorServicesHost.StartLanguageService($languageServiceConfig, $profilePaths); | ||
$editorServicesHost.StartDebugService($debugServiceConfig, $profilePaths, $true); | ||
} | ||
|
||
return $editorServicesHost | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,19 @@ param( | |
$WaitForDebugger, | ||
|
||
[switch] | ||
$ConfirmInstall | ||
$ConfirmInstall, | ||
|
||
[switch] | ||
$Stdio, | ||
|
||
[switch] | ||
$DebugServiceOnly, | ||
|
||
[string] | ||
$LanguageServicePipeName = $null, | ||
|
||
[string] | ||
$DebugServicePipeName = $null | ||
) | ||
|
||
# This variable will be assigned later to contain information about | ||
|
@@ -149,8 +161,9 @@ if ((Test-ModuleAvailable "PowerShellEditorServices" -RequiredVersion $parsedVer | |
Import-Module PowerShellEditorServices -RequiredVersion $parsedVersion -ErrorAction Stop | ||
|
||
# Locate available port numbers for services | ||
$languageServicePort = Get-AvailablePort | ||
$debugServicePort = Get-AvailablePort | ||
# There could be only one service on Stdio channel | ||
if (-not (($Stdio.IsPresent -and -not $DebugServiceOnly.IsPresent) -or $LanguageServicePipeName)) { $languageServicePort = Get-AvailablePort } | ||
if (-not (($Stdio.IsPresent -and $DebugServiceOnly.IsPresent) -or $DebugServicePipeName)) { $debugServicePort = Get-AvailablePort } | ||
|
||
$editorServicesHost = | ||
Start-EditorServicesHost ` | ||
|
@@ -162,16 +175,22 @@ $editorServicesHost = | |
-AdditionalModules @() ` | ||
-LanguageServicePort $languageServicePort ` | ||
-DebugServicePort $debugServicePort ` | ||
-Stdio $Stdio.IsPresent` | ||
-LanguageServiceNamedPipe $LanguageServicePipeName ` | ||
-DebugServiceNamedPipe $DebugServicePipeName ` | ||
-BundledModulesPath $BundledModulesPath ` | ||
-DebugServiceOnly:$DebugServiceOnly.IsPresent` | ||
-WaitForDebugger:$WaitForDebugger.IsPresent | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming the switch to parameter sets, this would need to be switched to a splat (probably should anyway to fit common community style guides) $splat = @{
AdditionalModules = @()
WaitForDebugger = $WaitForDebugger.IsPresent
# etc...
}
switch ($PSCmdlet.ParameterSetName) {
ByTcp {
$splat.LanguageServicePort = $languageServicePort
# etc...
}
}
$editorServicesHost = Start-EditorServicesHost @splat |
||
|
||
# TODO: Verify that the service is started | ||
|
||
$resultDetails = @{ | ||
"status" = "started"; | ||
"channel" = "tcp"; | ||
"languageServicePort" = $languageServicePort; | ||
"debugServicePort" = $debugServicePort; | ||
"languageServiceNamedPipe" = $LanguageServicePipeName; | ||
"debugServiceNamedPipe" = $DebugServicePipeName; | ||
"Stdio" = $Stdio.IsPresent; | ||
}; | ||
|
||
# Notify the client that the services have started | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,5 +34,12 @@ public class FileChange | |
/// The 1-based column offset where the change ends. | ||
/// </summary> | ||
public int EndOffset { get; set; } | ||
|
||
/// <summary> | ||
/// Indicates that the InsertString is an overwrite | ||
/// of the content, and all stale content and metadata | ||
/// should be discarded. | ||
/// </summary> | ||
public bool Reloaded { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is really nit picky, but could we change this member to |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this parameter a
[switch]
parameter instead of[bool]
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, might be better to change name to
UseStdio
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SeeminglyScience I tried to make it a switch as the outer starting script does, but I don't know how to relay the switch in without splitting the command into two, one with the switch and one without. Sorry I'm still learning my way on powershell 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically it is
-InnerSwitch:$OuterSwitch