bug#9867: 24.0.90; quit-window should provide quit-window-hook

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Christoph Scholtes]

A `quit-window-hook' should be provided for the `quit-window' function.

See discussion here:
http://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00919.html

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

Christoph Scholtes <cschol2112@googlemail.com> writes:

> A `quit-window-hook' should be provided for the `quit-window' function.
>
> See discussion here:
> http://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00919.html

(I'm going through old bug reports that have unfortunately gotten no
attention yet.)

It seems like Chong OK'd this hook at the time, but it was never added
to Emacs.  I've now added it to Emacs 27, because it seems like a useful
addition to have -- now modes that need to do some clean-up thing before
doing whatever `quit-window' does can just add that to the hook
(buffer-locally).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[martin rudalics]

 > It seems like Chong OK'd this hook at the time, but it was never added
 > to Emacs.  I've now added it to Emacs 27, because it seems like a useful
 > addition to have -- now modes that need to do some clean-up thing before
 > doing whatever `quit-window' does can just add that to the hook
 > (buffer-locally).

'quit-restore-window' can quit _any_ window not just the selected one.
This means that if a user puts something on a normal hook run by that
function, that something can only guess at which window is really quit
by that function.  Also, running a buffer-local value of that hook is
meaningless given the current implementation.  See the tribulations
run_window_configuration_change_hook runs into when trying to overcome
a similar problem (and note the unwind-protection overhead it incurs).

If people really need such a hook at all, please

- Make it either an abnormal hook run with the window quit as argument
   or run it from 'quit-window' but then only if it quits the selected
   window.

- Make sure to call any buffer-local value of that hook for the buffer
   of the window that is quit.

- Prefix it with 'quit-restore-' if you intend to run it from
   'quit-restore-window' (that function may be called by other
   functions as well).

- Fix the reference to the non-existent function 'quit-buffer'.

Thanks, martin

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 20 Aug 2019 10:19:29 +0200
> Cc: 9867@debbugs.gnu.org
> 
> If people really need such a hook at all, please
> 
> - Make it either an abnormal hook run with the window quit as argument
>    or run it from 'quit-window' but then only if it quits the selected
>    window.
> 
> - Make sure to call any buffer-local value of that hook for the buffer
>    of the window that is quit.
> 
> - Prefix it with 'quit-restore-' if you intend to run it from
>    'quit-restore-window' (that function may be called by other
>    functions as well).
> 
> - Fix the reference to the non-existent function 'quit-buffer'.

And also:

  - Add the new hook to the list in the node "Standard Hooks" in the
    ELisp manual.

Thanks.

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> If people really need such a hook at all, please
>
> - Make it either an abnormal hook run with the window quit as argument
>   or run it from 'quit-window' but then only if it quits the selected
>   window.

I was going back and forth on whether to call the hook from
`quit-window' or `quit-restore-window', so I ended up with invalid vode
in the latter.  Thinking about it a bit more, I think it makes more
sense to just run it from the former command -- other callers that call
`quit-restore-window' should perhaps call that hook manually (or not) at
all.

I'll move the hook over.

> - Fix the reference to the non-existent function 'quit-buffer'.

OK; done.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> And also:
>
>   - Add the new hook to the list in the node "Standard Hooks" in the
>     ELisp manual.

OK; done.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[martin rudalics]

 >> - Make it either an abnormal hook run with the window quit as argument
 >>    or run it from 'quit-window' but then only if it quits the selected
 >>    window.
 >
 > I was going back and forth on whether to call the hook from
 > `quit-window' or `quit-restore-window', so I ended up with invalid vode
 > in the latter.  Thinking about it a bit more, I think it makes more
 > sense to just run it from the former command -- other callers that call
 > `quit-restore-window' should perhaps call that hook manually (or not) at
 > all.
 >
 > I'll move the hook over.

But it's still misconfigured when WINDOW is not the selected window:
The function run by the hook would not know which window is quit and
which is its buffer.

 >> - Fix the reference to the non-existent function 'quit-buffer'.
 >
 > OK; done.

Thanks.

martin

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> But it's still misconfigured when WINDOW is not the selected window:
> The function run by the hook would not know which window is quit and
> which is its buffer.

Yup; should be fixed now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[martin rudalics]

 >> But it's still misconfigured when WINDOW is not the selected window:
 >> The function run by the hook would not know which window is quit and
 >> which is its buffer.
 >
 > Yup; should be fixed now.

The function on the hook still wouldn't reliably know which window was
quit, for example, when its buffer is displayed in two windows at the
same time.  Consider:

(let ((window (split-window)))
   (quit-window nil window))

martin

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> The function on the hook still wouldn't reliably know which window was
> quit, for example, when its buffer is displayed in two windows at the
> same time.  Consider:
>
> (let ((window (split-window)))
>   (quit-window nil window))

That's true, but is there a use case for the hook function having to
know what window it's quitting?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[martin rudalics]

 > That's true, but is there a use case for the hook function having to
 > know what window it's quitting?

I don't know.  But you could replace 'with-current-buffer' with
'with-selected-window' to avoid worrying about that (and better check
if WINDOW is not already selected before doing that internal state
saving rigmarole).  Or, as I suggested earlier, run the hook only when
quitting the selected window.

martin

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> That's true, but is there a use case for the hook function having to
>> know what window it's quitting?
>
> I don't know.  But you could replace 'with-current-buffer' with
> 'with-selected-window' to avoid worrying about that (and better check
> if WINDOW is not already selected before doing that internal state
> saving rigmarole).

I wasn't aware of `with-selected-window' -- it sounds a bit dramatic.
Does it have any side effects?

