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

Different breakpoint marker for unverified breakpoint before debug session start #135105

Open
polinasok opened this issue Oct 14, 2021 · 9 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues under-discussion Issue is under discussion for relevance, priority, approach ux User experience issues
Milestone

Comments

@polinasok
Copy link

polinasok commented Oct 14, 2021

If a user sets a breakpoint before the debug session starts, it will look like a successful breakpoint in red. Then once the debug session starts, it might get registered and stay red or it might fail to register and turn grey. Unless the program stops on entry, gets paused, etc, the user might never even notice that the breakpoint failed to register and wonder why it never got hit.

Displaying the preliminary breakpoint as registered is misleading. Users do not always realize that the validation will happen later and that they need to check that it succeeded (example, example). I propose that a different marker is used for such breakpoints with a hover message that it will be validated once the debug session is launched.

@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues under-discussion Issue is under discussion for relevance, priority, approach labels Oct 14, 2021
@roblourens
Copy link
Member

I don't think that we should show the breakpoint as gray - that tells me that there was a problem that I need to fix. As far as we know, the breakpoint is fine and should look like a real breakpoint. I think it comes down to learning how debugging works. I might be open to basically adding a new state with a different icon or ux indicator. So the breakpoint states would be

  • unknown (new state)
  • verified in a debug session (red)
  • unverified in a debug session (gray outline)
  • disabled (gray solid)

@roblourens roblourens added the ux User experience issues label Oct 14, 2021
@miguelsolorio
Copy link
Contributor

miguelsolorio commented Oct 14, 2021

I'd be in favor of this as I had similar feelings about not being able to hit certain breakpoints due to my lack of understanding with debugging. And to add some pictures to words:

CleanShot 2021-10-14 at 13 16 06@2x

@ 100%
CleanShot 2021-10-14 at 13 16 20@2x

@roblourens
Copy link
Member

Thanks @misolori, I could find something I like. I want to understand the problems a little more though, @polinasok in the first one there is some confusion about how stopOnEntry works, and I don't really follow whether a change like this would help that situation. The second one sounds like it's about function breakpoints which would be different. What exactly are you expecting to see? Maybe this is just something that people learn about as they use the debugger?

@polinasok
Copy link
Author

As I indicated in my original summary, I am also in favor of an entirely different marker for the unknown state. I agree that unverified/unregistered marker should not be used. And this should apply to markers for other types of breakpoints as well (e.g. red vs some other kind of triangle for function breakpoints).

In the stopOnEntry issue that I linked a user wanted to use stopOnEntry to either stop at the beginning of main function or to stop at the beginning of the runtime initialization code and see source, stack and be able to step. Neither of those happened. So they asked what was the point of this feature if you can also set breakpoints just as successfully before the session and the target start running. The user didn't realize that the difference between setting breakpoints before the session and during the session on entry was that in the former case you won't actually know if the breakpoints registered successfully while on entry you can set them with confidence.

In the second issue, it was pointed out that sometimes users set breakpoints before the session start. They don't know that the breakpoints didn't stick. Then start debugging. The breakpoints don't register, and the program exits. This happens fast and once the session exits, the breakpoint view no longer shows the unverified markers and error messages. Without digging through internal opt-in dap traffic logs , the user has no way of telling what happened. The issue suggests that we explicitly log a message for each failed breakpoint to the Debug Console to make diagnosing this case easier. However, that could mean logging many noisy messages about breakpoints that do not even apply to this particular session. So the conclusion was to start with function breakpoints only as those are much easier to get wrong since the user types the function name manually, instead of clicking a line in the source code, where they can generally see that a breakpoint would stick.

By having an informative distinct marker you will aid the users in learning about the debugger and how debugging works. They will study the hover text. They will have a mental image that something does change about the breakpoints before and after session start. And when things do go wrong, it will be much easier for them to look in the right place for diagnostics.

@roblourens
Copy link
Member

Thanks for the explanation. Will think about this. I wonder whether @connor4312 has seen users confused about this before?

