Skip to content
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

Support for ltex-ls-plus #48

Closed
rpls opened this issue Nov 29, 2024 · 10 comments
Closed

Support for ltex-ls-plus #48

rpls opened this issue Nov 29, 2024 · 10 comments
Labels

Comments

@rpls
Copy link

rpls commented Nov 29, 2024

ltex-ls doesn't seem to be maintained anymore. ltex-ls-plus is a quite well maintained fork, already including a range of important updates, bug fixes, and enhancements (most important for me, much fewer HTTP 413 errors when using the premium service, due to smaller request sizes). It's already possible to convince lsp-ltex to use ltex-ls-plus by manually downloading and placing it in the ~/.emacs.d/.cache/lsp/ltex-ls/latest folder. Everything appears to be compatible, so most likely not much has to be changed.

@jcs090218
Copy link
Member

Do you know what happens to the upstream? 🤔

I think we can simply change the variable lsp-ltex-repo-path's value to "ltex-plus/ltex-ls-plus".

@jcs090218 jcs090218 pinned this issue Nov 30, 2024
@rpls
Copy link
Author

rpls commented Nov 30, 2024

I think we can simply change the variable lsp-ltex-repo-path's value to "ltex-plus/ltex-ls-plus".

A bit more: The folder- and executable names all changed to ltex-ls-plus (one thing I forgot in my comment above, I also placed a symlink from ltex-ls to ltex-ls-plus in the bin folder). But so far, it would probably only involve renaming a bunch of stuff (or making things customizable).

Not sure what happened to the original upstream, just appears to be inactive and ltex-ls-plus fixes a lot of issues I encounter.

@jcs090218
Copy link
Member

jcs090218 commented Nov 30, 2024

Should we create another package named lsp-ltex-plus? I think it's safer since the upstream has changed the original name ltex to ltex-plus. 🤔

@rpls
Copy link
Author

rpls commented Nov 30, 2024

Good question. ltex-ls-plus has recently introduced support for more file types. But that's just adding more modes to lsp-ltex-active-modes and should work as-is (and is customizable right now). From what I see so far, they don't have additional features, e.g., more/different server settings. Right now, it's probably just a matter of making repo-path and maybe something like server-executable-name customizable, not really worth making a fork IMO. lsp-pyright, for example, also supports multiple different servers (pyright and basedpyright). And these even have differing feature sets. The ltex-ls-plus team did, however, fork the VS Code extension to make their own. I think because you can't adapt the original extension to use ltex-ls-plus, which is not the case here.

@real-or-random
Copy link
Contributor

Let me just add that I also think that supporting ltex-ls-plus is a good idea. It's apparently actively maintained.

@real-or-random
Copy link
Contributor

real-or-random commented Jan 20, 2025

As a quick fix, this override seems to do the job for me (if you have a system installation of ltex-ls-plus):

(defun lsp-ltex--server-command ()
   "Startup command for LTEX language server."
   (list (executable-find "ltex-ls-plus"))))

edit: And yes, this indicates that a fork is overkill. All the internal settings (e.g., ltex.language) have not been renamed, probably for a good reason.

@jcs090218
Copy link
Member

edit: And yes, this indicates that a fork is overkill. All the internal settings (e.g., ltex.language) have not been renamed, probably for a good reason.

Maybe we can just expose the executable name so users can configure it themselves? 🤔

@real-or-random
Copy link
Contributor

edit: And yes, this indicates that a fork is overkill. All the internal settings (e.g., ltex.language) have not been renamed, probably for a good reason.

Maybe we can just expose the executable name so users can configure it themselves? 🤔

Yes, I believe that's a simple thing that will work. But if we later want to add download functionality etc., I think a simple toggle use-ltex-ls-plus is better. The executable name can then depend on this. (Though, it probably won't hurt to let the user override it.)

@jcs090218
Copy link
Member

I attempted to integrate it into this package, but it turned out to be more challenging than I expected. As a result, I've decided to create a new package and name it lsp-ltex-plus.

Sorry if this isn't what you are looking for. 😓

@real-or-random
Copy link
Contributor

Thanks! I think that's totally fine. Users will want to switch to lsp-ltex-plus, so it's good to have a separate package with a corresponding name.

Should lsp-ltex development ever start again and not join efforts with lsp-ltex-plus, it's probably annoying to maintain to variants of the package. But all of this is rather unexpected for now... :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants