Address Zed Not Consider Empty String As 'Falsy' - #158
Conversation
LSP spec allows omitting non-mandatory fields, empty string can be problematic, Zed for example doesn't consider empty string as 'falsy'
|
This seems like a hack to me and I think it's released to the JSON streamer not supporting optional types so '' is passed instead. I.e, there's a default value which should be omitted but instead returns the default value for the Pascal type. Is that what you mean? |
|
Let's take insertText for example. In src/serverprotocol/PasLS.Completion.pas TCompletionItem.insertText property only populated when Identifier.IsProcNodeWithParams, otherwise it is in default value (empty string?). When this object serialized, it always included into response JSON. Since fInsertText is a string I don't think it can be set nil in order to omit this property during serialization, hence the hack "remove empty string" after serialization. |
|
See We added that but only after using the wrong non-optional types is most places and just hoping the client will handle default values gracefully. |
|
The spec defined as Switching this PR to draft. |
Empty fields in JSON responses can be problematic, specifically Zed, they don't consider empty string as flasy. So if for example filterText is empty Zed don't fall back to "label" and doesn't show suggestion pop-up.
LSP allows omitting non-mandatory fields so removing those fields completely is better.
Also in this PR: use textEdit instead relying on client falling back to "label" for textDocument/completioni request