google / go-dap Goto Github PK
View Code? Open in Web Editor NEWGo implementation of the Debug Adapter Protocol
License: Apache License 2.0
Go implementation of the Debug Adapter Protocol
License: Apache License 2.0
This was reported and fixed for an individual case in #66. #66 (comment) lists a few more fields with the same problem, but that list is also not complete. As one more example, I ran into this for ErrorResponseBody.Error
:
Line 104 in f58c894
The issue is that Go cannot set this fields to nil
and will instead when parsing create an empty struct. Then when that message is serialized again, it will not be omitted as requested, but the empty values will be written to JSON, in my case leading to something like
"body":{"error":{"id":0,"format":"","showUser":false}}
instead of
"body":{}
and that then confuses VS Code and others.
I think the correct fix would be to go through and make all non-pointer struct fields that have omitempty on them, and turn them into pointers. Do you agree with this solution? Then I could look into preparing a PR for this.
Can we add a Getter/Setter for the embedded Response/Request fields of the different Response-Request-Types?
If these methods would exist we could add an interface for Requests and an interface for Responses which would allow code to easily work with any kind of Request/Response. Currently such code does require a giant switch-statement or the use of reflection.
The Breakpoint is defined as
type Breakpoint struct {
Id int `json:"id,omitempty"`
Verified bool `json:"verified"`
Message string `json:"message,omitempty"`
Source Source `json:"source,omitempty"`
Line int `json:"line,omitempty"`
Column int `json:"column,omitempty"`
EndLine int `json:"endLine,omitempty"`
EndColumn int `json:"endColumn,omitempty"`
InstructionReference string `json:"instructionReference,omitempty"`
Offset int `json:"offset,omitempty"`
}
It says "source,omitempty"
, but an empty object is serialised as:
data, _ := json.Marshal(dap.Breakpoint{})
fmt.Println(string(data))
// {"verified":false,"source":{}}
// ^^^^^^^^
This means that BreakpointEvent
types with missing source are serialised with an empty source object, which clears the breakpoint location in the UI.
Can the Source
field be changed to a pointer field instead?
Using json.RawMessage
for request arguments makes the users of go-dap
unnecessary cumbersome. any
would be a better choice. Compare
msg, err := json.Marshal(someArgs)
if err != nil {
....
}
AttachRequest {
Arguments: json.RawMessage(msg),
}
and
AttachRequest {
Arguments: someArgs,
}
Based on review of PR #19, it is suggested we make server a type with handle, dispatch, etc methods and writer and bufio.Reader fields, so those do not have to be passed around as arguments.
Many debug adapters make use of custom DAP messages, i.e. messages that have the shape of a regular DAP message, which are not part of the protocol. This works, because client-side extensions intercept these DAP messages and do not pass them on to VS Code.
However, go-dap does not have a way to work with such custom DAP messages. I'd like to introduce a new type Codec
in the codec.go
file with the following API:
// NewCodec constructs a new codec that extends the vanilla DAP protocol.
// Unless you need to register custom DAP messages, use
// DecodeProtocolMessage instead.
func NewCodec() *Codec { .. }
// RegisterRequest registers a new DAP command, so that it can be
// unmarshalled by DecodeMessage. Returns an error when the event already
// exists.
func (c *Codec) RegisterRequest(command string, requestCtor, responseCtor MessageCtor) error { .. }
// OR RegisterCustomRequest
// RegisterEvent registers a new DAP event, so that it can be unmarshalled
// by DecodeMessage. Returns an error when the event already exists.
func (c *Codec) RegisterEvent(event string, ctor MessageCtor) error { .. }
// DecodeMessage parses the JSON-encoded data and returns the result of
// the appropriate type within the ProtocolMessage hierarchy. If message type,
// command, etc cannot be cast, returns DecodeProtocolMessageFieldError.
// See also godoc for json.Unmarshal, which is used for underlying decoding.
func (c *Codec) DecodeMessage(data []byte) (Message, error) { .. }
The last function is the equivalent of DecodeProtocolMessage()
.
NOTE: This means that MessageCtor
is now exported, while it was not before.
@polinasok Would such an extension be acceptable?
The field ContinueResponseBody.AllThreadsContinued is marked with "omitempty" ( https://github.com/google/go-dap/blob/master/schematypes.go#L136 ).
This causes the json-encoder to omit the field if it's value is false (as false is the empty value for bools).
Unfortunately the spec says if the value is missing it is assumed to be true for backwards-compatibility. ( https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Continue (see the comment of the field))
This makes it impossible to set this field to false from go.
If it is set to false, it is omitted and the client assumes it to be true.
( If it is true, the client accepts it as true. But this is not the case I am interested in).
Tl;dr: Remove omitempty from the field so it can actually be set to false.
Consider using https://staticcheck.io/docs/running-staticcheck/ci/github-actions/ to run the staticcheck tests.
This also may allow us to get rid of the shell script and make it easier to make it work on windows.
This is a follow up from #77
There were a couple of types that were unintentionally changed in the v0.9.0 release (see #80). We will retract this module version as soon as we release v0.9.1 with the fixed types.
// type = event
for Event, etc (type
field subtype selection)interface{}
to represent a union of different typesMove the logic from the long function that generates the go type into a helper with good function and inline comments. Possible names: shouldEmitField(), skipField()
Some fields are redefined in hierarchical types, but to ensure consistent translation between JSON & Go, we must limit each field to appear only once. We need skip two types of duplicate fields:
Programatic detection of these is tricky since we process one type at a time, so for now we just hardcode known fields/type pairs that fall into the above categories. Comments to document these cases will help.
"type" - use in ProtocolMessage, skip assignments in children (1)
"command" - use in Request and Response, skip assignments in children (1)
"event" - use in Event, skip assignments in all children (1)
"arguments" - skip any type in Request, use specific types in children (2)
"body" - skip any type in Response and Event, use specific types in children (2)
It appears straightforward to use this as a vscode.DebugAdapterServer
(e.g localhost:4711
), but also looking to use this for a vscode.DebugAdapterExecutable
that uses stdin/stdout for the protocol communication. Is is supported? Example?
In RunInTerminalRequestArguments
, env
of type object
is emitted as a map[string]sting
. However, values can also theoretically be nil
, so we need a more generic type like interface{}
in case multiple types are listed.
Currently the fields are emitted in alphabetical order. Preserve the original order of the JSON schema.
There have been a few additions to the DAP protocol, see its changelog. Most notably, there is now a reverse startDebugging
request (spec).
It would be great to update this package to the latest spec to benefit from these new features.
As these additions to the spec are all backwards compatible, I think they can mostly be added independently, and step-by-step. Would you accept PRs for the schema types?
See #19#discussion_r359641154 for more context.
I'm now using emacs with dap-mode.
is the idea integrate this with dlv ?
or is there way to test this as stand alone ? replace nodejs + vscode extension at dap-mode.
As proposed by @eliben, use a map like so:
type MessageConstructor func() Message
var requestCommandToType map[string]MessageConstructor = map[string]MessageConstructor{
"initialize": func() Message { return &InitializeRequest{} },
"source": func() Message { return &SourceRequest{} },
"cancel": func() Message { return &CancelRequest{} },
<...>
}
msg := requestCommandToTypecommand
json.Unmarshal(b, msg);
For example, LaunchRequestArguments in vscode-go calling delve might contain the following implementation specific attributes:
"name":"Launch",
"type":"go",
"request":"launch",
"mode":"debug",
"program":"/Users/foo/go/src/helloworld",
"env": {...} ,
"args:[],
"showLog":true,
"trace":"log",
"logOutput":"rpc",
"stopOnEntry":true,
... etc...
See the following for more information:
https://github.com/Microsoft/vscode-go/wiki/Debugging-Go-code-using-VS-Code#set-up-configurations-in-launchjson
https://github.com/microsoft/vscode-go/blob/be0cc9ed9cd9a7eb5e8501197c8551458fbc2aaf/package.json#L409
The workflow has not been run for 2 years as can be seen on the Actions tab and the tests are not being run on incoming PRs. It appears this is due to the name change of the default branch from master -> main.
Currently, go-dap reprents JSON booleans as bool
, regardless of whether they are optional or required. As a result, a message that does not set a boolean will have it set to false
when deserialized and then re-serialized.
Unfortunately, it is not always specified in the DAP protocol how to treat a missing boolean field. Take for example https://microsoft.github.io/debug-adapter-protocol/specification#message:
/**
* If true show user.
*/
showUser?: boolean;
Strictly speaking (because it is not "Iff" aka "if and only if"), this comment does not specify what should happen when this is unset. And sure enough VS Code interprets missing as true for this field:
https://github.com/microsoft/vscode/blob/27a0cbb26647670ad719bcb47e2d1ca4cee133bf/src/vs/workbench/contrib/debug/browser/debugService.ts#L631
We are deserializing and serializing all messages in a proxy for logging and sanitization, so go-dap turning unset into false is a problem for us.
This could be fixed by representing optional booleans as *bool
(and then using omitempty
), similar to how we represent optional nested messages as *struct
. Unfortunately this is a breaking change as users of the library would then need to dereference the pointer as they read and assign, and handle the nil case explicitly. However, I think this is the correct handling.
What do you think?
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.