> Or, as I suggested earlier, run the hook only when
> quitting the selected window.

That's possible, but the semantics become perhaps a bit complicated?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[martin rudalics]

 > I wasn't aware of `with-selected-window' -- it sounds a bit dramatic.
 > Does it have any side effects?

Not normally.  It's wrapped in an unwind protection form like
'with-current-buffer'.  The following untested snippet should be (1)
fairly optimal for the normal case where the selected window is quit
and (2) guard against the case where the function run on the hook does
soemthing unexpected with the window configuration:

(let ((window (window-normalize-window window))
       (buffer (window-buffer window)))
   (if (and (eq window (selected-window))
	   (eq buffer (current-buffer)))
       (run-hooks 'quit-window-hook)
     ;; Select WINDOW for `quit-window-hook'.
     (with-selected-window window
       (run-hooks 'quit-window-hook)))
   ;; Run 'quit-restore-window' only if 'quit-window-hook' has left
   ;; WINDOW alone.
   (when (and (window-live-p window)
	     (eq (window-buffer window) buffer))
     (quit-restore-window window (if kill 'kill 'bury))))

Something like (2) is needed, for example, when a function run by the
hook kills WINDOW's buffer and 'kill-buffer' cleans up WINDOW by
deleting it which in its turn would cause 'quit-restore-window' act on
the window selected after 'replace-buffer-in-windows'.  For an amusing
example of why such a thing is necessary have a look at how often
'kill-buffer' checks whetheer the buffer it's supposed to kill is
still alive.

 >> Or, as I suggested earlier, run the hook only when
 >> quitting the selected window.
 >
 > That's possible, but the semantics become perhaps a bit complicated?

You would have to provide a somewhat disputable doc-string, indeed.

martin

bug#9867: 24.0.90; quit-window should provide quit-window-hook

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Not normally.  It's wrapped in an unwind protection form like
> 'with-current-buffer'.  The following untested snippet should be (1)
> fairly optimal for the normal case where the selected window is quit
> and (2) guard against the case where the function run on the hook does
> soemthing unexpected with the window configuration:
>
> (let ((window (window-normalize-window window))
>       (buffer (window-buffer window)))
>   (if (and (eq window (selected-window))
> 	   (eq buffer (current-buffer)))
>       (run-hooks 'quit-window-hook)
>     ;; Select WINDOW for `quit-window-hook'.
>     (with-selected-window window
>       (run-hooks 'quit-window-hook)))
>   ;; Run 'quit-restore-window' only if 'quit-window-hook' has left
>   ;; WINDOW alone.
>   (when (and (window-live-p window)
> 	     (eq (window-buffer window) buffer))
>     (quit-restore-window window (if kill 'kill 'bury))))

I think the semantics of the command becomes rather muddled with this
change.  We currently provide no guarantees for what or how the hook
function should do, or whether it basically disables the command if it
changes the window and so on (which is what this change will do).

If you think this should be what the semantics are, then I won't argue,
so please feel free to go ahead and make whatever changes you want here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no