Feature/multiple encodings handled

[jeremyelalouf]

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

[jeremyelalouf]

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 ! 😄

[McPatate]

Hey @jeremyelalouf thanks for the PR!

run the CI ?

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?

[jeremyelalouf]

Hi @McPatate,

Concerning the CI

Firstly, I apologize for not being aware of the issues you've experienced with Actions and forks. As you can see, as a result I've initiated the CI on my fork, and I'm awaiting a runner to pick up my jobs.

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 tests

I'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. Additionally, I conducted manual functional tests in an editor to compare the behavior of the current version with my fixed version.

Here's a video demonstrating the previous behavior of the LSP when handling non-supported encodings, leading to a crash.

https://github.com/huggingface/llm-ls/assets/75589941/63e4a244-1d15-4a56-abc6-932913177c9d

Then, 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 🫡.

https://github.com/huggingface/llm-ls/assets/75589941/2843b197-cbe1-4d5c-9d4e-db4f914f6936

Unfortunately, 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. Especially in a production environment where the codebase might be extensive, and the LSP particularly busy.

Thanks again 😄 !

[McPatate]

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 ?

You don't that's & that's what's difficult when having external contributions 😄

Appreciate you reproducing the bug & displaying the fix!

I kindly request your understanding that, despite the checks I've performed, I cannot fully guarantee the stability of the current version. Especially in a production environment where the codebase might be extensive, and the LSP particularly busy.

No problem, this looks good enough to me. I'd appreciate if you can run testbed, but not a hard requirement.

[jeremyelalouf]

You don't that's & that's what's difficult when having external contributions 😄

Totally makes sense 😂 !

I will then try to run testbed locally once I've fixed all your comments.

Cheers !

[jeremyelalouf]

Hello again @McPatate,

I've just pushed the fix of almost all comments you've made. The only unanswered questions I had were in response to your comments, here and here.

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 !

[jeremyelalouf]

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 !

[jeremyelalouf]

With great pleasure !

It was a great experience to contribute to this project 🥳

(and thanks to my best Astek @rojas-diego)