Skip to content
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

DAP: fix serialization of Options #2959

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

the-mikedavis
Copy link
Member

DAP follows the same strict TypeScript interface syntax as LSP
which states:

The document uses TypeScript interfaces in strict mode to describe
these. This means for example that a null value has to be explicitly
listed and that a mandatory property must be listed even if a [falsy]
value might exist.

So we have to skip serializing any fields that end in ? instead
of passing null.

Also included is a change for a cwd field that was in an Option but is a required field.

More context: erlang-ls/erlang_ls#1346

It looks like lsp-types does the same verbose #[serde(skip_serializing_if = "Option::is_none")] dance for optional fields: https://github.com/gluon-lang/lsp-types/blob/b26003aa38ad8a996b583de162b66615212b94a3/src/signature_help.rs#L13-L24

DAP follows the same strict TypeScript interface syntax as LSP
which states:

> The document uses TypeScript interfaces in strict mode to describe
> these. This means for example that a `null` value has to be explicitly
> listed and that a mandatory property must be listed even if a falsify
> value might exist.

So we have to skip serializing any fields that end in `?` instead
of passing `null`.
@archseer archseer merged commit 85411be into helix-editor:master Jul 5, 2022
@the-mikedavis the-mikedavis deleted the md-dap-serialization branch July 5, 2022 12:26
danyspin97 added a commit to danyspin97/helix that referenced this pull request Jul 29, 2022
cwd field has been changed in helix-editor#2959 by making it a required field.
However, when starting the debugging session, cwd is not set, causing
an error in the deserialization part. Manually set cwd to the current
directory as a quick fix.
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.

2 participants