Fix for vscode-powershell #837 callstack missing line info. #508
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR addresses two bugs. First the missing line info was due to use passing back null when we didn't have end line/col info. The protocol claims that should work. But when I changed the end line/col to reflect the same values as the start line/col (assuming the end values were null - not set) then the callstack started to display the line info. It is a bit weird but the very first frame in some cases contains a col other than 0. The second bug was that the call stack was getting duplicated. I implemented the new stackTraceRequest for paging stack frames back to VSCode and now that bug is fixed. I tried to implement StackFrame.PresentationHint == "subtle" for script outside the initial working directory. Do you have a better way to get the workspaceRoot path in the debugger? I had to cache the working dir when SetWorkingDirectory() gets called. And when you attach to a process, there isn't a way to set the "cwd". Finally, I ran into one bug I'll file later that has to do with the base of the call stack sometimes showing "<ScriptBlock> <No File> 1". If you click on that you get an error telling you <No File> can't be opened.
rkeithhill
commented
Jun 11, 2017
| @@ -43,7 +63,7 @@ public class StackFrameDetails | |||
| /// <summary> | |||
| /// Gets the line number of the script where the stack frame occurred. | |||
| /// </summary> | |||
| public int EndLineNumber { get; internal set; } | |||
| public int? EndLineNumber { get; internal set; } | |||
There was a problem hiding this comment.
Considering that Get-PSCallStack doesn't return any info about end line/col it seems like these values should be nullable (even though I'm setting them to startLine / 0 respectively).
There was a problem hiding this comment.
Yep, it's clearer if the contract is that those values may not be populated.
|
Looks like there are a couple missing XML comments causing the build to fail: |
daviwil
reviewed
Jun 11, 2017
| string moduleId = string.Empty; | ||
| var presentationHint = StackFramePresentationHint.Normal; | ||
|
|
||
| var invocationInfo = callStackFrameObject.Properties["InvocationInfo"].Value as InvocationInfo; |
There was a problem hiding this comment.
Have you tested this with remote debugging? Sometimes these objects come back as a weird serialized type that will cause issues.
daviwil
reviewed
Jun 11, 2017
| /// Gets the working directory path the PowerShell context was inititially set when the debugger launches. | ||
| /// This path is used to determine whether a script in the call stack is an "external" script. | ||
| /// </summary> | ||
| public string InitialWorkingDirectory { get; private set; } |
There was a problem hiding this comment.
This is fine for now, we'll come up with a better way to get the workspace path soon.
|
Thanks for implementing the new stack frame behavior too! Feel free to merge after the XML comment issues are fixed. |
daviwil
approved these changes
Jun 11, 2017
…area better. Handle possibilty of there not being a InvocationInfo property on a PSObject.
rkeithhill
force-pushed
the
rkeithhill/fix-callstack-missing-line-info
branch
from
0b77e6f to
792787b
Compare
|
Thanks man! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses two bugs. First the missing line info was due to use passing back null when we didn't have end line/col info. The protocol claims that should work. But when I changed the end line/col to reflect the same values as the start line/col (assuming the end values were null - not set) then the callstack started to display the line info. It is a bit weird but the very first frame in some cases contains a col other than 0.
The second bug was that the call stack was getting duplicated. I implemented the new stackTraceRequest for paging stack frames back to VSCode and now that bug is fixed.
I tried to implement StackFrame.PresentationHint == "subtle" for script outside the initial working directory. Do you have a better way to get the workspaceRoot path in the debugger? I had to cache the working dir when SetWorkingDirectory() gets called. And when you attach to a process, there isn't a way to set the "cwd".
Finally, I ran into one bug I'll file later that has to do with the base of the call stack sometimes showing " 1". If you click on that you get an error telling you can't be opened.