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.

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

Jump to bottom

When the built-in $null was watched its value was incorrect #2097

Merged
merged 1 commit into from
Dec 7, 2023
Merged

When the built-in $null was watched its value was incorrect #2097

merged 1 commit into from
Dec 7, 2023

Conversation

andyleejordan
Copy link
Member

No idea why its value was "", but it was, so we special case it and return $null just like you get from watched variables that were assigned the built-in $null.

Resolves PowerShell/vscode-powershell#4816.

@andyleejordan andyleejordan requested a review from a team November 10, 2023 23:23
@andyleejordan
Copy link
Member Author

image

Tada!

@thegraffix
Copy link

Just curious - does this PR address other cases in which $null -eq $var == $true in the WATCH list as well? The built-in $null: "" wasn't the only case in which a null value was represented as an empty string "" e.g. an unitilialized variable should be shown as $VarThatDoesNotExist: $null and not $VarThatDoesNotExist: "".

@andyleejordan
Copy link
Member Author

Hm, I hadn't tried $VarThatDoesNotExist and it wasn't in the issue, so probably not. I can take a look. Anything else you know about?

@thegraffix
Copy link

Ha no problem - $VarThatDoesNotExist is actually mentioned in the first bullet point of the issue and also in the screenshot. When debugging a PowerShell script, I would except all cases where $null -eq XYZ == $true would show XYZ: $null in the WATCH pane -- rather than sometimes XYZ: $null and other times XYZ: "". There's some MS Learn documentation on some of the subtleties of evaluating $null.

@andyleejordan andyleejordan force-pushed the andyleejordan/handle-watched-null branch from 203877b to dfb8199 Compare December 7, 2023 01:57
@andyleejordan
Copy link
Member Author

andyleejordan commented Dec 7, 2023 •
edited
Loading

Ok I took another look at this and now not only do I have answer as to why $null was being evaluated to "", I fixed the root cause 🥳

image

@andyleejordan
When the built-in $null was watched its value was incorrect
3dd91d5 
This fixes the evaluation of `$null` itself and variables that don't
exist such that they display correctly in the watched variables pane of
the debugger. As noted, the issue was that we were doing a string join
even for a single (potentially null) value.

This also fixes the excess quotes bug around a single value.
@andyleejordan andyleejordan force-pushed the andyleejordan/handle-watched-null branch from dfb8199 to 3dd91d5 Compare December 7, 2023 02:04
SeeminglyScience
SeeminglyScience approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM!

@andyleejordan andyleejordan added this pull request to the merge queue Dec 7, 2023
Merged via the queue into main with commit 25122e9 Dec 7, 2023
13 checks passed
@andyleejordan andyleejordan deleted the andyleejordan/handle-watched-null branch December 7, 2023 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeeminglyScience SeeminglyScience SeeminglyScience approved these changes

Assignees
No one assigned
Labels
Area-Debugging Issue-Bug A bug to squash.
Projects
Flying Fox
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When the built-in $null was watched its value was incorrect
3 participants
@andyleejordan @thegraffix @SeeminglyScience