Skip to content

Fix several bugs in the debugger #1975

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

Merged
merged 5 commits into from
Dec 20, 2022
Merged

Fix several bugs in the debugger #1975

merged 5 commits into from
Dec 20, 2022

Conversation

andyleejordan
Copy link
Member

@andyleejordan andyleejordan commented Dec 17, 2022

@andyleejordan andyleejordan force-pushed the andschwa/debugger branch 3 times, most recently from c53be4f to 817a3f6 Compare December 19, 2022 21:03
@andyleejordan andyleejordan changed the title Fix some bugs in the debugger Fix several bugs in the debugger Dec 19, 2022
@andyleejordan andyleejordan marked this pull request as ready for review December 19, 2022 21:54
@andyleejordan andyleejordan requested a review from a team December 19, 2022 21:54
@andyleejordan andyleejordan force-pushed the andschwa/debugger branch 2 times, most recently from d751d6e to 0ef094e Compare December 19, 2022 22:06
@andyleejordan
Copy link
Member Author

@JustinGrote I accidentally implemented the same idea you had in #1688 to fix another bug, so I borrowed your test. Can you pull this branch and give it a whack, see how it does? Things seem good to me.

@andyleejordan andyleejordan force-pushed the andschwa/debugger branch 2 times, most recently from 1e24ba0 to 36af0ee Compare December 20, 2022 19:06
andyleejordan and others added 3 commits December 20, 2022 11:24
Which fixes our test that gets the properties of a process object, since
the value for `ExitCode` fails (as the process hasn't ended), we now
handle that a bit more gracefully and can get all 130 properties.

This also seemed to fix the overall bug where lots of expected
properties failed to show up. While investigating, I also found many
properties duplicated, seemingly due to being retrieved both off the
PSObject and off the .NET object, so now when we do the latter we check
(by name) that it hasn't already been added.
This now works fine since the expansion takes place on the pipeline
thread.
So that it doesn't deadlock with the pipeline thread.
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! Well done, this is gonna be a huge boost to UX ❤️

@andyleejordan andyleejordan merged commit c0ff2ce into main Dec 20, 2022
@andyleejordan andyleejordan deleted the andschwa/debugger branch December 20, 2022 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal error when expanding auto variable holding Linq.GroupBy result.
3 participants