Skip to content

Conversation

@weinand
Copy link
Contributor

@weinand weinand commented Jul 18, 2016

Protocol addition "set-next-statement" (#28) contains:

  • a new optional supportsGotoTargetsRequest capability
  • a new type GotoTarget
  • a new optional gotoTargetsRequest request
  • a new optional GotoRequest request

@gregg-miskelly
Copy link
Member

LGTM

/** 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 {
Copy link
Member

@gregg-miskelly gregg-miskelly Jul 18, 2016

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregg-miskelly yes, makes sense and brings it closer to the similar StepInTarget.

@jacdavis
Copy link

IMHO, I think it would be better to adopt the VS model here. Essentially, you'd just need a GotoRequest that takes the file path and line number and can handle errors.

@gregg-miskelly
Copy link
Member

@jacdavis that wouldn't work very well with trying to build a debug engine that exposes out the debug protocol - AD7 has a bunch of different SetNextStatement related methods (EnumCodeContexts, CanSetNextStament, IDebugCodeContex2.Compare, SetNextStatement) with a disambiguation dialog in between some of them. I think we can safely combine EnumCodeContexts+CanSetNextStatement in the enumeration API. But I think if we try and combine them all together will not be able to create a debug engine that can support this.

@jacdavis
Copy link

That’s right, I forgot about EnumCodeContexts. I guess we already have the scalability issue even in vs.

From: Gregg Miskelly [mailto:[email protected]]
Sent: Monday, July 18, 2016 10:28 AM
To: Microsoft/vscode-debugadapter-node [email protected]
Cc: Jackson Davis [email protected]; Mention [email protected]
Subject: Re: [Microsoft/vscode-debugadapter-node] Proposed protocol addition "set-next-statement" #28 (#45)

@jacdavishttps://github.com/jacdavis that wouldn't work very well with trying to build a debug engine that exposes out the debug protocol - AD7 has a bunch of different SetNextStatement related methods (EnumCodeContexts, CanSetNextStament, IDebugCodeContex2.Compare, SetNextStatement) with a disambiguation dialog in between some of them. I think we can safely combine EnumCodeContexts+CanSetNextStatement in the enumeration API. But I think if we try and combine them all together will not be able to create a debug engine that can support this.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/45#issuecomment-233398311, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AM3W_cIcP9fgc1LU_B4Eic3G6Z3PhOpwks5qW7ejgaJpZM4JO2mq.

@weinand
Copy link
Contributor Author

weinand commented Jul 22, 2016

I've merged the PR and added a label attribute to the GotoTarget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants