Interpret Position offset as UTF16 #419
Conversation
|
I don't unfortunately. Position within the file is sent back and forth from and to the language server all over the language server API. The best test at this point is probably to open up a complex file connected to the language server, add a bunch of characters from outside the basic multilingual plane, and then mess around to make sure you don't get offset hovers, incorrect completion, offset codeinfo, and no crashes for attempting out-of-bounds access when the cursor is near the end of lines. |
|
Looks right to me. Would be good to have some tests, of course. |
|
Ok, cool. I'll add tests (and fix the ones that are now broken), and then hopefully @ZacLN can also take a look. |
|
While trying to fix the tests I realized that I thoroughly don't understand our policy with respect to 0 and 1 based indices in this code base :) I would have thought that whole doc offsets are 1 based? But they don't seem to? I'm afraid @ZacLN will have to carry this over the finishing line, I'm a little lost right now. |
|
@ZacLN could you take a look at this? This PR is not correct, I don't understand our internal design what an offset actually is :) But we need something like that because right now we are not interpreting positions correctly... See the issue linked a the top for a bit more background. I think it would be good if we try to fix this, I'm pretty positive that this explains a number of the failures we are seeing on telemetry... |
|
Will dig into this tonight |
Fixes #401. Maybe.
@StefanKarpinski, if you could take a quick look at what I'm doing here, it would be great!
The other question is whether is is really enough to change those two functions, or whether there are other places in the code base that also need to change. I fear that is beyond my understanding of the code base right now, and we'll have to get @ZacLN's input.
@non-Jedi do you have a way to test whether this branch fixes the issue as you have identified it?