bug#60557: [PATCH] Fix eglot prompt when connection already exists

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: joaotavora@gmail.com
>> From: Evgeni Kolev <evgeni.d.kolev@gmail.com>
>> Date: Wed, 4 Jan 2023 17:44:27 +0200
>> 
>> [CC-ing João Távora, author of eglot]
>> 
>> This patch fixes an incorrect behavior of eglot. To reproduce:
>> - run M-x eglot
>> - run M-x eglot (again)
>> - eglot now prompts whether to reconnect to the existing LSP
>> - enter "n" (for no)
>> - now eglot shuts down the LSP process and reconnects - this is not
>> correct, the expected behavior is to not reconnect
>> 
>> The patch is below.
>
> João, any comments?

Yes, sorry for the delay.

Evgeni Kolev <evgeni.d.kolev@gmail.com> writes:

> This patch fixes an incorrect behavior of eglot. To reproduce:
> - run M-x eglot
> - run M-x eglot (again)
> - eglot now prompts whether to reconnect to the existing LSP
> - enter "n" (for no)
> - now eglot shuts down the LSP process and reconnects - this is not
> correct, the expected behavior is to not reconnect

Well you _are_ seing the expected behaviour!  To "disconnect and connect
again" is different from "reconnect".  Thought often not much in
practice, the former means you can use different command arguments to
the server, like when doing C-u M-x eglot or when M-x eglot can't guess
a command and lets you type one in.

So in a way, it's like the message should be:

   "Do you want to reconnect to same language server or do you want to
    disconnect and connect again with whatever new things you've passed
    to M-x eglot?"

However, I agree 100% with you that this message is very confusing.
I've been baffled by it before.  So we should attempt a fix.

Unfortunately, your patch introduces a problem, so I don't think it
should be merged.  If we install it, it means that a user doing:

  M-x eglot RET /path/to/some/server-that-works-but/not-very-well RET
  M-x eglot RET /this/one/is/much-better RET

will never have an interactive chance to actually try the 'much-better'
server.  For this user, it's either reconnect to 'not-very-well' or
don't do anything at all.  And that's probably not what the user meant,
and she has wasted the effort of typing "/this/one/is/much-better".  Do
you understand?

Going forward, we must ask: do we want to keep the "do you want to
reconnect instead" possibility?

1) if yes, then there probably has to be a three-way query somewhere
   offering options a) reconnect, b) disconnect-then-connect, and c) do
   nothing.  This is a bit akward, but it is doable.  This query should
   happen sooner than it does not, probably in 'eglot--guess-contact',
   _before_ asking the user about any interactive arguments to 'M-x
   eglot'

2) if no, then we are saying that an interactive 'M-x eglot' is either
   going to tear down any current connection or do anything at all.
   That's fine by me.  A simple reconnect is still just an 'M-x
   eglot-reconnect' away.  If we 2.1) don't touch 'eglot--guess-contact'
   it'll still be a bit akward to be given the possiblity to answer "no"
   after typing in the new server path.  If we 2.2) adjust
   'eglot--guess-contact', that quirk could be solved.

I think 2.1 and 2.2 are the best solutions here, 2.2 being clearly
better.  But 2.1 is less disruptive and easier to code than 2.2.

João