GNU bug report logs - #70958
30.0.50; eglot-managed-mode hooks not called on shutdown

Previous Next

Package: emacs;

Reported by: Troy Brown <brownts <at> troybrown.dev>

Date: Wed, 15 May 2024 12:39:01 UTC

Severity: normal

Merged with 70835

Found in version 30.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 70958 in the body.
You can then email your comments to 70958 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Wed, 15 May 2024 12:39:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Troy Brown <brownts <at> troybrown.dev>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 15 May 2024 12:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Troy Brown <brownts <at> troybrown.dev>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; eglot-managed-mode hooks not called on shutdown
Date: Wed, 15 May 2024 08:38:28 -0400
The documentation for eglot-managed-mode-hook indicates that the hook
is run after Eglot has started/stopped managing a buffer.  I was
trying to use this to perform setup/teardown of functionality when
this happened, but it appears the registered hooks are never called on
shutdown.  The following is a little test that can be run in a buffer
which has LSP support, to demonstrate the problem.  My expectation is
that "Buffer not managed" is output and the my-eglot-hook-var is
changed to 'not-managed when a shutdown occurs.

--8<---------------cut here---------------start------------->8---
(defun my-eglot-test ()
  (defun my-eglot-hook ()
    (message "my-eglot-hook invoked")
    (if (eglot-managed-p)
        (progn
          (message "Buffer is managed")
          (setq-local my-eglot-hook-var 'managed))
      (message "Buffer not managed")
      (setq-local my-eglot-hook-var 'not-managed)))
  (add-hook 'eglot-managed-mode-hook #'my-eglot-hook)
  (setq-local my-eglot-hook-var 'initial)
  (cl-assert (not (eglot-managed-p)))
  (cl-assert (eq my-eglot-hook-var 'initial))
  (call-interactively #'eglot)
  (cl-assert (eglot-managed-p))
  (cl-assert (eq my-eglot-hook-var 'managed))
  (sleep-for 3) ; wait for server connection
  (call-interactively #'eglot-shutdown)
  (cl-assert (not (eglot-managed-p)))
  (cl-assert (eq my-eglot-hook-var 'not-managed)))
--8<---------------cut here---------------end--------------->8---




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Sat, 25 May 2024 07:54:01 GMT) Full text and rfc822 format available.

Message #8 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Troy Brown <brownts <at> troybrown.dev>, João Távora
 <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Sat, 25 May 2024 10:53:08 +0300
> From: Troy Brown <brownts <at> troybrown.dev>
> Date: Wed, 15 May 2024 08:38:28 -0400
> 
> The documentation for eglot-managed-mode-hook indicates that the hook
> is run after Eglot has started/stopped managing a buffer.  I was
> trying to use this to perform setup/teardown of functionality when
> this happened, but it appears the registered hooks are never called on
> shutdown.  The following is a little test that can be run in a buffer
> which has LSP support, to demonstrate the problem.  My expectation is
> that "Buffer not managed" is output and the my-eglot-hook-var is
> changed to 'not-managed when a shutdown occurs.
> 
> --8<---------------cut here---------------start------------->8---
> (defun my-eglot-test ()
>   (defun my-eglot-hook ()
>     (message "my-eglot-hook invoked")
>     (if (eglot-managed-p)
>         (progn
>           (message "Buffer is managed")
>           (setq-local my-eglot-hook-var 'managed))
>       (message "Buffer not managed")
>       (setq-local my-eglot-hook-var 'not-managed)))
>   (add-hook 'eglot-managed-mode-hook #'my-eglot-hook)
>   (setq-local my-eglot-hook-var 'initial)
>   (cl-assert (not (eglot-managed-p)))
>   (cl-assert (eq my-eglot-hook-var 'initial))
>   (call-interactively #'eglot)
>   (cl-assert (eglot-managed-p))
>   (cl-assert (eq my-eglot-hook-var 'managed))
>   (sleep-for 3) ; wait for server connection
>   (call-interactively #'eglot-shutdown)
>   (cl-assert (not (eglot-managed-p)))
>   (cl-assert (eq my-eglot-hook-var 'not-managed)))
> --8<---------------cut here---------------end--------------->8---

João, any comments or suggestions?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Sun, 26 May 2024 22:49:02 GMT) Full text and rfc822 format available.

Message #11 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Felician Nemeth <felician.nemeth <at> gmail.com>, 
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 70958 <at> debbugs.gnu.org, Troy Brown <brownts <at> troybrown.dev>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Sun, 26 May 2024 23:46:38 +0100
On Sat, May 25, 2024 at 8:53 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Troy Brown <brownts <at> troybrown.dev>
> > Date: Wed, 15 May 2024 08:38:28 -0400
> >
> > The documentation for eglot-managed-mode-hook indicates that the hook
> > is run after Eglot has started/stopped managing a buffer.  I was
> > trying to use this to perform setup/teardown of functionality when
> > this happened, but it appears the registered hooks are never called on
> > shutdown.  The following is a little test that can be run in a buffer
> > which has LSP support, to demonstrate the problem.  My expectation is
> > that "Buffer not managed" is output and the my-eglot-hook-var is
> > changed to 'not-managed when a shutdown occurs.
> >
> > --8<---------------cut here---------------start------------->8---
> > (defun my-eglot-test ()
> >   (defun my-eglot-hook ()
> >     (message "my-eglot-hook invoked")
> >     (if (eglot-managed-p)
> >         (progn
> >           (message "Buffer is managed")
> >           (setq-local my-eglot-hook-var 'managed))
> >       (message "Buffer not managed")
> >       (setq-local my-eglot-hook-var 'not-managed)))
> >   (add-hook 'eglot-managed-mode-hook #'my-eglot-hook)
> >   (setq-local my-eglot-hook-var 'initial)
> >   (cl-assert (not (eglot-managed-p)))
> >   (cl-assert (eq my-eglot-hook-var 'initial))
> >   (call-interactively #'eglot)
> >   (cl-assert (eglot-managed-p))
> >   (cl-assert (eq my-eglot-hook-var 'managed))
> >   (sleep-for 3) ; wait for server connection
> >   (call-interactively #'eglot-shutdown)
> >   (cl-assert (not (eglot-managed-p)))
> >   (cl-assert (eq my-eglot-hook-var 'not-managed)))
> > --8<---------------cut here---------------end--------------->8---
>
> João, any comments or suggestions?

eglot-managed-mode-hook is an abnormal minor mode hook because there
is no eglot-managed-mode minor mode, there is only eglot--managed-mode
which is a "--" definition on purpose (that does have the normal hooks
of course).

So when e-m-m-hook was added it was made to run only on "turn on"
because that's where it was most needed.  We can try changing Eglot
to also run it on "turn off", but that is a backward incompatible
change.

Alternatively, we can add a new eglot-managed-mode-off-hook.

Whatever the decision, there is the additional question of _when_ to
run the "off".  Maybe it's simple to  decide, but at least the the
"on" e-m -m-hook is specially  designed to run after some
LSP communication has taken  place,  which is of the  reasons
eglot--managed-mode-hook wasn't  suitable.  So maybe the "off"
should run in a similarly careful symmetrical position.

Finally, Troy can probably also make use of the internal
eglot--managed-mode-hook.  It's a "--", unsupported and dangerous
in theory, in practice it should be OK.

I'm CCing Felicián and Stefan for suggestions.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 12:31:01 GMT) Full text and rfc822 format available.

Message #14 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Troy Brown <brownts <at> troybrown.dev>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 08:29:40 -0400
On Sun, May 26, 2024 at 6:46 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> So when e-m-m-hook was added it was made to run only on "turn on"
> because that's where it was most needed.  We can try changing Eglot
> to also run it on "turn off", but that is a backward incompatible
> change.
>

If it was never intended to run on "turn off", then the comment string
for eglot-managed-mode-hook likely should be updated as it indicates
it is run when Eglot stops managing the buffer (i.e., "... run by
Eglot after it started/stopped managing a buffer").




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 12:37:01 GMT) Full text and rfc822 format available.

Message #17 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Troy Brown <brownts <at> troybrown.dev>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 13:35:13 +0100
On Mon, May 27, 2024 at 1:29 PM Troy Brown <brownts <at> troybrown.dev> wrote:
>
> On Sun, May 26, 2024 at 6:46 PM João Távora <joaotavora <at> gmail.com> wrote:
> >
> > So when e-m-m-hook was added it was made to run only on "turn on"
> > because that's where it was most needed.  We can try changing Eglot
> > to also run it on "turn off", but that is a backward incompatible
> > change.
> >
>
> If it was never intended to run on "turn off", then the comment string
> for eglot-managed-mode-hook likely should be updated as it indicates
> it is run when Eglot stops managing the buffer (i.e., "... run by
> Eglot after it started/stopped managing a buffer").

Ah, so this is only a doc bug and you don't actually need it?  If so,
fine with me, just provide a trivial patch for that and I'm sure some
will install it ASAP.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 12:47:02 GMT) Full text and rfc822 format available.

Message #20 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Troy Brown <brownts <at> troybrown.dev>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 08:45:58 -0400
On Mon, May 27, 2024 at 8:35 AM João Távora <joaotavora <at> gmail.com> wrote:
>
> Ah, so this is only a doc bug and you don't actually need it?  If so,
> fine with me, just provide a trivial patch for that and I'm sure some
> will install it ASAP.
>

I had tried to use it and found out it didn't work as expected, so I
took a different approach.  At this point, no I don't need it.  I did
however think it was a legitimate bug (not just a doc bug) and
reported it to prevent someone else from running into it in the
future.  If it's just a doc bug, that's fine, it wasn't clear to me.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 14:11:02 GMT) Full text and rfc822 format available.

Message #23 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Troy Brown <brownts <at> troybrown.dev>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 15:09:25 +0100
On Mon, May 27, 2024 at 1:46 PM Troy Brown <brownts <at> troybrown.dev> wrote:
> I did
> however think it was a legitimate bug (not just a doc bug) and

Bugs are only "legitimate" when they are harming someone somewhere.
This hook has been there for a number of years, and noone has complained
that I can remember. If you have a use for the on-shutdown, then it's
a bug.  It'd help to know about this use case. If you don't have a use,
it's just a doc bug, and patches welcome.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 14:33:02 GMT) Full text and rfc822 format available.

Message #26 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Troy Brown <brownts <at> troybrown.dev>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 10:32:01 -0400
On Mon, May 27, 2024 at 10:09 AM João Távora <joaotavora <at> gmail.com> wrote:
>
> Bugs are only "legitimate" when they are harming someone somewhere.
> This hook has been there for a number of years, and noone has complained
> that I can remember. If you have a use for the on-shutdown, then it's
> a bug.  It'd help to know about this use case. If you don't have a use,
> it's just a doc bug, and patches welcome.
>

The use case is that I was experimenting with updating the
buffer-local indent-region-function (and indirectly
indent-line-function) to be based on eglot-format when the buffer was
connected to the language server.  I was attempting to use the
eglot-managed-mode-hook so I could update these variables when the
Eglot buffer management changed.  Since the hook wasn't being called
on shutdown it would still attempt to call eglot-format when it was no
longer managing the buffer.  The workaround was to use a mode-specific
function for indent-region-function and then having that call
eglot-managed-p to determine if it should call eglot-format or
something else (e.g., indent-relative).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 15:47:02 GMT) Full text and rfc822 format available.

Message #29 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Troy Brown <brownts <at> troybrown.dev>, 
 GNU bug tracker automated control server <control <at> debbugs.gnu.org>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 16:45:01 +0100
merge 70958 70835
thanks

On Mon, May 27, 2024 at 3:32 PM Troy Brown <brownts <at> troybrown.dev> wrote:
>
> On Mon, May 27, 2024 at 10:09 AM João Távora <joaotavora <at> gmail.com> wrote:
> >
> > Bugs are only "legitimate" when they are harming someone somewhere.
> > This hook has been there for a number of years, and noone has complained
> > that I can remember. If you have a use for the on-shutdown, then it's
> > a bug.  It'd help to know about this use case. If you don't have a use,
> > it's just a doc bug, and patches welcome.

Actually I was wrong.  I have been recently warned of this exact
same issue.  I thought you and that person were the same.
bug#70835, which this bug is a duplicate of (so I've merged them,
hopefully)

> The use case is that I was experimenting with updating the
> buffer-local indent-region-function (and indirectly
> indent-line-function) to be based on eglot-format when the buffer was
> connected to the language server.  I was attempting to use the
> eglot-managed-mode-hook so I could update these variables when the
> Eglot buffer management changed.  Since the hook wasn't being called
> on shutdown it would still attempt to call eglot-format when it was no
> longer managing the buffer.  The workaround was to use a mode-specific
> function for indent-region-function and then having that call
> eglot-managed-p to determine if it should call eglot-format or
> something else (e.g., indent-relative).

Anyway, to your use case.  The "off" hook would solve your problem,
but not as well as your solution.  When setting variables, there's
no clean solution to the "undo problem", unless the variable in
question is a hook.  Think:

 var is originally X
 activate minor mode foo, saves var value of X, sets to Y,
 activate minor mode bar, that also sets var, saves Y, sets to Z
 deactivate foo, sets var to X
 deactivate bar, sets var to Y
 now both modes are inactive, variable is set to Y, in error

When the variable being affected is a hook with certain rules, this
problem doesn't exist.

Anyway, it's not a problem for Eglot to solve.  So given there is
also bug#70835 requesting the same, I think we can risk just running
eglot-managed-mode-hook like so let's try this patch:

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 6896baf30ce..2fab9e7f38b 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2059,6 +2059,7 @@ eglot--managed-mode
     (when eglot--current-flymake-report-fn
       (eglot--report-to-flymake nil)
       (setq eglot--current-flymake-report-fn nil))
+    (run-hooks 'eglot-managed-mode-hook)
     (let ((server eglot--cached-server))
       (setq eglot--cached-server nil)
       (when server

There will possibly be people complaining we broke their configs,
so this might not be the end of the story. But it's reasonable to try it
since this is how  the documentation says it _should_ work and is
consistent with the normal minor-mode hooks.

João




Merged 70835 70958. Request was from João Távora <joaotavora <at> gmail.com> to control <at> debbugs.gnu.org. (Mon, 27 May 2024 15:47:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 15:53:02 GMT) Full text and rfc822 format available.

Message #34 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Troy Brown <brownts <at> troybrown.dev>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 16:51:17 +0100
By the way Troy, can you show your indent-region-function based
on eglot-format? I once tried this and it didn't work very well.  With the
clangd server.  Would like to see your attempt.

João

On Mon, May 27, 2024 at 4:45 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> merge 70958 70835
> thanks
>
> On Mon, May 27, 2024 at 3:32 PM Troy Brown <brownts <at> troybrown.dev> wrote:
> >
> > On Mon, May 27, 2024 at 10:09 AM João Távora <joaotavora <at> gmail.com> wrote:
> > >
> > > Bugs are only "legitimate" when they are harming someone somewhere.
> > > This hook has been there for a number of years, and noone has complained
> > > that I can remember. If you have a use for the on-shutdown, then it's
> > > a bug.  It'd help to know about this use case. If you don't have a use,
> > > it's just a doc bug, and patches welcome.
>
> Actually I was wrong.  I have been recently warned of this exact
> same issue.  I thought you and that person were the same.
> bug#70835, which this bug is a duplicate of (so I've merged them,
> hopefully)
>
> > The use case is that I was experimenting with updating the
> > buffer-local indent-region-function (and indirectly
> > indent-line-function) to be based on eglot-format when the buffer was
> > connected to the language server.  I was attempting to use the
> > eglot-managed-mode-hook so I could update these variables when the
> > Eglot buffer management changed.  Since the hook wasn't being called
> > on shutdown it would still attempt to call eglot-format when it was no
> > longer managing the buffer.  The workaround was to use a mode-specific
> > function for indent-region-function and then having that call
> > eglot-managed-p to determine if it should call eglot-format or
> > something else (e.g., indent-relative).
>
> Anyway, to your use case.  The "off" hook would solve your problem,
> but not as well as your solution.  When setting variables, there's
> no clean solution to the "undo problem", unless the variable in
> question is a hook.  Think:
>
>  var is originally X
>  activate minor mode foo, saves var value of X, sets to Y,
>  activate minor mode bar, that also sets var, saves Y, sets to Z
>  deactivate foo, sets var to X
>  deactivate bar, sets var to Y
>  now both modes are inactive, variable is set to Y, in error
>
> When the variable being affected is a hook with certain rules, this
> problem doesn't exist.
>
> Anyway, it's not a problem for Eglot to solve.  So given there is
> also bug#70835 requesting the same, I think we can risk just running
> eglot-managed-mode-hook like so let's try this patch:
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 6896baf30ce..2fab9e7f38b 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -2059,6 +2059,7 @@ eglot--managed-mode
>      (when eglot--current-flymake-report-fn
>        (eglot--report-to-flymake nil)
>        (setq eglot--current-flymake-report-fn nil))
> +    (run-hooks 'eglot-managed-mode-hook)
>      (let ((server eglot--cached-server))
>        (setq eglot--cached-server nil)
>        (when server
>
> There will possibly be people complaining we broke their configs,
> so this might not be the end of the story. But it's reasonable to try it
> since this is how  the documentation says it _should_ work and is
> consistent with the normal minor-mode hooks.
>
> João



-- 
João Távora




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 16:24:02 GMT) Full text and rfc822 format available.

Message #37 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Felician Nemeth <felician.nemeth <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Troy Brown <brownts <at> troybrown.dev>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50; eglot-managed-mode hooks not called on
 shutdown
Date: Mon, 27 May 2024 18:21:47 +0200
João Távora <joaotavora <at> gmail.com> writes:

> So when e-m-m-hook was added it was made to run only on "turn on"

If I remember correctly, the original intention was to use the hook when
Eglot "turns off" as well.  Since e-m-m-hook is not a normal hook, it is
not straightforward to determine in which case the hook is called, so
the docstring helps: "Use `eglot-managed-p' to determine if current
buffer is managed."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 17:24:02 GMT) Full text and rfc822 format available.

Message #40 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Troy Brown <brownts <at> troybrown.dev>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 18:22:14 +0100
On Mon, May 27, 2024 at 5:21 PM Felician Nemeth
<felician.nemeth <at> gmail.com> wrote:
>
> João Távora <joaotavora <at> gmail.com> writes:
>
> > So when e-m-m-hook was added it was made to run only on "turn on"
>
> If I remember correctly, the original intention was to use the hook when
> Eglot "turns off" as well.

By "it was made to" I was just stating that that's how it turned
out and that's where it probably solved practical problems that
some user was facing.  IOW, I don't think there was ever a time
when it was ever run at "off" time.  But you're right the docstring
seems to indicate the intention was to replicate
eglot--managed-mode-hook. So that lends a bit more credibility
to the fix I proposed, I guess.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 17:37:01 GMT) Full text and rfc822 format available.

Message #43 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Felician Nemeth <felician.nemeth <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Troy Brown <brownts <at> troybrown.dev>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50; eglot-managed-mode hooks not called on
 shutdown
Date: Mon, 27 May 2024 19:35:09 +0200
João Távora <joaotavora <at> gmail.com> writes:

>> > So when e-m-m-hook was added it was made to run only on "turn on"
>>
> IOW, I don't think there was ever a time when it was ever run at "off"
> time.

I don't think it matters much, but originally it did run in both cases:
https://github.com/joaotavora/eglot/commit/8d8c90d0d654d262e0ff43f45444ec6a512b8185

So the current situation can be regarded as a regression.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 21:07:02 GMT) Full text and rfc822 format available.

Message #46 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Troy Brown <brownts <at> troybrown.dev>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 22:05:28 +0100
On Mon, May 27, 2024 at 6:35 PM Felician Nemeth
<felician.nemeth <at> gmail.com> wrote:
>
> João Távora <joaotavora <at> gmail.com> writes:
>
> >> > So when e-m-m-hook was added it was made to run only on "turn on"
> >>
> > IOW, I don't think there was ever a time when it was ever run at "off"
> > time.
>
> I don't think it matters much, but originally it did run in both cases:
> https://github.com/joaotavora/eglot/commit/8d8c90d0d654d262e0ff43f45444ec6a512b8185
>
> So the current situation can be regarded as a regression.

I think it does matter, it's one less thing to worry about if we
push that one-liner patch.  Can you comment on it, is it doing
more or less the same in your opinion?  I opted to put it before
before the server is shutdown, so that, like the on case, it has
access to a presumably fully setup server.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Mon, 27 May 2024 22:23:03 GMT) Full text and rfc822 format available.

Message #49 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Troy Brown <brownts <at> troybrown.dev>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Mon, 27 May 2024 23:21:29 +0100
On Mon, May 27, 2024 at 10:05 PM João Távora <joaotavora <at> gmail.com> wrote:

> > So the current situation can be regarded as a regression.
>
> I think it does matter, it's one less thing to worry about if we
> push that one-liner patch.  Can you comment on it, is it doing
> more or less the same in your opinion?  I opted to put it before
> before the server is shutdown, so that, like the on case, it has
> access to a presumably fully setup server.

I've just pushed the one-line patch.  I hope someone can test it
and then we can probably close this bug.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Tue, 28 May 2024 13:02:01 GMT) Full text and rfc822 format available.

Message #52 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Troy Brown <brownts <at> troybrown.dev>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Felician Nemeth <felician.nemeth <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Tue, 28 May 2024 09:00:47 -0400
On Mon, May 27, 2024 at 6:21 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> I've just pushed the one-line patch.  I hope someone can test it
> and then we can probably close this bug.
>

I ran the original test case I provided with this change and it now passes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#70958; Package emacs. (Sat, 01 Jun 2024 14:19:01 GMT) Full text and rfc822 format available.

Message #55 received at 70958 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Troy Brown <brownts <at> troybrown.dev>
Cc: 70958 <at> debbugs.gnu.org, felician.nemeth <at> gmail.com, joaotavora <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Sat, 01 Jun 2024 17:18:19 +0300
> From: Troy Brown <brownts <at> troybrown.dev>
> Date: Tue, 28 May 2024 09:00:47 -0400
> Cc: Felician Nemeth <felician.nemeth <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>, 
> 	Stefan Monnier <monnier <at> iro.umontreal.ca>, 70958 <at> debbugs.gnu.org
> 
> On Mon, May 27, 2024 at 6:21 PM João Távora <joaotavora <at> gmail.com> wrote:
> >
> > I've just pushed the one-line patch.  I hope someone can test it
> > and then we can probably close this bug.
> >
> 
> I ran the original test case I provided with this change and it now passes.

João, should this bug be closed now?




Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sat, 01 Jun 2024 16:02:02 GMT) Full text and rfc822 format available.

Notification sent to Troy Brown <brownts <at> troybrown.dev>:
bug acknowledged by developer. (Sat, 01 Jun 2024 16:02:02 GMT) Full text and rfc822 format available.

Message #60 received at 70958-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: 70958-done <at> debbugs.gnu.org
Subject: Re: bug#70958: 30.0.50;
 eglot-managed-mode hooks not called on shutdown
Date: Sat, 01 Jun 2024 19:01:17 +0300
> From: João Távora <joaotavora <at> gmail.com>
> Date: Sat, 1 Jun 2024 16:48:47 +0100
> 
> Yes, thanks

Thanks, done.




Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sat, 01 Jun 2024 16:02:02 GMT) Full text and rfc822 format available.

Notification sent to João Pedro <jpedrodeamorim <at> gmail.com>:
bug acknowledged by developer. (Sat, 01 Jun 2024 16:02:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 30 Jun 2024 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 14 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.