-
Notifications
You must be signed in to change notification settings - Fork 159
feat: add instruction reference to GotoTargets request
#578
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
base: main
Are you sure you want to change the base?
Conversation
debugAdapterProtocol.json
Outdated
| }, | ||
| "offset": { | ||
| "type": "integer", | ||
| "description": "The offset from the instruction reference in bytes.\nThis can be negative. Clients may set this property only if the `supportsGotoInstruction` is true." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "description": "The offset from the instruction reference in bytes.\nThis can be negative. Clients may set this property only if the `supportsGotoInstruction` is true." | |
| "description": "The offset from the instruction reference in bytes.\nThis can be negative. This may only be provided when an `instructionReference` is included in the request." |
debugAdapterProtocol.json
Outdated
| }, | ||
| "instructionReference": { | ||
| "type": "string", | ||
| "description": "The instruction reference for which the goto targets are determined.\nThis should be a memory or instruction pointer reference from an `EvaluateResponse`, `Variable`, `StackFrame`, `GotoTarget`, or `Breakpoint`. Clients may set this property only if the `supportsGotoInstruction` is true." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd do something like...
| "description": "The instruction reference for which the goto targets are determined.\nThis should be a memory or instruction pointer reference from an `EvaluateResponse`, `Variable`, `StackFrame`, `GotoTarget`, or `Breakpoint`. Clients may set this property only if the `supportsGotoInstruction` is true." | |
| "description": "The instruction reference for which the goto targets are determined.\nThis should be a memory or instruction pointer reference from an `EvaluateResponse`, `Variable`, `StackFrame`, `GotoTarget`, or `Breakpoint`. Clients may set this property only if the `supportsGotoInstruction` is true. If `instructionReference` is set, the debug adapter should ignore the `line`, `column`, and `source` on the request and clients may set them to zero or empy values." |
Unfortunately making them actually optional in the types is an interface-breaking change.
debugAdapterProtocol.json
Outdated
| "type": "boolean", | ||
| "description": "The debug adapter supports ANSI escape sequences in styling of `OutputEvent.output` and `Variable.value` fields." | ||
| }, | ||
| "supportsGotoInstruction": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would name this supportsGotoInstructionTargets just in case we ever have another type of 'go to' action
c1e87e7 to
f00c30f
Compare
Fixes #577
I added this feature based on the
instuctionRefenence+offsetpair, similar to thesetInstuctionBreakpointrequest. I don't know what to do with existing required fields (sourceandline). It looks like that client could set them based on the selected stack frame. And the debug adapter should be able to resolve the selected instruction to the correct location. WDYT?