Skip to content

Fix debugger step through on Unix platforms #592

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 1 commit into from
Jan 6, 2018
Merged

Fix debugger step through on Unix platforms #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions PowerShellEditorServices.build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ task TestPowerShellApi -If { !$script:IsUnix } {
task Build {
exec { & $script:dotnetExe build -c $Configuration .\src\PowerShellEditorServices.Host\PowerShellEditorServices.Host.csproj $script:TargetFrameworksParam }
exec { & $script:dotnetExe build -c $Configuration .\src\PowerShellEditorServices.VSCode\PowerShellEditorServices.VSCode.csproj $script:TargetFrameworksParam }
exec { & $script:dotnetExe publish -c $Configuration .\src\PowerShellEditorServices\PowerShellEditorServices.csproj -f netstandard1.6 }
Copy link
Member

Choose a reason for hiding this comment

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

Just curious - why are you are calling dotnet publish? Does a normal build not have what you're looking for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, dependencies aren't copied on build when you target standard or core. publish is supposed to the the final product with everything needed to run including packages, runtimes and framework assemblies. Most of that is already provided by being hosted in PowerShell though, so I just pick out what's needed.

Copy-Item $PSScriptRoot\src\PowerShellEditorServices\bin\$Configuration\netstandard1.6\publish\UnixConsoleEcho.dll -Destination $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\netstandard1.6
Copy-Item $PSScriptRoot\src\PowerShellEditorServices\bin\$Configuration\netstandard1.6\publish\runtimes\osx-64\native\libdisablekeyecho.dylib -Destination $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\netstandard1.6
Copy-Item $PSScriptRoot\src\PowerShellEditorServices\bin\$Configuration\netstandard1.6\publish\runtimes\linux-64\native\libdisablekeyecho.so -Destination $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\netstandard1.6
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not my wheelhouse but are we comfortable that this .so (and the above .dylib) will work on all OS distributions supported by VSCode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rkeithhill Good question, I initially only tested on Ubuntu. I just pulled the microsoft/powershell docker images for centos7, opensuse42.1, and opensuse13.2 to make sure they at least can load the lib and turn off echo. But that's not a full test, and I don't know enough about docker and Linux to know if what I did is even an accurate test. I'm pretty comfortable because of how small the code is, but I definitely can't say with absolute certainly that it will work everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

I think you could maybe be a little bit more certain on docker by installing vscode in the image along with some vnc server. Then you should be able to rdp/vnc into the container and mess around with VSCode to verify the fix.

It's possibly a bit lengthly, but if you want to be more certain, it will certainly help.

I'm not actually 100% certain if this is possible though - haven't tried it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tylerl0706 Yeah for sure, though it sounds like about the same time and effort as spinning up a vm for each distro. Either way that's a bit more involved than I have the bandwidth for, but if someone wants to take that up I'm all for it.

Copy link
Member

Choose a reason for hiding this comment

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

I'll make some Dockerfiles one of these days that can grab the distros, install PowerShell, VNC, VSCode, and the extension so we have them for next time.

}

function UploadTestLogs {
Expand Down Expand Up @@ -185,9 +189,12 @@ task LayoutModule -After Build {
New-Item -Force $PSScriptRoot\module\PowerShellEditorServices\bin\Core -Type Directory | Out-Null

Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\netstandard1.6\* -Filter Microsoft.PowerShell.EditorServices*.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Core\
Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\netstandard1.6\UnixConsoleEcho.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Core\
Copy link
Contributor

Choose a reason for hiding this comment

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

@tylerl0706 before you can ship these binaries with PSES and the PowerShell extension, you'll need to request OSS approval for Patrick's library. You may also need to include his copyright in a Third Party Notices file like the one in the PS extension. I highly doubt they will reject the request to use this code but better to follow the necessary process anyway :)

Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\netstandard1.6\libdisablekeyecho.* -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Core\
if (!$script:IsUnix) {
Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\net451\* -Filter Microsoft.PowerShell.EditorServices*.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Desktop\
Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\net451\Newtonsoft.Json.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Desktop\
Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\net451\UnixConsoleEcho.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Desktop\
}

# Lay out the PowerShellEditorServices.VSCode module's binaries
Expand Down
64 changes: 25 additions & 39 deletions src/PowerShellEditorServices/Console/ConsoleReadLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using UnixConsoleEcho;

namespace Microsoft.PowerShell.EditorServices.Console
{
Expand All @@ -20,8 +21,6 @@ internal class ConsoleReadLine
{
#region Private Field

private object readKeyLock = new object();
private ConsoleKeyInfo? bufferedKey;
private PowerShellContext powerShellContext;

#endregion
Expand Down Expand Up @@ -61,7 +60,7 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok
{
while (!cancellationToken.IsCancellationRequested)
{
ConsoleKeyInfo keyInfo = await this.ReadKeyAsync(cancellationToken);
ConsoleKeyInfo keyInfo = await ReadKeyAsync(cancellationToken);

if ((int)keyInfo.Key == 3 ||
keyInfo.Key == ConsoleKey.C && keyInfo.Modifiers.HasFlag(ConsoleModifiers.Control))
Expand Down Expand Up @@ -129,6 +128,28 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok

#region Private Methods

private static async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken)
{
await WaitForKeyAvailableAsync(cancellationToken);
return Console.ReadKey(true);
}

private static async Task WaitForKeyAvailableAsync(CancellationToken cancellationToken)
{
InputEcho.Disable();
try
{
while (!Console.KeyAvailable)
{
await Task.Delay(50, cancellationToken);
}
}
finally
{
InputEcho.Enable();
}
}

private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancellationToken)
{
string inputBeforeCompletion = null;
Expand All @@ -154,7 +175,7 @@ private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancel
{
while (!cancellationToken.IsCancellationRequested)
{
ConsoleKeyInfo keyInfo = await this.ReadKeyAsync(cancellationToken);
ConsoleKeyInfo keyInfo = await ReadKeyAsync(cancellationToken);

// Do final position calculation after the key has been pressed
// because the window could have been resized before then
Expand Down Expand Up @@ -466,41 +487,6 @@ await this.powerShellContext.ExecuteCommand<PSObject>(
return null;
}

private async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken)
Copy link
Contributor

Choose a reason for hiding this comment

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

Really happy to get rid of this horrible piece of code :)

{
return await
Task.Factory.StartNew(
() =>
{
ConsoleKeyInfo keyInfo;

lock (this.readKeyLock)
{
if (cancellationToken.IsCancellationRequested)
{
throw new TaskCanceledException();
}
else if (this.bufferedKey.HasValue)
{
keyInfo = this.bufferedKey.Value;
this.bufferedKey = null;
}
else
{
keyInfo = Console.ReadKey(true);

if (cancellationToken.IsCancellationRequested)
{
this.bufferedKey = keyInfo;
throw new TaskCanceledException();
}
}
}

return keyInfo;
});
}

private int CalculateIndexFromCursor(
int promptStartCol,
int promptStartRow,
Expand Down
4 changes: 4 additions & 0 deletions src/PowerShellEditorServices/PowerShellEditorServices.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@
<Reference Include="System.Xml" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="UnixConsoleEcho" Version="0.1.0" />
</ItemGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard1.6' ">
<DefineConstants>$(DefineConstants);CoreCLR</DefineConstants>
</PropertyGroup>
Expand Down