-
Notifications
You must be signed in to change notification settings - Fork 234
Fix crash when untitled files are opened as PowerShell #815
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
Conversation
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.
LGTM!
If there's interest, I could try and improve the situation by changing the visitor to have a null PSScriptRoot mean ignore PSScriptRoot paths, or even try to make an untitled dir... (1st feels like it has merit, second maybe not).
How about making it the working directory? That should accomplish the same as the untitled drive, and would be most likely what someone is going for using $PSScriptRoot
in an untitled file.
I can't like @SeeminglyScience's comment... but if I could I 100% would |
BTW this scenario via
OTOH ISE doesn't crash. I dunno, I think the right answer might be what @rjmholt has implemented for the untitled scenario - just punt on analyzing dot sourced files using $PSScriptRoot. Seems like the more we try to support this scenario, the more we are setting folks up for failure later (no breakpoint support, etc). I view this scenario for folks just wanting to play around a little bit. |
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.
LGTM
@@ -648,7 +648,14 @@ private void ParseFileContents() | |||
.Select(ScriptFileMarker.FromParseError) | |||
.ToArray(); | |||
|
|||
//Get all dot sourced referenced files and store them | |||
// Untitled files have no directory | |||
// TODO: Make untitled files support dot-sourced references |
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.
I wouldn't bother but that is just my 2 cents.
I'm going to favour simplicity here. We can work out if we really want to support untitled files later I think. |
Fixes PowerShell/vscode-powershell#1649.
Finding dot-sourced references when we parse script files currently relies on the file having a directory, which is not true for untitled files.
This is a simple fix for the crash that occurs as a result.
If there's interest, I could try and improve the situation by changing the visitor to have a null PSScriptRoot mean ignore PSScriptRoot paths, or even try to make an untitled dir... (1st feels like it has merit, second maybe not).