Skip to content

fix: debug console variables output problem #127132

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
Merged
isidorn merged 1 commit into from
Aug 2, 2021
Merged

fix: debug console variables output problem #127132

isidorn merged 1 commit into from
Aug 2, 2021

Conversation

@erha19
Copy link

@erha19 erha19 commented Jun 25, 2021

This PR fixes #126967

@ghost
Copy link

ghost commented Jun 25, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@erha19 erha19 force-pushed the fix/debug-console-variables-ouput-problem branch from c98672f to 783e55c Compare June 25, 2021 07:37
@isidorn isidorn self-assigned this Jun 25, 2021
@isidorn
Copy link
Collaborator

isidorn commented Jun 25, 2021

Thank you for this PR. Unfortunately I will not have time this milestone to review this. I plan to look into this in July
Also assigning to @connor4312 as a reviewer since I think he did a PR which touches the Code your are changing.

@isidorn isidorn requested review from connor4312 and isidorn June 25, 2021 08:06
@erha19 erha19 force-pushed the fix/debug-console-variables-ouput-problem branch from 783e55c to a565d5e Compare July 5, 2021 02:54
connor4312
connor4312 requested changes Jul 6, 2021
View reviewed changes
Copy link
Member

@connor4312 connor4312 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think the idea of requesting variables before the output is dequeued is fine, but this also moves appendToRepl outside of the output queue which can cause logs to be displayed in incorrect order.

Also see: #126967 (comment)

@erha19
Copy link
Author

erha19 commented Jul 7, 2021

thanks @connor4312 , you are right, and I have been modified my code to put appendToRepl into the outputQueue.

@erha19 erha19 force-pushed the fix/debug-console-variables-ouput-problem branch from a565d5e to 3317ebf Compare July 7, 2021 02:04
connor4312
connor4312 reviewed Jul 7, 2021
View reviewed changes
@erha19 erha19 force-pushed the fix/debug-console-variables-ouput-problem branch from 3317ebf to d99e248 Compare July 8, 2021 01:55
@erha19
Copy link
Author

erha19 commented Jul 12, 2021

Hi @isidorn, the PR can be merged.

@erha19
Copy link
Author

erha19 commented Jul 26, 2021

@connor4312 hi, do you know who can review or merge this PR？

@isidorn
Copy link
Collaborator

isidorn commented Jul 26, 2021

Thanks for this PR! This week we are in the endgame which means we are not accepting any new code changes.
So I am assigning this to August so we potentially merge this start of next week.
Thanks!

@isidorn isidorn added this to the August 2021 milestone Jul 26, 2021
@isidorn isidorn added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jul 26, 2021
@isidorn
Copy link
Collaborator

isidorn commented Aug 2, 2021

As @connor4312 mentioned this PR is an improvement so let's merge it in.
@erha19 thanks a lot for your contribution 👏

@isidorn isidorn merged commit ca273fc into microsoft:main Aug 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@connor4312 connor4312 connor4312 approved these changes

@isidorn isidorn Awaiting requested review from isidorn

Assignees

@isidorn isidorn

Labels

debug Debug viewlet, configurations, breakpoints, adapter issues

Projects

None yet

Milestone

August 2021

Development

Successfully merging this pull request may close these issues.

Debug console

3 participants

@erha19 @isidorn @connor4312