search

bscan / PerlNavigator

Perl Language Server that includes syntax checking, perl critic, and code navigation
MIT License
198 stars 39 forks source link

default 'end' of 500th character breaking nightly neovim #133

Closed WhoIsSethDaniel closed 3 weeks ago

WhoIsSethDaniel commented 3 months ago

I haven't looked through all the code but there is at least one place where the 'end' of a range is hard-coded to 500 and character 500 does not exist on the line to be jumped to. This breaks jump-to-definition in recent nightly neovim. The reason for the breakage is that neovim recently tightened up range checking in some code. If I change the 500 to 5 the jump-to-definition works (in this case it works. If the first line was less than 5 characters long it would fail).

WhoIsSethDaniel commented 3 months ago

I don't have a particularly good solution for fixing this unless it's possible to simply make the 'end' either 0 or 1 (whichever is the lowest possible number).

WhoIsSethDaniel commented 3 months ago

Looks like this 500 value is also used here and here. I don't know if these particular instances affect recent neovim.

bscan commented 3 months ago

Ah, I figured that would come back to bite me someday. For navigation and diagnostics, I usually think about it as a specific line of code, rather than a subset of the line. So, most of the places that give definitions simply list the line number and then go-to-definition to that line. Similarly, for perl syntax errors, they are usually given as only a line number.

For vscode at least, it accepts these large values. For go-to-definition, vscode will highlight the entire line. For diagnostics, vscode will put a squiggly line under all of the code in that line. I suppose the alternative would be to look up the actual length of the line in the file and return that length instead. It's probably straightforward to do, since resolveElemForNav is already reading the file (to improve accuracy and account for off-by-one issues stemming from the perl compiler)

WhoIsSethDaniel commented 3 months ago

If you can retrieve the length of the line being jumped to that would be preferable. In looking at a few other language servers they seem to do something like that.

WhoIsSethDaniel commented 3 months ago

One other option that may be simpler is to have 'end' be start line + 1 and character be 0. This assumes that there are at least start + 1 lines in the file (which may be easier to verify than the number of characters in a line).

bscan commented 3 weeks ago

Hi @WhoIsSethDaniel, looks like this issue was fixed on the Neovim side by https://github.com/neovim/neovim/issues/28281. Would you mind verifying if the fix works once it lands in some release? Thanks!

WhoIsSethDaniel commented 3 weeks ago

Per conversation in #144 this issue was resolved upstream in https://github.com/neovim/neovim/issues/28281.