search

derickr / dbgp

Common DeBugGer Protocol as used by Xdebug and other implementations.
http://xdebug.org/docs-dbgp.php
MIT License
23 stars 5 forks source link

Allow for an additional 'scope_change' breakpoint type. #9

Closed derickr closed 7 years ago

derickr commented 8 years ago

This breakpoint is meant to signal to the debugger engine whether it should break on changes in the engine's state. The only type that I am currently adding to the spec is a "scope change". This breakpoint will trigger when the active scope of the current state of the debugger engine changes.

This, combined with another PR (https://github.com/derickr/dbgp/pull/8) regarding the validity of configured breakpoints should allow for the detection of breakpoints on lines of code that are not valid.

johnislarry commented 8 years ago

Regarding knowing when breakpoints resolve, what advantages does this have over using dbgp notifications?

https://xdebug.org/docs-dbgp.php#notifications

It seems to me that if all you want to know is when a breakpoint resolves, it's weird to have to subscribe to this scope_change breakpoint. This conflates two discrete concepts: knowing when a breakpoint resolves, and knowing when scope changes. Under this model, to answer questions of the form "I have a breakpoint A; when does A resolve?", the IDE would have to poll the debugger engine every time we hit a scope_change breakpoint. This could lead to a lot of unnecessary work on both the IDE and debugger engine's part. With notifications, whenever the debugger engine resolves the breakpoint, it can just send a notification and we're done with no wasted effort spent polling. I think scope_change breakpoints are useful in their own right, but using them as a way to determine when a breakpoint resolves is sub-optimal, and notifications seem better suited to the task.

derickr commented 8 years ago

I had forgotten notifications even existed. In that case, I think that is indeed a way to do that. I'll have a good think about that and add a new PR for that - and likely update the wording in this one.

derickr commented 7 years ago

Closing this, in favour of the notification approach in https://github.com/derickr/dbgp/pull/11