Skip to content

Change debug launch handler to treat null/empty cwd to not change dir #694

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
Jul 4, 2018
Merged

Change debug launch handler to treat null/empty cwd to not change dir #694

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f48cc1c
Change debug launch handler to treat null/empty cwd to not change dir
rkeithhill Jul 2, 2018
8ee61a3
Address PR feedback, reorg to not execute working dir code unless used
rkeithhill Jul 3, 2018
db17acb
Get rid of unnecessary pragma warning disable
rkeithhill Jul 4, 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
66 changes: 35 additions & 31 deletions src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,41 +254,38 @@ protected async Task HandleLaunchRequest(
{
this.RegisterEventHandlers();

// Set the working directory for the PowerShell runspace to the cwd passed in via launch.json.
string workingDir = launchParams.Cwd;

// Assuming we have a specified working dir, unescape the path and verify it.
if (!string.IsNullOrEmpty(workingDir))
// Determine whether or not the working directory should be set in the PowerShellContext.
if ((this.editorSession.PowerShellContext.CurrentRunspace.Location == RunspaceLocation.Local) &&
!this.editorSession.DebugService.IsDebuggerStopped)
{
workingDir = PowerShellContext.UnescapePath(workingDir);
try
// Get the working directory that was passed via the debug config
// (either via launch.json or generated via no-config debug).
string workingDir = launchParams.Cwd;

// Assuming we have a non-empty/null working dir, unescape the path and verify
// the path exists and is a directory.
if (!string.IsNullOrEmpty(workingDir))
{
if ((File.GetAttributes(workingDir) & FileAttributes.Directory) != FileAttributes.Directory)
workingDir = PowerShellContext.UnescapePath(workingDir);
try
{
workingDir = Path.GetDirectoryName(workingDir);
if ((File.GetAttributes(workingDir) & FileAttributes.Directory) != FileAttributes.Directory)
Copy link
Member

Choose a reason for hiding this comment

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

what is this magical syntax?
File.GetAttributes(workingDir) & FileAttributes.Directory

Copy link
Contributor

Choose a reason for hiding this comment

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

Enums as flags. Attributes are usually modelled as flag enums by C# to make them more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

(See here)

Copy link
Contributor

Choose a reason for hiding this comment

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

I usually see the check as != 0, but another recommendation I've seen is to use HasFlag():

if (File.GetAttributes(workingDir).HasFlag(FileAttributes.Directory))
{
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently HasFlag() was added in .NET 4.

{
workingDir = Path.GetDirectoryName(workingDir);
}
}
catch (Exception ex)
{
workingDir = null;
Logger.Write(
LogLevel.Error,
$"The specified 'cwd' path is invalid: '{launchParams.Cwd}'. Error: {ex.Message}");
}
}
catch (Exception ex)
{
Logger.Write(LogLevel.Error, "cwd path is invalid: " + ex.Message);

workingDir = null;
}
}

// A null or empty string value for CWD is an indicator to NOT change the working directory
// but only if we are debugging in the integrated console where there is an existing cwd.
// However, if we are running in a temporary integrated console, there is no pre-existing
// cwd, so fall through to the "else if: where we set a working directory.
if (string.IsNullOrEmpty(workingDir) && !launchParams.CreateTemporaryIntegratedConsole)
{
Logger.Write(LogLevel.Verbose, "Launch config requested working dir not be changed/set");
}
else if (this.editorSession.PowerShellContext.CurrentRunspace.Location == RunspaceLocation.Local &&
!this.editorSession.DebugService.IsDebuggerStopped)
{
// If we have no working dir by this point, pick some reasonable default.
if (string.IsNullOrEmpty(workingDir))
// If we have no working dir by this point and we are running in a temp console,
// pick some reasonable default.
if (string.IsNullOrEmpty(workingDir) && launchParams.CreateTemporaryIntegratedConsole)
{
#if CoreCLR
//TODO: RKH 2018-06-26 .NET standard 2.0 has added Environment.CurrentDirectory - let's use it.
Copy link
Member

Choose a reason for hiding this comment

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

soon.... :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.youtube.com/watch?v=39S3dLDEbYs

Expand All @@ -298,8 +295,15 @@ protected async Task HandleLaunchRequest(
#endif
}

await editorSession.PowerShellContext.SetWorkingDirectory(workingDir, isPathAlreadyEscaped: false);
Logger.Write(LogLevel.Verbose, "Working dir set to: " + workingDir);
// At this point, we will either have a working dir that should be set to cwd in
// the PowerShellContext or the user has requested (via an empty/null cwd) that
// the working dir should not be changed.
if (!string.IsNullOrEmpty(workingDir))
{
await editorSession.PowerShellContext.SetWorkingDirectory(workingDir, isPathAlreadyEscaped: false);
}

Logger.Write(LogLevel.Verbose, $"Working dir " + (string.IsNullOrEmpty(workingDir) ? "not set." : $"set to '{workingDir}'"));
}

// Prepare arguments to the script - if specified
Expand Down