-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Text_document has bugs when performing updates and getting positions #1207
Comments
Another possibility is to have the lsp check the on disk file, when it gets a file-saved notification to see if the current editor file has diverged from the file on disk, if so it could freak out and also load the on-disk file |
I don't understand the issue with the first test. As far as I see, you're inserting the string |
ahh, sorry, the tests actually fail. The test shows what should happen, if you actually run them you see the output is wrong.
let beginning_of_line t =
let line=t.line in
let t = prev_newline t in
if is_begin t &&t.line=line then t else advance_char t
|
Signed-off-by: Rudi Grinberg <me@rgrinberg.com> <!-- ps-id: 271f1802-b582-4bac-b946-fe86c8e85541 -->
I attempted to reproduce in #1216. The first test seems to pass. Haven't looked closely at the second one yet. |
Oh okay, I'll test it myself again but my results were
You can see foo gets duplicated |
Thanks for the bug report @faldor20 and @emilienlemaire for the fix |
The Text_document module has some weird bugs:
These expect tests show the issue:
Test one demonstrates how making the change causes text to become duplicated
Test two demonstrates how after making a change the absolute position comes out messed up.
I noticed that in my testing the end_ position when running
absolute_range
seemed unaffected, maybe that helps.I discovered these issues while trying to use the lsp as a library in another project, they were very very confusing to debug 😅
I've noticed that sometimes it seems ocmallsp kind of gets confused and has to be restarted, I suspect this is a contributor to that issue as it could cause the editor and language-service to go out of sync.
I wonder if we could set up a property based test for the text_Document that run's the same edit's on a string using a simpler method than the zipper, then just feed in a huge sample of random positions and change text and see what happens. Just a thought.
The text was updated successfully, but these errors were encountered: