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.

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

Inline action for "edit condition" action in Breakpoints view #113805

Closed
isidorn opened this issue Jan 5, 2021 · 12 comments
Closed

Inline action for "edit condition" action in Breakpoints view #113805

isidorn opened this issue Jan 5, 2021 · 12 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Jan 5, 2021

As @bpasero suggested we could use an inline action for the "Edit Condition" action in the breakpoints view.
This inline action would appear on hover for exception breakpoints, function breakpoints, data breakpoints.

Currently we only allow to edit conitions once debugging is started and we know what debug adapter we are using. And we only show these actions if the debug adapter supports the corresponding capabilites. We might also want to consider to show this actions also when not debugging.

fyi @connor4312 @weinand
@misolori for icon ideas

@isidorn isidorn added debug Debug viewlet, configurations, breakpoints, adapter issues under-discussion Issue is under discussion for relevance, priority, approach labels Jan 5, 2021
@isidorn isidorn self-assigned this Jan 5, 2021
@weinand weinand added this to the On Deck milestone Jan 5, 2021
@weinand
Copy link
Contributor

weinand commented Jan 5, 2021

Yes, enabling to edit the condition outside of a debug session would be helpful because it makes the feature more discoverable. In addition it better aligns exception breakpoints with source breakpoints which already support 7/24 condition editing.

(Clarification: exception breakpoints are provided by a DA on "first contact" and then they are shown as long as the same DA is used. So condition editing should only be enabled if the DA has returned the corresponding capabilities on first contact and then this enablement is persisted and can be used outside of a debug session)

And using inline actions is another thing to try (but only after implementing the 7/24 condition editing).

@weinand weinand added the feature-request Request for new features or functionality label Jan 5, 2021
@isidorn
Copy link
Contributor Author

isidorn commented Jan 5, 2021

Makes sense, let me assign to Jaunary to investigate more into this.

@isidorn isidorn modified the milestones: On Deck, January 2021 Jan 5, 2021
@miguelsolorio
Copy link
Contributor

For icon, i'd suggest using our standard edit icon:

image

@isidorn
Copy link
Contributor Author

isidorn commented Jan 8, 2021

@misolori makes good sense, thanks!

@isidorn
Copy link
Contributor Author

isidorn commented Jan 13, 2021

I just verified that we actually already have 24/7 condition editing, seems like I was thinking about when coding this up before Christmas break 👏
It just took me some time to figure out that I need the js-debug-nightly for the condition editing. @connor4312 do we plan to release this in the js-debug regular release?

Since we have the 24/7 condition editing, I will now look to add the inline action using the Icon Miguel proposed.

@weinand
Copy link
Contributor

weinand commented Jan 13, 2021

@isidorn yes indeed, Mock Debug has 24/7 condition editing.

@isidorn
Copy link
Contributor Author

isidorn commented Jan 13, 2021

Here's how this looks in practice. I will render this inline action for all breakpoints since all breakpoints support conditions now.

Screenshot 2021-01-13 at 16 49 50

@isidorn isidorn added on-testplan and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jan 14, 2021
lszomoru pushed a commit that referenced this issue Jan 15, 2021
…unction breakpoints. Allow to edit conditions for function breakpoints

fixes #113805
fixes #3646
@yannickowow
Copy link
Contributor

Hi,
I don't know if you were facing it yet, but, when debugging, I cannot use Edit Hit Count or Edit Condition from actions menu, but I can use pencil icon to edit condition.
When not debugging, it seems to work fine and usage is intuitive.
My debugger supports conditionalBreakpoints, hitConditionalBreakpoints and functionBreakpoints
See:
image
image
image

Regards.

@isidorn
Copy link
Contributor Author

isidorn commented Jan 15, 2021

@yannickowow thanks a lot for trying it out and for providing feedback. I have just pushed a fix for this issue which you found. Thanks!

@yannickowow
Copy link
Contributor

yannickowow commented Jan 25, 2021

Hi
I did found a bug associated to "Edit Condition" for DataBreakpoints. It shows "Edit Condition" action but it does not open "Edit" view. Edit icon is not displayed as well.
It works fine actually with FunctionBreakpoints.

Thanks!

@isidorn
Copy link
Contributor Author

isidorn commented Jan 25, 2021

@yannickowow that's a good catch. However for now we simply did not implement this, I was not aware of debuggers that would want condition on data breakpoints. Though I see that it makes good sense.
For this I have created a new feature request and I can look into it if there is demand
#114903

@yannickowow
Copy link
Contributor

Thanks.
Since DataBreakpoints implementation in DebugAdapterProtocol supports the same arguments (ie Hit Count and Condition), it would be great if VS Code can show an unified attribute for this.

@isidorn isidorn added the on-release-notes Issue/pull request mentioned in release notes label Jan 29, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Feb 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan
Projects
None yet
Development

No branches or pull requests

6 participants
@weinand @isidorn @miguelsolorio @yannickowow and others