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

Show breakpoint hint even if there are other line decorations #180379

Closed
wants to merge 1 commit into from

Conversation

joyceerhl
Copy link
Collaborator

@joyceerhl joyceerhl commented Apr 20, 2023

For #179725

This allows easy setting of breakpoints even if the current line already has a non-breakpoint decoration.

breakpoint-hint.mp4

One potential issue with this change is that it will cause the margin to resize if hovering on a line which already has a non-breakpoint decoration, which would cause a lot of visual flickering. We could address this by rendering a sentinel breakpoint to reserve the second lane for debug, which avoids the flickering at the expense of extra margin space even when the decoration has yet to be rendered. Another approach could be to debounce the margin resizing, which would reduce flickering at the expense of potential degraded performance.

@joyceerhl joyceerhl self-assigned this Apr 20, 2023
@joyceerhl joyceerhl requested a review from connor4312 April 20, 2023 04:31
@vscodenpa vscodenpa added this to the April 2023 milestone Apr 20, 2023
@connor4312
Copy link
Member

I'm not sure how I feel about this. Changing the layout of things on hover has accessibility concerns and also feels pretty disruptive. At least I think this should have a setting.

I wonder about an alternative:

  • For non-clickable decorations (i.e. decorations from extensions) hovering the decoration replaces it with the breakpoint symbol
  • For clickable decorations, the user has to right click on the gutter in order to set a breakpoint (same as the last stable)

@joyceerhl
Copy link
Collaborator Author

For non-clickable decorations (i.e. decorations from extensions) hovering the decoration replaces it with the breakpoint symbol

This seems reasonable, is the idea then that when you click to set the breakpoint, the second lane appears? If so I think this behavior is best controlled by the glyph margin rather than in the breakpoint contribution to avoid duplicating the logic for which lane to render the breakpoint in.

@connor4312
Copy link
Member

is the idea then that when you click to set the breakpoint, the second lane appears?

Yea, I think so

@joyceerhl joyceerhl marked this pull request as draft April 20, 2023 17:50
@joyceerhl
Copy link
Collaborator Author

👍 I'll revisit the problem of how to show breakpoint hints when there are already non-breakpoint decorations after we have a notion of clickable decorations (glyph widgets).

@joyceerhl joyceerhl modified the milestones: April 2023, May 2023 Apr 21, 2023
@joyceerhl joyceerhl closed this May 26, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants