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

[wasm][debugger] Adding support to receive evaluationOptions from dap. #71464

Merged
merged 8 commits into from
Dec 30, 2022
Merged

[wasm][debugger] Adding support to receive evaluationOptions from dap. #71464

merged 8 commits into from
Dec 30, 2022

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Jun 29, 2022 •
edited
Loading

To support receiving EvaluationOptions from dap.

Then we will be able to auto evaluate properties or not based on what is defined on VS settings.
https://user-images.githubusercontent.com/4503299/176544439-3ae898e5-4eb6-4883-bd57-a277fb19ff30.mp4

Depends on:
microsoft/vscode-js-debug#1323

With this check disabled:
image
image

With this check enabled:
image
image

Fixes #49144

@ghost ghost assigned thaystg Jun 29, 2022
@ghost
Copy link

ghost commented Jun 29, 2022

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

To support receiving EvaluationOptions from dap.

Then we will be able to auto evaluate properties or not based on what is defined on VS settings.
https://user-images.githubusercontent.com/4503299/176544439-3ae898e5-4eb6-4883-bd57-a277fb19ff30.mp4

Depends on:
microsoft/vscode-js-debug#1323

Author: thaystg
Assignees: thaystg
Labels:

area-Debugger-mono
Milestone: -

@radical
Copy link
Member

radical commented Jun 29, 2022

Can we add a test where it sends the setting, and we check if properties are being auto expanded or not?

@thaystg
Copy link
Member Author

thaystg commented Jun 30, 2022

For sure we can do it, I just didn't add it yet, because I'm waiting if Connor will approve the changes the way it is implemented on the PR on vscode-js-debug repo.

@radical radical mentioned this pull request Aug 11, 2022
Closed
@thaystg
Copy link
Member Author

thaystg commented Aug 11, 2022

As I discussed with Larry we would like to merge it on our side and hope to get the merge on VS side without any change.
I'll add the tests in the PR and we can merge both of them before RC1.

@thaystg thaystg marked this pull request as ready for review August 18, 2022 18:37
@lewing
Copy link
Member

lewing commented Sep 12, 2022

should we land this in main now?

@marek-safar
Copy link
Contributor

@thaystg what's the plan with this PR?

@thaystg
Copy link
Member Author

thaystg commented Nov 11, 2022

Still waiting for the approval of the PR on vscode-js-debug repo: microsoft/vscode-js-debug#1323

@thaystg
Merge remote-tracking branch 'origin/main' into thays_support_Evaluat…
2f93cc4 
…ionOptions

# Conflicts:
#	src/mono/wasm/debugger/BrowserDebugProxy/MemberObjectsExplorer.cs
#	src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
@build-analysis build-analysis bot mentioned this pull request Dec 28, 2022
Closed
@thaystg thaystg merged commit c71ad8c into dotnet:main Dec 30, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radical radical radical left review comments

@lewing lewing lewing approved these changes

@ilonatommy ilonatommy ilonatommy approved these changes

Assignees

@thaystg thaystg

Labels
area-Debugger-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[wasm][debugger] Properties in the watch window show "𝑓 <Property>" instead of the value of the property
5 participants
@thaystg @radical @lewing @marek-safar @ilonatommy