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

Request: eglot-ensure in an async manner #68

Closed
jojojames opened this issue Aug 10, 2018 · 11 comments
Closed

Request: eglot-ensure in an async manner #68

jojojames opened this issue Aug 10, 2018 · 11 comments

Comments

@jojojames
Copy link
Contributor

I am using KotlinLanguageServer (https://github.com/fwcd/KotlinLanguageServer) and using

(push '(kotlin-mode . ("kotlin-language-server")) eglot-server-programs)
(add-hook 'kotlin-mode-hook #'eglot-ensure)

kotlin-language-server takes a very long time to start up (upwards of 20-30 seconds) which blocks emacs from opening the file until the server is done starting up.

I've tried with/without eglot-ensure along with that server to narrow it down to this language server. (O f course there could be some other interactions between different packages).

Could provide a smaller emacs -Q repo if that would help but I think the main challenge is compiling the server.

@joaotavora
Copy link
Owner

Of course there could be some other interactions between different packages

No, I think I've seen slow server startup time mentioned somewhere already.

Could provide a smaller emacs -Q repo if that would help but I think the main challenge is compiling the server.

Yeah. For this rather put a dummy "sleep" somehwere in the invocation of any other server than spend time messing with Javaland. I will add kotlin to the README.md tho (or take your PR that does it).

@terlar
Copy link

terlar commented Aug 10, 2018

I see similar slowness during the start of bash-language-server, so I don't think it is unique to that server. Although not that long, it blocks around 2-3 seconds when opening the first bash script that starts the server.

@joaotavora
Copy link
Owner

Regarding the actual problem. I think lsp mode suffers from this problem too. It's not particularly hard to fix, I think. I'm actually thinking a hybrid scheme: Wait up to N seconds synchronously, then wait asynchrously.

The hard part here is eglot-ensure: if you don't block until something happens you're not really "ensuring" anything, are you? It's a question of semantics though: i think we can say that we "ensured that we tried" :-)

@jojojames
Copy link
Contributor Author

I will add kotlin to the README.md tho (or take your PR that does it).

I'll PR something for kotlin when I get some free time.

The hard part here is eglot-ensure: if you don't block until something happens you're not really "ensuring" anything, are you? It's a question of semantics though: i think we can say that we "ensured that we tried" :-)

Something like eglot-start-async or whatever would work for me but the naming was never too important for me anyways!

joaotavora added a commit that referenced this issue Aug 11, 2018
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to 30 seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el (slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
@joaotavora
Copy link
Owner

I've pushed a commit to fix this. Its failing some tests (and I don't know why exactly yet), so it's not ready to go to master.

However, I would appreciate it if you could give it some testing. eglot-sync-connect is the new defcustom you should look to. Tell me if the docstring is legible (I didn't double-check those bits) and if you understand how it works. Then tell me if it did work like you wanted it to.

@jojojames
Copy link
Contributor Author

I think eglot-ensure with the latest commit (on your async branch) doesn't seem to connect anymore for me.

I'll describe it symptomatically as I didn't have enough time to do some digging. For most variations of eglot-sync-connect (even t ,don't quote me on t,), emacs will print out a message saying 'connected, eglot managing, ...' but the mode-line indicator doesn't change ([eglot:kotlin] for example is not added) (or something similar, I'm going off memory).

Completions/etc don't seem to be working either.

Given that aside, if it did work (eglot connecting everything), the asynchronous mechanism seems to be working well.

Switching back to the commit right before (without the new defcustom) produces the synchronous eglot-ensure that works for me (flymake providing diagnostics, completions working, etc).

If you point me to where I should be looking, maybe I can dig up more info for you.

As for the docstring,

(defcustom eglot-sync-connect 3
  "Control blocking of LSP connection attempts.
If t, block as long as a .  A positive
integer number means block for that many seconds, and then wait
for the connection in the background.  nil has the same meaning
as 0."
  :type '(choice (boolean :tag "Whether to inhibit autoreconnection")
                 (integer :tag "Number of seconds")))

If t, block as long as a .

I don't know what this means but I assumed it meant to just restore similar behavior to before.

nil has the same meaning as 0

I think I can guess the conclusion of nil == 0 but I think it would be clarifying to explain what 0 does (e.g. don't block at all). I assume that's the case but maybe someone else would be confused.

@joaotavora
Copy link
Owner

Thanks for the feedback. Seems there's still quite a bit of work to do.

You're right about the docstring, even if it's missing part of a sentence

joaotavora added a commit that referenced this issue Aug 12, 2018
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to eglot-connect-timeout, which defaults to 30
seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el (slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
(eglot-connect-timeout): New defcustom.
@joaotavora
Copy link
Owner

@jojojames I've updated the branch. It's still failing tests when the suite runs non-interactively (have to check that). But you can try again and all the other problems (completion, etc) should be gone.

@jojojames
Copy link
Contributor Author

Thanks, works for me. Docstring is also ++.

joaotavora added a commit that referenced this issue Aug 12, 2018
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to eglot-connect-timeout, which defaults to 30
seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el
(eglot--call-with-dirs-and-files): Simplify cleanup logic.
(slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
(eglot-connect-timeout): New defcustom.
(Package-requires): Require jsonrpc 1.0.6
@nickyr
Copy link

nickyr commented Nov 8, 2018

Hey! Thanks for making this. I am a n00b at elisp. Is this the correct way to use eglot-sync-connect if I want no blocking at all?

(setq eglot-sync-connect 0)
(add-hook 'php-mode-hook 'eglot-ensure)

@joaotavora
Copy link
Owner

joaotavora commented Nov 8, 2018 via email

bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 18, 2022
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to eglot-connect-timeout, which defaults to 30
seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el
(eglot--call-with-dirs-and-files): Simplify cleanup logic.
(slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
(eglot-connect-timeout): New defcustom.
(Package-requires): Require jsonrpc 1.0.6
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to eglot-connect-timeout, which defaults to 30
seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el
(eglot--call-with-dirs-and-files): Simplify cleanup logic.
(slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
(eglot-connect-timeout): New defcustom.
(Package-requires): Require jsonrpc 1.0.6
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to eglot-connect-timeout, which defaults to 30
seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el
(eglot--call-with-dirs-and-files): Simplify cleanup logic.
(slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
(eglot-connect-timeout): New defcustom.
(Package-requires): Require jsonrpc 1.0.6

#68: joaotavora/eglot#68
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this issue Oct 12, 2022
A new defcustom eglot-sync-connect controls this feature.  If it is t,
eglot should behave like previously, waiting synchronously for a
connection to be established, with the exception that there is now a
non-nil timeout set to eglot-connect-timeout, which defaults to 30
seconds.

eglot-connect is now considerably more complicated as it replicates
most of the work that jsonrpc-request does vis-a-vis handling errors,
timeouts and user quits..

* eglot-tests.el
(eglot--call-with-dirs-and-files): Simplify cleanup logic.
(slow-sync-connection-wait)
(slow-sync-connection-intime, slow-async-connection)
(slow-sync-error): New tests.

* eglot.el (eglot-sync-connect): New defcustom.
(eglot-ensure, eglot): Simplify.
(eglot--connect): Honour eglot-sync-connect.  Complicate
considerably.
(eglot-connect-timeout): New defcustom.
(Package-requires): Require jsonrpc 1.0.6

GitHub-reference: close joaotavora/eglot#68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants