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

Parse JSON false as :json-false instead of nil #4191

Conversation

robbert-vdh
Copy link
Contributor

This would break code actions where the server sends us an action containing a false boolean parameter. lsp-mode would translate this into nil, and then send a null back to the language server. An example of this causing problems was
#4184.

There may be parts of lsp-mode that expect false values to be parsed as nil, so any logic involving parsed booleans may need to be updated. I have not dug around much, so input from people who are more familiar with lsp-mode's internals is very much appreciated!

Fixes #4184.

This would break code actions where the server sends us an action
containing a `false` boolean parameter. `lsp-mode` would translate this
into `nil`, and then send a `null` back to the language server. An
example of this causing problems was
emacs-lsp#4184.

There may be parts of `lsp-mode` that expect `false` values to be parsed
as `nil`, so any logic involving parsed booleans may need to be updated.
@robbert-vdh robbert-vdh marked this pull request as draft October 13, 2023 12:03
@robbert-vdh
Copy link
Contributor Author

This indeed causes problems elsewhere in lsp-mode where it expects parsed booleans to either be truthy or nil, and :false is truthy. This change would require any part of lsp-mode and its clients that interact with booleans to be changed to handle this.

@yyoncho
Copy link
Member

yyoncho commented Oct 13, 2023

The preferred solution is to patch it in place. There are several other places where we have this issue. See: https://github.com/emacs-lsp/lsp-mode/blob/master/lsp-completion.el#L694

@robbert-vdh
Copy link
Contributor Author

Having to explicitly patch lsp-mode for every code action that uses booleans doesn't sound ideal, but somehow this doesn't seem to cause a lot of problems. In this case, does lsp-mode have a mechanism that lets this hack live in lsp-haskell (which doesn't do a whole lot other than defining the client) or does it need to live in lsp-mode itself?

@yyoncho
Copy link
Member

yyoncho commented Oct 13, 2023

Technically you can put it wherever you want. I would suggest it to put in the place in which it will be easier to implement it. I believe it will be easier to put it in lsp-mode.

Unfortunately, there is no alternative (easy) solution and we should go with this ugly approach...

@robbert-vdh
Copy link
Contributor Author

Superseded by #4194.

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.

Parsing and encoding JSON booleans doesn't roundtrip correctly for code actions
2 participants