I also don't want to confuse people more by changing the breakpoint icon. It could make people think that there is something wrong and they need to do something to set the breakpoint correctly. Another option could be adding a hover without the new icon, but I don't know that would really help. We would also have to solve this for conditional breakpoint and logpoint icons.

@connor4312
Copy link
Member

has seen users confused about this before?

Very rarely, I think I've seen one or two from users who set breakpoints and then run their program in an ordinary terminal and expect them to bind. That's pretty easy to fix with some guidance. Though Andre might have seen more since he triages the debug label.

While I'm not at all opposed to this, I also haven't seen a big need for it.

@roblourens roblourens added this to the Backlog milestone Oct 18, 2021
@weinand
Copy link
Contributor

weinand commented Oct 19, 2021

First a clarification. @polinasok you said:

I propose that a different marker is used for such breakpoints with a hover message that it will be validated once the debug session is launched.

Please note that the breakpoint validation is dynamic and not necessarily finished when a debug session starts. If you set a breakpoint in the activate method of a "hello world" extension, the breakpoint turns from a red disk to a grey ring as soon as the debug session starts, indicating that it is "unbound" (because the code has not yet been loaded or seen). Only if the extension is activated, will the breakpoint turn into the red disk to indicate that it became bound.


And now my take of the general issue:

VS Code has this "problem" from its inception (more than six years ago), but based on my experience I don't think that introducing another color/shape for the "unknown" state of a breakpoint would help users understanding why a breakpoint is not hit (and the fact that I'm not aware of a single issue filed about this problem seems to support this claim).

I'm against introducing another icon color or shape for the "unknown" state because that would change the default breakpoint UI too drastically.
If we still come to an agreement that a UI change would be helpful, then we should try to change the icon of the "verified" state in a small way, e.g. by adding a small check mark in one corner of the red disk.

But again, I still think that introducing a new icon is not worth the trouble...

@polinasok
Copy link
Author

I like the idea of changing the verified icon instead. And I think adding a more informative hover could be helpful too.
I don't think this problem is so confusing that users would be filing many issues for it. Like everybody pointed out, it's not impossible to figure out with use. It's just a bump on the road, not a big ditch that requires calling roadside assistance. If it's not a big investment to improve, then why not? I think people generally consider debugging a high friction activity, so smoothing out rough edges is always helpful.

@hyangah
Copy link

hyangah commented Jan 7, 2022

Happy new year! Thanks for taking the time to understand and come up with ideas.

We (vscode-go) got another user report where breakpoints couldn't set because, this time, setting breakpoints on non-statement line isn't possible in Go. I recall we've seen reports many times - usually when Go users try to debug a binary that has different file path info in dwarf (e.g. because they built the binary in different file systems/containers, or there were symlinks, etc). Often users start reporting like "debugger doesn't work, it doesn't stop at breakpoints" as part of other general debugging issue report.

Debug adapter already provides useful info about why a breakpoint couldn't set as part of the set breakpoint response, but I see all the info gets removed as soon as a session ends.

Can't the info stick around after a debug session ends? I am not sure if a different icon is sufficient enough to help users without the detailed info.

We are also considering to report the explanation on non-verifiable breakpoints as OutputEvents. But having breakpoints set for code that's not part of compiled debug target is normal (e.g. multiple debug sessions, debugging a small test code while working in large code base, ...), we are not sure if that's the best UX.

Somewhat related - when working with multiple debug sessions, breakpoint viewlet shows a union of the state. And the hover message for failure from one session can shadow failure message from another session. I wonder if it's possible to have a separate state for each session (but have no good idea on how that will look).

EDIT: It looks like we get related issues regularly (today I got golang/vscode-go#2009). If there is a better way to communicate with users about unverified breakpoints, I am all ears.... 👂🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues under-discussion Issue is under discussion for relevance, priority, approach ux User experience issues
Projects
None yet
Development

No branches or pull requests

7 participants