Feature/multiple encodings handled #88
Conversation
Actually, the only supported encoding was UTF-8. In cases where the editor sends updates encoded in UTF-16, the mirror of the user's workspace goes out of sync, leading to a server crash. Furthermore, UTF-16 is the default and mandatory encoding for the protocol, and servers must support it. Therefore, it is imperative to ensure its support. Now, to avoid any unnecessary conversion, the server negotiates the encoding with the client during the initialization phase. This allows the server to choose its preferred encoding in cases where it is available. See: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocuments
|
By the way, do you think you could run the CI ? It would be great for me to ensure that everything works fine. Thanks @McPatate ! 😄 |
|
Hey @jeremyelalouf thanks for the PR!
I haven't been able to find a way to circumvent the fact that secrets are not injected in workflows coming from forks. If you could run it manually it'd be nice, otherwise I'll see later myself if this causes issues. Did you test your code manually & can confirm it works as intended? |
|
Hi @McPatate, Concerning the CIFirstly, I apologize for not being aware of the issues you've experienced with Actions and forks. I would also add that I didn't know that I had access to self hosted runners of an Organization by forking their repository, it could be dangerous don't you think ? About my testsI've taken measures to ensure that I haven't introduced any issues and that everything is functioning as expected. To achieve this, I added tests for the document::Document struct in the document.rs file. Here's a video demonstrating the previous behavior of the LSP when handling non-supported encodings, leading to a crash. Demonstration.of.llm-ls.crash.movThen, another video showcases the current behavior of the LSP after integrating the fixed code. As you can see, there are no more crashes to report 🫡. Demonstration.of.the.llm-ls.fixed.movUnfortunately, I didn't have the time to run testbed locally, which is why I requested the CI to assist me with this aspect. I kindly request your understanding that, despite the checks I've performed, I cannot fully guarantee the stability of the current version. Thanks again 😄 ! |
There was a problem hiding this comment.
This looks very nice, thanks a lot for the hard work!
Regarding type conversion from the custom type to the lsp_types, here is a small example of what I mean:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=91e9f5a118e2616dee680e1fdafd05dc
I've left comments on what I think needs some work, but lgtm other than that!
You don't that's & that's what's difficult when having external contributions 😄 Appreciate you reproducing the bug & displaying the fix!
No problem, this looks good enough to me. I'd appreciate if you can run testbed, but not a hard requirement. |
Totally makes sense 😂 ! I will then try to run Cheers ! |
Use of the `document::PositionEncodingKind` enum everywhere it's possible and TryFrom implemented on it. Also, LspError are not used anymore.
|
Hello again @McPatate, I've just pushed the fix of almost all comments you've made. Keep in mind that for the first one, I did, however, offer a solution that I felt was appropriate in my last commit. Have a good night ! |
|
There you go, @McPatate! I believe I've addressed all your comments. Please let me know if I've overlooked anything or if you have any additional feedback ! |
|
With great pleasure ! It was a great experience to contribute to this project 🥳 (and thanks to my best Astek @rojas-diego) |
Hi everyone 👋🏻
Here, you will find the integration of this Gist made by @rojas-diego, which adds support for multiple encodings.
Actually
The only supported encoding was UTF-8.
In cases where the editor sends updates encoded in UTF-16,
the mirror of the user's workspace goes out of sync, leading to a server crash.
Furthermore, UTF-16 is the default and mandatory encoding for the protocol,
and servers must support it.
Therefore, it is imperative to ensure its support.
Also
To avoid any unnecessary conversion, the server negotiates the encoding
with the client during the initialization phase.
This allows the server to choose its preferred encoding in cases where it is available.
See: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocuments