-
Notifications
You must be signed in to change notification settings - Fork 86
Proposed protocol addition "set-next-statement" #28 #45
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -411,6 +411,26 @@ export module DebugProtocol { | |
| export interface RestartFrameResponse extends Response { | ||
| } | ||
|
|
||
| /** Goto request; value of command field is "goto". | ||
| The request sets the location where the debuggee will continue to run. | ||
| This makes it possible to skip the execution of code or to executed code again. | ||
| The code between the current location and the goto target is not executed but skipped. | ||
| The debug adapter first sends the GotoResponse and then a StoppedEvent (event type 'goto'). | ||
| */ | ||
| export interface GotoRequest extends Request { | ||
| arguments: GotoArguments; | ||
| } | ||
| /** Arguments for "goto" request. */ | ||
| export interface GotoArguments { | ||
| /** Set the goto target for this thread. */ | ||
| threadId: number; | ||
| /** The location where the debuggee will continue to run. */ | ||
| targetId: number; | ||
| } | ||
| /** Response to "goto" request. This is just an acknowledgement, so no body field is required. */ | ||
| export interface GotoResponse extends Response { | ||
| } | ||
|
|
||
| /** Pause request; value of command field is "pause". | ||
| The request suspenses the debuggee. | ||
| The debug adapter first sends the PauseResponse and then a StoppedEvent (event type 'pause') after the thread has been paused successfully. | ||
|
|
@@ -599,6 +619,31 @@ export module DebugProtocol { | |
| }; | ||
| } | ||
|
|
||
| /** GotoTargets request; value of command field is "gotoTargets". | ||
| This request retrieves the possible goto targets for the specified source location. | ||
| These targets can be used in the "goto" request. | ||
| The GotoTargets request may only be called if the "supportsGotoTargetsRequest" capability exists and is true. | ||
| */ | ||
| export interface GotoTargetsRequest extends Request { | ||
| arguments: GotoTargetsArguments; | ||
| } | ||
| /** Arguments for "gotoTargets" request. */ | ||
| export interface GotoTargetsArguments { | ||
| /** The source location for which the goto targets are determined. */ | ||
| source: Source; | ||
| /** The line location for which the goto targets are determined. */ | ||
| line: number; | ||
| /** An optional column location for which the goto targets are determined. */ | ||
| column?: number; | ||
| } | ||
| /** Response to "gotoTargets" request. */ | ||
| export interface GotoTargetsResponse extends Response { | ||
| body: { | ||
| /** The possible goto targets of the specified location. */ | ||
| targets: GotoTarget[]; | ||
| }; | ||
| } | ||
|
|
||
| //---- Types | ||
|
|
||
| /** Information about the capabilities of a debug adapter. */ | ||
|
|
@@ -619,6 +664,8 @@ export module DebugProtocol { | |
| supportsSetVariable?: boolean; | ||
| /** The debug adapter supports restarting a frame. */ | ||
| supportsRestartFrame?: boolean; | ||
| /** The debug adapter supports the gotoTargetsRequest. */ | ||
| supportsGotoTargetsRequest?: boolean; | ||
| } | ||
|
|
||
| /** An ExceptionBreakpointsFilter is shown in the UI as an option for configuring how exceptions are dealt with. */ | ||
|
|
@@ -631,6 +678,22 @@ export module DebugProtocol { | |
| default?: boolean | ||
| } | ||
|
|
||
| /** A GotoTarget describes a code location that can be used as a target in the 'goto' request. | ||
| The possible goto targets can be determined via the 'gotoTargets' request. | ||
| */ | ||
| export interface GotoTarget { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also need a 'label' string to provide a disambiguation dialog if there is more than one.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gregg-miskelly yes, makes sense and brings it closer to the similar |
||
| /** Unique identifier for a goto target. This is used in the goto request. */ | ||
| id: number; | ||
| /** The line of the goto target. */ | ||
| line: number; | ||
| /** An optional column of the goto target. */ | ||
| column?: number; | ||
| /** An optional end line of the range covered by the goto target. */ | ||
| endLine?: number; | ||
| /** An optional end column of the range covered by the goto target. */ | ||
| endColumn?: number; | ||
| } | ||
|
|
||
| /** A structured message object. Used to return errors from requests. */ | ||
| export interface Message { | ||
| /** Unique identifier for the message. */ | ||
|
|
||
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.
There are some cases where the set next statement will not be possible. For instance, we typically block a set next statement into a different function than the current function. The existing error fields in response should be sufficient to account for that, but I want to point out that the correct behavior in that case should be to display the error and leave the current statement in place.
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.
@jacdavis I thought that the
GotoTargetsRequestwould only return legal targets. Why would it return a target that would make theGotoRequestfail later on?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 think that an implementation would try to filter such things out, but the UI shouldn't assume that such techniques will be 100% successful.
Uh oh!
There was an error while loading. Please reload this page.
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.
The semantics of this protocol are a bit different than the way VS works. In VS we don't retrieve the list of valid targets first because it could potentially be very large (some customers have large functions, especially in auto generated code.) So instead, the ui attempts the set next statement wherever the user has the cursor and then fails if it is an invalid location. We may want to consider that model since this could be a very large response.
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.
Sure, but then the erroneous cases are the exception and those can be handled through the error response.
Uh oh!
There was an error while loading. Please reload this page.
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.
@jacdavis the current proposal cannot handle that case because the
GotoRequesttakes an ID of a target returned from theGotoTargetsRequest. You cannot manufacture these targets from a cursor location.From @gregg-miskelly 's initial request I was assuming that all targets have to be returned from
GotoTargetsRequest.