fixes bug with textDocument/definition on LanguageClient-neovim #13
Conversation
|
Hmm yeah, I noticed Sublime also sent some extra data. Seems like most implementations aren't as strict as ours in checking their data.. But this will quickly become unmaintainable for us if we have to add a special case field for every editor out there, so I think I'll fix this by introducing a relaxed check parameter to The position parameter thing is a bit more tricky. It seems that the issue here is that Nim can return things with negative position if it's a general error and not related to some specific part of the document. I think it would be better to change this check to also disallow these numbers, and find some other way of returning them that doesn't require us to place them within a document. |
|
I left a comment over in the main LSP repository. Apparently this is not a new issue, but something that hasn't been resolved yet: microsoft/language-server-protocol#256 (comment) |
| response.add create(Diagnostic, | ||
| create(Range, | ||
| create(Position, diagnostic.line-1, diagnostic.column), | ||
| create(Position, diagnostic.line-1, max(diagnostic.column, endcolumn)) | ||
| create(Position, diagnosticLine, diagnosticColumn), |
There was a problem hiding this comment.
This should always be diagnostic.column, and not endColumn. This logic was stolen from the VSCode plugin which tries to find the thing that was wrong and underline it. So the range here is supposed to go from where Nim finds the issue, and to the end of the thing that is in error.
| @@ -387,11 +387,13 @@ while true: | |||
| # Try to guess the size of the identifier | |||
| let | |||
| message = diagnostic.nimDocstring | |||
| endcolumn = diagnostic.column + message.rfind('\'') - message.find('\'') - 1 | |||
| endcolumn = max(diagnostic.column + message.rfind('\'') - message.find('\'') - 1, 0) | |||
There was a problem hiding this comment.
This doesn't really do much as it got max'ed with diagnostic.column later anyways. I think the problem here is that things with line or column <0 aren't visible in the document and therefore should be returned in some other way.
| rawLine = name["position"]["line"].getInt | ||
| rawChar = name["position"]["character"].getInt | ||
| rawLine = max(name["position"]["line"].getInt, 0) | ||
| rawChar = max(name["position"]["character"].getInt, 0) |
There was a problem hiding this comment.
This change means that rawLine and rawChar aren't really raw any longer so it should be renamed to something else.
|
I also noticed that |
|
Just added an |
It seems like LanguageClient-neovim sends extra data on the textDocument/definition request, which casues the server to fail the validation. I added those fields as optionals to the message schema and it seems to solve the issue.
I also found that I was getting constant errors about LanguageServer-neovim failing to deserialize the position params when it received a
-1, as it expects anu64. Passing 0 instead seems to stop the constant errors.