[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#60557: [PATCH] Fix eglot prompt when connection already exists
From: |
Evgeni Kolev |
Subject: |
bug#60557: [PATCH] Fix eglot prompt when connection already exists |
Date: |
Sat, 14 Jan 2023 08:02:59 +0200 |
Hi João, thank you for the detailed reply.
Could you confirm I correctly understand proposal 2.1)? Example interactions:
- M-x eglot RET ;; eglot guesses correctly the CONTACT
- live connection exists?
- no - new connection
- yes - disconnect + new connection
- M-x eglot /some/path RET ;; eglot can't guess so prompts the user
- live connection exists?
- no - new connection
- yes - disconnect + new connection
If I understand correctly, 2.1) always disconnects + reconnects.
Regarding 2.2), I don't fully understand it - could you clarify how
2.2) is different from the _user's_ perspective?
I'm also thinking about option 3) making M-x eglot able to detect a
change in CONTACT:
- M-x eglot RET
- live connection exists?
- no - new connection
- yes - new CONTACT provided?
- no - reconnect to old CONTACT
- yes - disconnect + new connection to new CONTACT
However, I couldn't find a way to detect a change in CONTACT because
it's not preserved as-is (for example '(gopls)) but changed to a
complex structure and stored to (eglot--saved-initargs server).
It's also possible to check current-prefix-arg - if nil, then M-x
eglot can deduce the CONTACT hasn't changed and can re-connect. If
non-nil, then disconnect + connect.
Still, I'm not sure 3) is worth it because the code will become
complex, but from the user's perspective nothing will change - for the
user reconnect is the same as disconnect + connect.
On Thu, Jan 12, 2023 at 12:53 PM João Távora <joaotavora@gmail.com> wrote:
>
> 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
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, Evgeni Kolev, 2023/01/04
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, Eli Zaretskii, 2023/01/12
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, João Távora, 2023/01/12
- bug#60557: [PATCH] Fix eglot prompt when connection already exists,
Evgeni Kolev <=
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, João Távora, 2023/01/14
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, João Távora, 2023/01/16
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, Eli Zaretskii, 2023/01/16
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, Eli Zaretskii, 2023/01/16
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, João Távora, 2023/01/16
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, Eli Zaretskii, 2023/01/16
- bug#60557: [PATCH] Fix eglot prompt when connection already exists, Evgeni Kolev, 2023/01/18