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

Debugger: Put "Run To Cursor" Into Breakpoint Context Menu #123872

Closed
hediet opened this issue May 14, 2021 · 12 comments · Fixed by #130039
Closed

Debugger: Put "Run To Cursor" Into Breakpoint Context Menu #123872

hediet opened this issue May 14, 2021 · 12 comments · Fixed by #130039
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 verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@hediet
Copy link
Member

hediet commented May 14, 2021

Currently, "Run To Cursor" is in a very long context menu:

image

I think it would make sense to add "Run To Cursor" to the context menu for adding breakpoints:

image

This new entry should be called "Run To Line".

Pro:

  • That context menu is already strictly debugging related
  • If a user does not know about "Run To Cursor" and manually places a breakpoint to the target location, they might discover this functionality in that context menu

Contra:

  • "Run To Line" is not a breakpoint
@hediet hediet added the debug Debug viewlet, configurations, breakpoints, adapter issues label May 14, 2021
@isidorn isidorn added the under-discussion Issue is under discussion for relevance, priority, approach label May 14, 2021
@isidorn
Copy link
Contributor

isidorn commented May 14, 2021

@hediet thanks for this suggestion, however please note that we show the Run To Cursor and Add Inline Breakpoint in the editor context menu only while a user is debugging. So it is not always there.

I understand you are always in debugging mode and I think we should improve the experience for "Always Debugging" mode. For this I have created #123873

For now I would probably not change this to not anger some users that are used to it. Especially since I hope we will have customisable context menus in the future. However I am open for ideas.

fyi @connor4312 @weinand

@hediet
Copy link
Member Author

hediet commented May 14, 2021

however please note that we show the Run To Cursor and Add Inline Breakpoint in the editor context menu only while a user is debugging. So it is not always there.

I wouldn't neccessarily remove it from the editor context menu, but just add it to the breakpoint context menu to improve its accessibility. My idea is not to shorten the editor context menu, but to make that feature easier to use.

I use that feature quite often (and should probably assign a keybinding to it), but the very long context menu makes it hard to quickly execute that action (especially when my focus is on debugging the problem, not finding that context menu entry).

The Chrome dev tools have an even more efficient approach - while debugging, you can simply Ctrl-Click on a statement to continue to it. That does not play very well with VS Codes "Go To Definition" though.

@isidorn
Copy link
Contributor

isidorn commented May 14, 2021

@hediet oh you would simply add it to the other context menu.
Yeah this could work for me.

Let's see what @connor4312 and @weinand think

@connor4312
Copy link
Member

Adding it to the breakpoint (gutter) context menu sounds good to me. Honestly I didn't even know that context menu entry existed, I never right click in the editor 😛

@isidorn
Copy link
Contributor

isidorn commented May 14, 2021

If we were to add it, I would put a separator after the breakpoint actions. And have the "Run to Line" at the bottom.
Let's assign to this milestone and if Andre does not object I can add it next week.

@isidorn isidorn added this to the May 2021 milestone May 14, 2021
@hediet
Copy link
Member Author

hediet commented May 14, 2021

I can also try to implement it if we go for it! Shouldn't be that hard.

@connor4312
Copy link
Member

Code pointer:

private getContextMenuActions(breakpoints: ReadonlyArray<IBreakpoint>, uri: URI, lineNumber: number, column?: number): IAction[] {

@hediet
Copy link
Member Author

hediet commented May 17, 2021

Do you think the logic in RunToCursorAction should be moved to IDebugService and its implementation (and there renamed to RunToPosition)?
Then there could be both a RunToCursorAction and RunToLineAction.

@isidorn
Copy link
Contributor

isidorn commented May 17, 2021

@hediet that might work. However I try to keep the IDebugService as thin as possible. So I would personally just use the commandService.executeCommand to execute one command from the other.
Though feel free to move the logic to the IDebugService if you prefer that

@isidorn
Copy link
Contributor

isidorn commented May 25, 2021

Leaving this assigned to you @hediet as you plan to do a PR.
Feel free to ping me on a PR for a review or let me know if I can help in any way.

@isidorn isidorn removed their assignment May 25, 2021
@hediet hediet modified the milestones: May 2021, June 2021 Jun 2, 2021
@hediet hediet added feature-request Request for new features or functionality and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jun 2, 2021
@hediet hediet modified the milestones: June 2021, August 2021, July 2021 Jun 28, 2021
@heartacker
Copy link
Contributor

consider add a decorate for user to click run to here, like VS2019?
image

@isidorn isidorn added the verification-needed Verification of issue is requested label Aug 3, 2021
@lramos15 lramos15 added the verified Verification succeeded label Aug 24, 2021
@lramos15
Copy link
Member

There's a bit of breakpoint flashing when you do run to line but the context menu appears to work so marking this as verified

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 verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@heartacker @isidorn @connor4312 @hediet @lramos15 and others