Special handling for empty Json RPC Diagnostics #125
Conversation
|
(nvm, I wasn't able to repro anymore with nightlies but that's because it only happens for multi files diagnostics...) |
src/haxeLanguageServer/Context.hx
Outdated
| case DResult("") if (method == DisplayMethods.Diagnostics): | ||
| haxeDisplayProtocol.handleMessage(({ | ||
| jsonrpc: Protocol.PROTOCOL_VERSION, | ||
| id: @:privateAccess haxeDisplayProtocol.nextRequestId - 1, // ew.. |
There was a problem hiding this comment.
would it make sense to use reflection to read id from message parameter to avoid having to guess what response callback might be waiting for a cleanup?
as far as I can see most writeMessage calls use an id field, except for notifications.
There was a problem hiding this comment.
I have no idea tbh :/ I just reused the ugly hack from 10 lines below
There was a problem hiding this comment.
yeah I saw that. when I think of e.g. rename feature it will send out lots of requests to resolve types of identifiers. I believe I only send them one by one, so they don't block other user requests. but I'm not sure if we can't run into a situation where there are two requests out?!
which would mean nextRequestId - 1 could hit the wrong one.
There was a problem hiding this comment.
I don't think that's currently possible as request handling is blocking (on Haxe compiler side at least), but yeah that doesn't sound ideal.
Afaiu, writeMessage there can only ever be called with either a RequestMessage or a NotificationMessage.
In the case of this diagnostics patch, we can safely cast message to RequestMessage and get the id. For the original hack, I'm not sure it's safe to assume we got an id, but I guess it'd still be better to try to read message.id and fallback to the hack only if that's null. That part seems a bit out of scope here though.
See HaxeFoundation/haxe#11709 (which will be handled, but this will still be necessary for older Haxe nightlies)