Skip to content

Support for DataBreakpointAccessType condition #117835

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
merged 2 commits into from
Mar 17, 2021
Merged

Support for DataBreakpointAccessType condition #117835

merged 2 commits into from
Mar 17, 2021

Conversation

yannickowow
Copy link
Contributor

@yannickowow yannickowow commented Feb 27, 2021 •
edited
Loading

This PR fixes #113922

Hi,
Here you will find a draft PR to support DataBreakpoint access type.
When right-clicking on a DataBreakpoint variable, you can by this way select an action based on DataBreakpointAccessTypes[] response.
By default, an empty array assumes Break When Value Changes (it was the current behaviour).

Here you will find a record about this new behaviour:

sampleWorkspace-1614455088109.mp4

For a better experience, I would suggest here a radiobutton instead of checkbox... Any ideas ?

I did update breakpointTests as well to ensure accessType is correctly set.

Thanks in advance !

@ghost
Copy link

ghost commented Feb 27, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@yannickowow yannickowow marked this pull request as ready for review March 5, 2021 19:35
@isidorn isidorn mentioned this pull request Mar 11, 2021
Closed
@isidorn isidorn assigned isidorn and unassigned isidorn Mar 11, 2021
@isidorn isidorn added this to the March 2021 milestone Mar 16, 2021
@isidorn
Copy link
Collaborator

isidorn commented Mar 16, 2021

@yannickowow thanks for your PR and sorry for the slow response.
I glanced over the PR and it looks good overall. First I would like to ask you do the following

  1. Can you please merge latest from main and resolve conflicts
  2. The updateDataBreakpoint work can be split up from the inital support of all accessTypes. Due to that it would be great if in this PR you could only leave the support for all accessTypes and all the updateDataBreakpoint work to be put in another PR. Once we merge this PR and once we gain enough user feedback we can consider merging in the other updateDataBreakpoint one

I hope my suggestion make sense and I wish you a nice day

@yannickowow
Copy link
Contributor Author

If I understood correctly, you are considering actions to support accessTypes but not updating them in Breakpoints view (used by updateDataBreakpoint) ?
If so, it is done. 👍
I did remove actions associated to DataBreakpoints in the Breakpoints view.

Regards.

isidorn
isidorn requested changes Mar 17, 2021
View reviewed changes
@isidorn
Copy link
Collaborator

isidorn commented Mar 17, 2021

Great work, thanks again for your PR. I have left some comments in the code, once you tackle that let me know and I can do another review.

Since we will probably merge this in this milestone it might be cool to write a test plan item so we cover this. Here's a good template #79799 @yannickowow if you have time maybe you could write a test plan item and ping me @isidorn on it
Though it might be cool if mock-debug supported these new changes so we could write a test plan item using mock debug @weinand

@yannickowow yannickowow requested a review from isidorn March 17, 2021 17:27
@yannickowow yannickowow mentioned this pull request Mar 17, 2021
Closed
2 tasks
@isidorn
Copy link
Collaborator

isidorn commented Mar 17, 2021

Fantastic work. Merging in, thanks a lot for your contribution ☀️

@isidorn isidorn merged commit 42af2c3 into microsoft:main Mar 17, 2021
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues on-testplan labels Mar 22, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@isidorn isidorn Awaiting requested review from isidorn

Assignees

@weinand weinand

@isidorn isidorn

Labels
debug Debug viewlet, configurations, breakpoints, adapter issues on-testplan
Projects
None yet
Milestone
March 2021
Development

Successfully merging this pull request may close these issues.

Support for editing 'accessType' property of data breakpoints
3 participants
@yannickowow @isidorn @weinand