Skip to content
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

Need to handle the different Get-Content calls for different versions of pswh #597

Closed
TylerLeonhardt opened this issue Jan 4, 2018 · 6 comments
Assignees
Labels
Issue-Enhancement A feature request (enhancement).

Comments

@TylerLeonhardt
Copy link
Member

PSCore removed -Encoding Byte for -AsByteStream.

This line:

$contentBytes = Get-Content -Path $filePathName -Raw -Encoding Byte

We should handle this like:

$params = @{this=that}
if ($PSVersionTable.PSEdition -eq "Core")
{
    $params += @{AsByteStream=$true}
}
else
{
    $params += @{Encoding="Byte"}
}

Get-Content @params

This will fix at least 1 bug with psedit over ssh. This might actually fix psedit for PSCore on Windows.

@TylerLeonhardt TylerLeonhardt added the Issue-Enhancement A feature request (enhancement). label Jan 4, 2018
@TylerLeonhardt TylerLeonhardt self-assigned this Jan 4, 2018
@rkeithhill
Copy link
Contributor

Good catch!

@daviwil
Copy link
Contributor

daviwil commented Jan 4, 2018

Ouch!

@TylerLeonhardt
Copy link
Member Author

But wait, there's more to this "psedit over ssh" bug... Check this out:
https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices/Session/RemoteFileManager.cs#L356-L368

Notice the use of computerName?

On macOS and Linux, $env:ComputerName is null. I'm thinking we can just have a null check there and set it to some other unique identifier. Anyone have any ideas of what would also be a good enough identifier?

@rkeithhill
Copy link
Contributor

Try this:

$ExecutionContext.Host.Runspace.ConnectionInfo.ComputerName

But you have to check for ConnectionInfo eq $null. if it is, then use localhost.

@TylerLeonhardt
Copy link
Member Author

Where should I try that, exactly?

Here? https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices/Session/SessionDetails.cs#L55-L64

That's where the ComputerName gets set and ComputerName seems to be used throughout PSES. I worry about situations like this:
https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs#L388

That use -ComputerName when macOS and Linux don't have $env:ComputerName set.. @SteveL-MSFT is that by design, btw - macOS and Linux not having a ComputerName?

@rkeithhill
Copy link
Contributor

Yes, modify that to something like:

"@{ 'computerName' = if ($ExecutionContext.Host.Runspace.ConnectionInfo) {$ExecutionContext.Host.Runspace.ConnectionInfo.ComputerName}  else {'localhost'}; ...

TylerLeonhardt pushed a commit to TylerLeonhardt/PowerShellEditorServices that referenced this issue Feb 26, 2019
Enable saving remote files opened with psedit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement A feature request (enhancement).
Projects
None yet
Development

No branches or pull requests

3 participants