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

Breakpoints on particular column #14784

Closed
roblourens opened this issue Nov 1, 2016 · 19 comments
Closed

Breakpoints on particular column #14784

roblourens opened this issue Nov 1, 2016 · 19 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Milestone

Comments

@roblourens
Copy link
Member

roblourens commented Nov 1, 2016

There might be an issue for this already but I can't find one, so here's a general feature request for discussion.

It would be very useful to be able to set a breakpoint on a particular column of a line. It's useful for debugging promises -

asyncFn().then(() => foo())
    .then(() => bar())
    .then(() => something())

or minified code -

a();b();c();

especially since we don't have any way to pretty-print scripts while debugging them. As is, there's no way to break inside the thens or on expressions that are packed on the same line without reformatting your code or setting breakpoints in other places.

This is handled by the protocol already but needs UI work.

  • How do we show which part of the line the breakpoint is associated with?
    • VS and F12 highlight the actual statement for the bp. I'm not sure that works for vscode, because we'll only know the column position and won't necessarily have the language knowledge to know the whole span in question.
    • Show a decoration inline if column != 0? Only visible when hovering over the bp in the margin?
    • Then these will often show up when setting a bp with F9, just because the cursor is somewhere else in the line, even if it's the same as a bp at column 0
  • How do we show multiple breakpoints on one line?
  • Need a 'add breakpoint' as a context menu option on the editor
  • Setting a breakpoint by F9 needs to respect the column position of the cursor

And a cheaper way to experiment with this could be to change the "Run to cursor" option to respect the column.

@roblourens roblourens added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Nov 1, 2016
@weinand weinand added this to the January 2017 milestone Nov 14, 2016
@isidorn
Copy link
Contributor

isidorn commented Dec 27, 2016

Agree that this is a nice feature to have.

As for managing breakpoints I propose the following:

  • Run to cursor, F9 and 'add conditional breakpoint' via command pallete now respect the column position of the curosr
  • Clicking on the glyph margin is unchanged. If there are multiple breakpoints on a line, clicking on the glyph removes them all
  • We do not add anything to the context menu of the editor since that is sensitive area and it is hard for us to detect if a user is interested in adding breakpoints. This can be added later if the need arises.

As for representing them I think we have two options:

Put a decoration on the exact line / column location

This approach is being done by Chrome, more details can be found here
What I dislike about this approach for us is that our ugly red circle of breakpoints could appear on a middle of a line which could be confused for an error.

We could only use this in column decorations when there is > 1 breakpoint on that line. Otherwise just decorate the margin and on hover show the column location.

Always only show an icon in the glyph margin

  • we introduce a new icon if there are two or more breakpoints on a line, could look like a red circle behind a red circle
  • hovering over the margin would highlight the column location where breakpoints are set on the line

In both cases breakpoints view would nicely show the column for each breakpoint

isidorn added a commit that referenced this issue Dec 27, 2016
@roblourens
Copy link
Member Author

roblourens commented Jan 2, 2017

There's going to be a problem in the short term with this in Node/Chrome that I didn't see- from this bug, microsoft/vscode-node-debug2#71, I realized that if you set a bp towards the end of a line (after the last valid break location, I guess) Chrome will actually set it on the next line. Filed this on V8, then realized that there's a brand new protocol API to cover this exact case, getPossibleBreakpoints. However it only exists in Canary at the moment. Probably will be available in Node 8 or later in 7.x.

I filed microsoft/vscode-chrome-debug-core#144 on myself to use this API, and fall back on the old behavior when needed. I didn't realize they were working on this in Chrome devtools - thanks for posting the link. I can have this set up to light up in any runtime that supports that API.

@roblourens
Copy link
Member Author

I propose that we revisit this for Feb or March, when we'll have this API in Chrome and Node. In the meantime I'll have to ignore the column coming from vscode (or we can revert 604e21b)

@isidorn
Copy link
Contributor

isidorn commented Jan 11, 2017

@roblourens agree to push this out for Feb and March
Let's not revert the whole commit since the work there is good. I will simply not send the column on my side.

@roblourens
Copy link
Member Author

Sounds good, thanks.

@roblourens roblourens modified the milestones: Backlog, January 2017 Jan 11, 2017
@isidorn isidorn modified the milestones: February 2017, Backlog Jan 19, 2017
isidorn added a commit that referenced this issue Feb 2, 2017
@kieferrm kieferrm mentioned this issue Feb 6, 2017
38 tasks
@isidorn
Copy link
Contributor

isidorn commented Feb 8, 2017

Done. You can check it out in tomorrow's insiders.
There is a special add column breakpoint action and you can also use a context menu during a debug session

columnbps

@isidorn isidorn closed this as completed Feb 8, 2017
@isidorn isidorn mentioned this issue Feb 8, 2017
3 tasks
@isidorn
Copy link
Contributor

isidorn commented Feb 15, 2017

@delmyers @gregg-miskelly @MSLaguana @DanTup @devoncarew @WebFreak001 @roblourens @andysterland @hbenl @lukaszunity @svaarala @ramya-rao-a @vadimcn @felixfbecker @daviwil @rkeithhill @DonJayamanne @rebornix

We have introduced support for column breakpoints which you can try out in latest vscode insiders.
In case a debug adapter does not support column breakpoints you can simply not return the breakpoint column in the breakpoints response. If you do that VSCode will simply transform the column brekapoint to a regular breakpoint.

If your debug adapter already supports column breakpoints then everything should work nicely out of the box - just try it out in vscode insiders.

@felixfbecker
Copy link
Contributor

XDebug doesn't support column breakpoints unfortunately.

@isidorn
Copy link
Contributor

isidorn commented Feb 15, 2017

@felixfbecker then make sure to return a breakpoint without a column specified (or set column to undefined) so vscode transforms your column breakpoints into regular breakpoints once a php session starts.

@WebFreak001
Copy link

GDB, LLDB and Mago don't support this.

tbh I think column breakpoints aren't really a useful feature, more like a feature that looks cool the first time you see it but then never touch it again.

@felixfbecker
Copy link
Contributor

@WebFreak001 it is very useful for breaking inside arrow functions in JS like

[1, 2, 3].map(x => x * 2)

@weinand
Copy link
Contributor

weinand commented Feb 17, 2017

@felixfbecker exactly, that's a good example where this feature shines.
VS Code tries to offer best-of-breed functionality to those who care.

@WebFreak001
Copy link

Well I can't really think of anything you would want to debug there because those functions are often very simple and not really a debugging target. Well but I guess for scripting languages where you often put multiple statements in one line it makes sense, but for native languages this wouldn't work most of the time anyway, as most of them only store a line/instruction pointer map (or something similar) for the debugger

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Feb 17, 2017

@isidorn I don't believe Python supports this either, hence no changes in the Python debugger.
Thanks for the great work

@felixfbecker
Copy link
Contributor

@WebFreak001 Just must have not worked with these often then. If you work with promises, you can have a promise chain with a dozen of these, and it is extremely valuable being able to break at each step in the chain and inspecting the results.

@rkeithhill
Copy link

Cool! PowerShell supports column breakpoints. I'll look into adding support for this in the PowerShell debugger. cc @daviwil

@daviwil
Copy link
Contributor

daviwil commented Feb 17, 2017

Nice :)

@weinand
Copy link
Contributor

weinand commented Feb 17, 2017

We are still working on the final UI, but this won't affect debug adapters.

@rkeithhill
Copy link

PR submitted. The next drop of PowerShell will support column breakpoints once the next version of VSCode drops.

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
Projects
None yet
Development

No branches or pull requests

9 participants