bug#32672: 27.0.50; image resize on window resizing

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

Like dynamically reformatting Man-mode buffers on window resizing in bug#32536
it would be useful to do the same for image-mode.  By default, when an image
is visited, it gets resized to fit into the window.  But on window shrinking,
parts of the image become truncated, and on window enlarging too much of
empty space is wasted since the image is not zoomed.

This patch relies on improvements in bug#32637.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: image-mode.1.patch --]
[-- Type: text/x-diff, Size: 1255 bytes --]

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 19fa28d440..9c7199ba9e 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -574,6 +574,7 @@ image-mode
 
 	(add-hook 'change-major-mode-hook 'image-toggle-display-text nil t)
 	(add-hook 'after-revert-hook 'image-after-revert-hook nil t)
+	(add-hook 'window-size-change-functions 'image-window-size-change nil t)
 	(run-mode-hooks 'image-mode-hook)
 	(let ((image (image-get-display-property))
 	      (msg1 (substitute-command-keys
@@ -822,6 +823,17 @@ image-after-revert-hook
           (get-buffer-window-list (current-buffer) 'nomini 'visible))
     (image-toggle-display-image)))
 
+(defun image-window-size-change (frame)
+  (walk-windows (lambda (window)
+                  (when (or (/= (window-pixel-width-before-size-change window)
+                                (window-pixel-width window))
+                            (/= (window-pixel-height-before-size-change window)
+                                (window-pixel-height window)))
+                    (with-current-buffer (window-buffer window)
+                      (when (derived-mode-p 'image-mode)
+                        (image-after-revert-hook)))))
+                'nomini frame))
+
 \f
 ;;; Animated images
 

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

> Like dynamically reformatting Man-mode buffers on window resizing in bug#32536
> it would be useful to do the same for image-mode.  By default, when an image
> is visited, it gets resized to fit into the window.  But on window shrinking,
> parts of the image become truncated, and on window enlarging too much of
> empty space is wasted since the image is not zoomed.

There is still a problem how to catch a moment when an image-mode buffer
is displayed after switching to it.  For example, when the selected window
is resized while it displayed a non-image-mode buffer, then after that
the current buffer switched to the image-mode buffer.  I can't find a hook
that could be activated to resize the image to fit the new sizes of the window.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > There is still a problem how to catch a moment when an image-mode buffer
 > is displayed after switching to it.

"Switching to a buffer" means to display it ...

 > For example, when the selected window
 > is resized while it displayed a non-image-mode buffer, then after that
 > the current buffer switched to the image-mode buffer.

... so please reformulate this in terms of a sequence of some sort of
pseudo actions.  In Emacs terminology, switching to a buffer implies
making it current but making a buffer current doesn't imply switching
to it.

 > I can't find a hook
 > that could be activated to resize the image to fit the new sizes of the window.

Tell us which action should trigger such a hook.

Thanks, martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> There is still a problem how to catch a moment when an image-mode buffer
>> is displayed after switching to it.
>
> "Switching to a buffer" means to display it ...
>
>> For example, when the selected window
>> is resized while it displayed a non-image-mode buffer, then after that
>> the current buffer switched to the image-mode buffer.
>
> ... so please reformulate this in terms of a sequence of some sort of
> pseudo actions.  In Emacs terminology, switching to a buffer implies
> making it current but making a buffer current doesn't imply switching
> to it.
>
>> I can't find a hook
>> that could be activated to resize the image to fit the new sizes of the window.
>
> Tell us which action should trigger such a hook.

It's not straightforward to find a hook that covers all cases, it seems
at least it should be called when an image-mode buffer comes to the top
of buffer-list, and the closest hook for that is buffer-list-update-hook.

I see that when it's buffer-local, then it guarantees that the affected
image-mode buffer is always current-buffer at the time of hook invocation.
It's fired every time the image-mode buffer comes to the top, even when
calling quit-window on another buffer that gives way to the image-mode buffer.

The only case when buffer-list-update-hook doesn't run is when the
image-mode buffer comes to the top by `C-x <C-left>' (previous-buffer),
whereas `C-x <right>' (next-buffer) works fine.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > It's not straightforward to find a hook that covers all cases, it seems
 > at least it should be called when an image-mode buffer comes to the top
 > of buffer-list, and the closest hook for that is buffer-list-update-hook.

I still don't get why you need to know whether the image-mode buffer
comes to top in order to decide on resizing the image.

 > I see that when it's buffer-local, then it guarantees that the affected
 > image-mode buffer is always current-buffer at the time of hook invocation.

There should not be any such guarantee.

 > It's fired every time the image-mode buffer comes to the top, even when
 > calling quit-window on another buffer that gives way to the image-mode buffer.
 >
 > The only case when buffer-list-update-hook doesn't run is when the
 > image-mode buffer comes to the top by `C-x <C-left>' (previous-buffer),
 > whereas `C-x <right>' (next-buffer) works fine.

This must be a bug but I don't understand how it could happen.
'switch-to-prev-buffer' calls 'set-window-buffer-start-and-point'
which calls 'set-window-buffer' which calls 'record-window-buffer'
which should call 'buffer-list-update-hook'.  Can you trace while it
fails for you?

Thanks, martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> It's not straightforward to find a hook that covers all cases, it seems
>> at least it should be called when an image-mode buffer comes to the top
>> of buffer-list, and the closest hook for that is buffer-list-update-hook.
>
> I still don't get why you need to know whether the image-mode buffer
> comes to top in order to decide on resizing the image.

For example, visit an image, type `C-h i', resize the window, type `C-x k RET'
(not `q', because `q' restores previous window sizes), then the image is not
updated to fit the resized window.

>> It's fired every time the image-mode buffer comes to the top, even when
>> calling quit-window on another buffer that gives way to the image-mode buffer.
>>
>> The only case when buffer-list-update-hook doesn't run is when the
>> image-mode buffer comes to the top by `C-x <C-left>' (previous-buffer),
>> whereas `C-x <right>' (next-buffer) works fine.
>
> This must be a bug but I don't understand how it could happen.
> 'switch-to-prev-buffer' calls 'set-window-buffer-start-and-point'
> which calls 'set-window-buffer' which calls 'record-window-buffer'
> which should call 'buffer-list-update-hook'.  Can you trace while it
> fails for you?

Trying to put to image-mode:

	(add-hook 'buffer-list-update-hook
                  (lambda () (message "buffer-list-update-hook %S"
                                      (current-buffer))) nil t)

shows in *Messages* that it's called only when navigating away from the
image-mode buffer, but not when coming back to the image-mode buffer, e.g.
`C-x <left>', then resize the window, then type `C-x <right>',
it's not called.  Also it's not called in another direction:
`C-x <right>', then resize the window, then type `C-x <left>'.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > Trying to put to image-mode:
 >
 > 	(add-hook 'buffer-list-update-hook
 >                    (lambda () (message "buffer-list-update-hook %S"
 >                                        (current-buffer))) nil t)
 >
 > shows in *Messages* that it's called only when navigating away from the
 > image-mode buffer, but not when coming back to the image-mode buffer, e.g.
 > `C-x <left>', then resize the window, then type `C-x <right>',
 > it's not called.  Also it's not called in another direction:
 > `C-x <right>', then resize the window, then type `C-x <left>'.

I forgot that you wanted to make this buffer-local as well.  We
curently have no special semantics attached to the buffer-local
version of this so yes: When the buffer is not current, its local hook
is not run.  To fix this, for example in 'bury-buffer-internal' we
would have to add an extra

   /* Run buffer-list-update-hook.  */
   if (!NILP (Vrun_hooks))
     call1 (Vrun_hooks, Qbuffer_list_update_hook);

with BUFFER current.  We probably could do that but I'm not very fond
of it.  The buffer list is decidedly more global than a window.  And
it's only the tip of an iceberg - how many more hooks would we have to
adapt in a similar fashion?

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> Trying to put to image-mode:
>>
>> 	(add-hook 'buffer-list-update-hook
>>                    (lambda () (message "buffer-list-update-hook %S"
>>                                        (current-buffer))) nil t)
>>
>> shows in *Messages* that it's called only when navigating away from the
>> image-mode buffer, but not when coming back to the image-mode buffer, e.g.
>> `C-x <left>', then resize the window, then type `C-x <right>',
>> it's not called.  Also it's not called in another direction:
>> `C-x <right>', then resize the window, then type `C-x <left>'.
>
> I forgot that you wanted to make this buffer-local as well.  We
> curently have no special semantics attached to the buffer-local
> version of this so yes: When the buffer is not current, its local hook
> is not run.  To fix this, for example in 'bury-buffer-internal' we
> would have to add an extra
>
>   /* Run buffer-list-update-hook.  */
>   if (!NILP (Vrun_hooks))
>     call1 (Vrun_hooks, Qbuffer_list_update_hook);
>
> with BUFFER current.  We probably could do that but I'm not very fond
> of it.  The buffer list is decidedly more global than a window.  And
> it's only the tip of an iceberg - how many more hooks would we have to
> adapt in a similar fashion?

I agree that the buffer list is more global than a window, but OTOH
it makes sense to use buffer-local hook for a buffer to get notified
when it comes to the top of the buffer list.  So this would be a good thing
to have, unless there is another hook that gets called when a buffer
becomes the current buffer, or at least displayed in a window.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

 > I agree that the buffer list is more global than a window, but OTOH
 > it makes sense to use buffer-local hook for a buffer to get notified
 > when it comes to the top of the buffer list.  So this would be a good thing
 > to have, unless there is another hook that gets called when a buffer
 > becomes the current buffer, or at least displayed in a window.

I try to fix the problem you mentioned earlier, namely that

 > > it's called only when navigating away from the
 > > image-mode buffer, but not when coming back to the image-mode buffer, e.g.
 > > `C-x <left>', then resize the window, then type `C-x <right>',
 > > it's not called.  Also it's not called in another direction:
 > > `C-x <right>', then resize the window, then type `C-x <left>'.

Please tell me if the attached patch works and what else should be
done.

Thanks, martin

[-- Attachment #2: buffer-list-update-hook.diff --]
[-- Type: text/plain, Size: 2719 bytes --]

--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4301,9 +4301,9 @@ set-window-buffer-start-and-point
   (setq window (window-normalize-window window t))
   (let ((selected (eq window (selected-window)))
 	(current (eq (window-buffer window) (current-buffer))))
-    (set-window-buffer window buffer)
     (when (and selected current)
       (set-buffer buffer))
+    (set-window-buffer window buffer)
     (when start
       ;; Don't force window-start here (even if POINT is nil).
       (set-window-start window start t))
diff --git a/src/window.c b/src/window.c
index 409b01f..33e1975 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3573,40 +3573,36 @@ depends on the value of (window-start WINDOW), so if calling this
 
 This function runs `window-scroll-functions' before running
 `window-configuration-change-hook'.  */)
-  (register Lisp_Object window, Lisp_Object buffer_or_name, Lisp_Object keep_margins)
+     (Lisp_Object window, Lisp_Object buffer_or_name, Lisp_Object keep_margins)
 {
-  register Lisp_Object tem, buffer;
-  register struct window *w = decode_live_window (window);
+  Lisp_Object old_buffer, new_buffer;
+  struct window *w = decode_live_window (window);
 
   XSETWINDOW (window, w);
-  buffer = Fget_buffer (buffer_or_name);
-  CHECK_BUFFER (buffer);
-  if (!BUFFER_LIVE_P (XBUFFER (buffer)))
+  old_buffer = w->contents;
+  new_buffer = Fget_buffer (buffer_or_name);
+  CHECK_BUFFER (new_buffer);
+  if (!BUFFER_LIVE_P (XBUFFER (new_buffer)))
     error ("Attempt to display deleted buffer");
 
-  tem = w->contents;
-  if (NILP (tem))
-    error ("Window is deleted");
-  else
+  if (!EQ (old_buffer, new_buffer))
     {
-      if (!EQ (tem, buffer))
-	{
-	  if (EQ (w->dedicated, Qt))
-	    /* WINDOW is strongly dedicated to its buffer, signal an
-	       error.  */
-	    error ("Window is dedicated to `%s'", SDATA (BVAR (XBUFFER (tem), name)));
-	  else
-	    /* WINDOW is weakly dedicated to its buffer, reset
-	       dedication.  */
-	    wset_dedicated (w, Qnil);
-
-	  call1 (Qrecord_window_buffer, window);
-	}
+      if (EQ (w->dedicated, Qt))
+	/* WINDOW is strongly dedicated to its buffer, signal an
+	   error.  */
+	error ("Window is dedicated to `%s'", SDATA (BVAR (XBUFFER (old_buffer), name)));
+      else
+	/* WINDOW is weakly dedicated to its buffer, reset
+	   dedication.  */
+	wset_dedicated (w, Qnil);
 
       unshow_buffer (w);
     }
 
-  set_window_buffer (window, buffer, true, !NILP (keep_margins));
+  set_window_buffer (window, new_buffer, true, !NILP (keep_margins));
+
+  if (!EQ (old_buffer, new_buffer))
+    call1 (Qrecord_window_buffer, window);
 
   return Qnil;
 }

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> I agree that the buffer list is more global than a window, but OTOH
>> it makes sense to use buffer-local hook for a buffer to get notified
>> when it comes to the top of the buffer list.  So this would be a good thing
>> to have, unless there is another hook that gets called when a buffer
>> becomes the current buffer, or at least displayed in a window.
>
> I try to fix the problem you mentioned earlier, namely that
>
>> > it's called only when navigating away from the
>> > image-mode buffer, but not when coming back to the image-mode buffer, e.g.
>> > `C-x <left>', then resize the window, then type `C-x <right>',
>> > it's not called.  Also it's not called in another direction:
>> > `C-x <right>', then resize the window, then type `C-x <left>'.
>
> Please tell me if the attached patch works and what else should be
> done.

I tried but the hook still not called.  Moreover, it broke something:
after `C-x <right>' I need to type `C-x <left>' twice to get back
to the original buffer.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > I tried but the hook still not called.  Moreover, it broke something:
 > after `C-x <right>' I need to type `C-x <left>' twice to get back
 > to the original buffer.

Suppose with emacs -Q I load the following code:

(defvar count 0)

(defun foo ()
   (setq count (1+ count))
   (message "%s ... current: %s ... window-buffer: %s"
	   count (current-buffer) (window-buffer)))

(with-current-buffer "*scratch*"
   (add-hook 'buffer-list-update-hook 'foo nil t))

This gets me an initial

1 ... current: *scratch* ... window-buffer: *scratch*

When I now type C-x <right> and then C-x <left> I get

2 ... current: *scratch* ... window-buffer: *scratch*

and my window shows *scratch*.  What do you get?

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> I tried but the hook still not called.  Moreover, it broke something:
>> after `C-x <right>' I need to type `C-x <left>' twice to get back
>> to the original buffer.
>
> Suppose with emacs -Q I load the following code:
>
> (defvar count 0)
>
> (defun foo ()
>   (setq count (1+ count))
>   (message "%s ... current: %s ... window-buffer: %s"
> 	   count (current-buffer) (window-buffer)))
>
> (with-current-buffer "*scratch*"
>   (add-hook 'buffer-list-update-hook 'foo nil t))
>
> This gets me an initial
>
> 1 ... current: *scratch* ... window-buffer: *scratch*
>
> When I now type C-x <right> and then C-x <left> I get
>
> 2 ... current: *scratch* ... window-buffer: *scratch*
>
> and my window shows *scratch*.  What do you get?

Please try with more buffers than initial 2 (*scratch* and *Messages*),
e.g. with *info* (just `C-h i'), I get

1 ... current: *scratch* ... window-buffer: *scratch*
2 ... current: *scratch* ... window-buffer: *info*

which is in an inconsistent state,
and after typing `q' the hook is called 3 times

3 ... current: *scratch* ... window-buffer: *scratch*
4 ... current: *scratch* ... window-buffer: *scratch*
5 ... current: *scratch* ... window-buffer: *scratch*

And there is the workflow how to break the correct order
of `C-x <right> C-x <left>':

1. Create *info* buffer: `C-h i' and quit `q'
2. Split windows `C-x 2' or `C-x 3' (both windows display *scratch*)
3. Check that `C-x <right> C-x <left>' correctly returns to the original
   buffer *scratch* in the first window.
4. After `C-x o' in another window `C-x <right> C-x <left>'
   doesn't return to the original buffer *scratch*.
   It displays *info*.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 3771 bytes --]

 > Please try with more buffers than initial 2 (*scratch* and *Messages*),
 > e.g. with *info* (just `C-h i'), I get
 >
 > 1 ... current: *scratch* ... window-buffer: *scratch*

... which is something you probably don't even want - it comes from
'get-buffer-create' when making the *info* buffer ...

 > 2 ... current: *scratch* ... window-buffer: *info*
 >
 > which is in an inconsistent state,

... not really - look at the backtrace:

   foo()
   run-hooks(buffer-list-update-hook)
   record-window-buffer(#<window 3 on *info*>)
   set-window-buffer(#<window 3 on *info*> #<buffer *info*>)
   window--display-buffer(#<buffer *info*> #<window 3 on *info*> reuse ((inhibit-same-window)))
   display-buffer-same-window(#<buffer *info*> ((inhibit-same-window)))
   display-buffer(#<buffer *info*> (display-buffer-same-window (inhibit-same-window)))
   pop-to-buffer("*info*" (display-buffer-same-window (inhibit-same-window)) nil)
   pop-to-buffer-same-window("*info*")
   info(nil nil)
   funcall-interactively(info nil nil)
   call-interactively(info nil nil)
   command-execute(info)

'pop-to-buffer' can "reasonably" select the window and thus implicitly
make its buffer current only _after_ the buffer has been displayed via
'display-buffer-same-window'.  I say reasonably because we could make
the buffer current earlier but this doesn't strike me as a very good
idea.  It would constitute an incompatible change in the internal
behavior of 'pop-to-buffer'.

 > and after typing `q' the hook is called 3 times
 >
 > 3 ... current: *scratch* ... window-buffer: *scratch*
 > 4 ... current: *scratch* ... window-buffer: *scratch*
 > 5 ... current: *scratch* ... window-buffer: *scratch*

Look at the backtraces:

3 ... current: *scratch* ... window-buffer: *scratch*
   run-hooks(buffer-list-update-hook)
   record-window-buffer(#<window 3 on *scratch*>)
   set-window-buffer(#<window 3 on *scratch*> #<buffer *scratch*>)
   set-window-buffer-start-and-point(#<window 3 on *scratch*> #<buffer *scratch*> 1 #<marker at 1598 in *scratch*>)
   quit-restore-window(nil bury)
   quit-window()
   Info-exit()
   funcall-interactively(Info-exit)
   call-interactively(Info-exit nil nil)
   command-execute(Info-exit)

This one comes from replacing the *info* buffer with *scratch* in the
window.

4 ... current: *scratch* ... window-buffer: *scratch*
   run-hooks(buffer-list-update-hook)
   select-window(#<window 3 on *scratch*>)
   quit-restore-window(nil bury)
   quit-window()
   Info-exit()
   funcall-interactively(Info-exit)
   call-interactively(Info-exit nil nil)
   command-execute(Info-exit)

This one comes from selecting that window.

5 ... current: *scratch* ... window-buffer: *scratch*
   run-hooks(buffer-list-update-hook)
   bury-buffer-internal(#<buffer *info*>)
   quit-restore-window(nil bury)
   quit-window()
   Info-exit()
   funcall-interactively(Info-exit)
   call-interactively(Info-exit nil nil)
   command-execute(Info-exit)

And this one comes from burying the *info* buffer.  What should we do
instead?

 > And there is the workflow how to break the correct order
 > of `C-x <right> C-x <left>':
 >
 > 1. Create *info* buffer: `C-h i' and quit `q'
 > 2. Split windows `C-x 2' or `C-x 3' (both windows display *scratch*)
 > 3. Check that `C-x <right> C-x <left>' correctly returns to the original
 >     buffer *scratch* in the first window.
 > 4. After `C-x o' in another window `C-x <right> C-x <left>'
 >     doesn't return to the original buffer *scratch*.
 >     It displays *info*.

Evidently, this comment for 'record-window-buffer'

;; The following function is called by `set-window-buffer' _before_ it
;; replaces the buffer of the argument window with the new buffer.

had its purpose.  Please try the attached patch.

Thanks, martin

[-- Attachment #2: buffer-list-update-hook.diff --]
[-- Type: text/plain, Size: 3585 bytes --]

--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4274,9 +4274,7 @@ record-window-buffer
 			       ;; (Bug#12588).
 			       point window-point-insertion-type)))))
 	  (set-window-prev-buffers
-	   window (cons entry (window-prev-buffers window)))))
-
-      (run-hooks 'buffer-list-update-hook))))
+	   window (cons entry (window-prev-buffers window))))))))
 
 (defun unrecord-window-buffer (&optional window buffer)
   "Unrecord BUFFER in WINDOW.
@@ -4301,9 +4299,9 @@ set-window-buffer-start-and-point
   (setq window (window-normalize-window window t))
   (let ((selected (eq window (selected-window)))
 	(current (eq (window-buffer window) (current-buffer))))
-    (set-window-buffer window buffer)
     (when (and selected current)
       (set-buffer buffer))
+    (set-window-buffer window buffer)
     (when start
       ;; Don't force window-start here (even if POINT is nil).
       (set-window-start window start t))
diff --git a/src/window.c b/src/window.c
index 409b01f..d765564 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3573,40 +3573,38 @@ depends on the value of (window-start WINDOW), so if calling this
 
 This function runs `window-scroll-functions' before running
 `window-configuration-change-hook'.  */)
-  (register Lisp_Object window, Lisp_Object buffer_or_name, Lisp_Object keep_margins)
+     (Lisp_Object window, Lisp_Object buffer_or_name, Lisp_Object keep_margins)
 {
-  register Lisp_Object tem, buffer;
-  register struct window *w = decode_live_window (window);
+  Lisp_Object old_buffer, new_buffer;
+  struct window *w = decode_live_window (window);
 
   XSETWINDOW (window, w);
-  buffer = Fget_buffer (buffer_or_name);
-  CHECK_BUFFER (buffer);
-  if (!BUFFER_LIVE_P (XBUFFER (buffer)))
+  old_buffer = w->contents;
+  new_buffer = Fget_buffer (buffer_or_name);
+  CHECK_BUFFER (new_buffer);
+  if (!BUFFER_LIVE_P (XBUFFER (new_buffer)))
     error ("Attempt to display deleted buffer");
 
-  tem = w->contents;
-  if (NILP (tem))
-    error ("Window is deleted");
-  else
+  if (!EQ (old_buffer, new_buffer))
     {
-      if (!EQ (tem, buffer))
-	{
-	  if (EQ (w->dedicated, Qt))
-	    /* WINDOW is strongly dedicated to its buffer, signal an
-	       error.  */
-	    error ("Window is dedicated to `%s'", SDATA (BVAR (XBUFFER (tem), name)));
-	  else
-	    /* WINDOW is weakly dedicated to its buffer, reset
-	       dedication.  */
-	    wset_dedicated (w, Qnil);
-
-	  call1 (Qrecord_window_buffer, window);
-	}
+      if (EQ (w->dedicated, Qt))
+	/* WINDOW is strongly dedicated to its buffer, signal an
+	   error.  */
+	error ("Window is dedicated to `%s'", SDATA (BVAR (XBUFFER (old_buffer), name)));
+      else
+	/* WINDOW is weakly dedicated to its buffer, reset
+	   dedication.  */
+	wset_dedicated (w, Qnil);
 
-      unshow_buffer (w);
+      call1 (Qrecord_window_buffer, window);
     }
 
-  set_window_buffer (window, buffer, true, !NILP (keep_margins));
+  unshow_buffer (w);
+
+  set_window_buffer (window, new_buffer, true, !NILP (keep_margins));
+
+  if (!NILP (Vrun_hooks) && !EQ (old_buffer, new_buffer))
+    call1 (Vrun_hooks, Qbuffer_list_update_hook);
 
   return Qnil;
 }
@@ -6415,7 +6413,12 @@ struct saved_window
 	      && BUFFER_LIVE_P (XBUFFER (p->buffer)))
 	    /* If a window we restore gets another buffer, record the
 	       window's old buffer.  */
-	    call1 (Qrecord_window_buffer, window);
+	    {
+	      call1 (Qrecord_window_buffer, window);
+
+	      if (!NILP (Vrun_hooks))
+		call1 (Vrun_hooks, Qbuffer_list_update_hook);
+	    }
 	}
 
       /* Disallow x_set_window_size, temporarily.  */

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> 2 ... current: *scratch* ... window-buffer: *info*
>>
>> which is in an inconsistent state,
>
> ... not really - look at the backtrace:

But the problem is that the buffer-local hook will have such idiomatic code:

  (with-current-buffer (window-buffer window)
    ...

where window-buffer will return a wrong buffer.

>> and after typing `q' the hook is called 3 times
>>
>> 3 ... current: *scratch* ... window-buffer: *scratch*
>> 4 ... current: *scratch* ... window-buffer: *scratch*
>> 5 ... current: *scratch* ... window-buffer: *scratch*
>
> Look at the backtraces:
>
> And this one comes from burying the *info* buffer.  What should we do
> instead?

I don't know.  In this particular feature request I'm going to use debounce,
but for general case I don't see how to optimize it.

>> And there is the workflow how to break the correct order
>> of `C-x <right> C-x <left>':
>>
>> 1. Create *info* buffer: `C-h i' and quit `q'
>> 2. Split windows `C-x 2' or `C-x 3' (both windows display *scratch*)
>> 3. Check that `C-x <right> C-x <left>' correctly returns to the original
>>     buffer *scratch* in the first window.
>> 4. After `C-x o' in another window `C-x <right> C-x <left>'
>>     doesn't return to the original buffer *scratch*.
>>     It displays *info*.
>
> Evidently, this comment for 'record-window-buffer'
>
> ;; The following function is called by `set-window-buffer' _before_ it
> ;; replaces the buffer of the argument window with the new buffer.
>
> had its purpose.  Please try the attached patch.

Thanks, now the problem is not reproducible.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >>> 2 ... current: *scratch* ... window-buffer: *info*
 >>>
 >>> which is in an inconsistent state,
 >>
 >> ... not really - look at the backtrace:
 >
 > But the problem is that the buffer-local hook will have such idiomatic code:
 >
 >    (with-current-buffer (window-buffer window)
 >      ...

Where does 'window' come from?  At the time 'buffer-list-update-hook'
gets called, there's no guarantee that the buffer whose local hook
runs is even shown in a window.

 > where window-buffer will return a wrong buffer.

It's the right buffer at the time you run this hook.  I'm afraid that
whatever we do here it will get us in a mess that strikes back sooner
or later.  Maybe we really should run a 'select-window-hook' instead.

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> But the problem is that the buffer-local hook will have such idiomatic code:
>>
>>    (with-current-buffer (window-buffer window)
>>      ...
>
> Where does 'window' come from?  At the time 'buffer-list-update-hook'
> gets called, there's no guarantee that the buffer whose local hook
> runs is even shown in a window.

Thank you, now it's possible to finish this feature,
it gets 'window' using get-buffer-window-list:

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 19fa28d440..3355ee6109 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -574,6 +574,9 @@ image-mode
 
 	(add-hook 'change-major-mode-hook 'image-toggle-display-text nil t)
 	(add-hook 'after-revert-hook 'image-after-revert-hook nil t)
+	(add-hook 'window-size-change-functions (debounce 'image-size-window-change 1) nil t)
+	(add-hook 'buffer-list-update-hook 'image-size-buffer-change nil t)
+
 	(run-mode-hooks 'image-mode-hook)
 	(let ((image (image-get-display-property))
 	      (msg1 (substitute-command-keys
@@ -822,6 +826,42 @@ image-after-revert-hook
           (get-buffer-window-list (current-buffer) 'nomini 'visible))
     (image-toggle-display-image)))
 
+(defun image-size-window-change (&optional frame)
+  (let (buffers)
+    (walk-windows (lambda (window)
+                    (with-current-buffer (window-buffer window)
+                      (when (derived-mode-p 'image-mode)
+                        (push (current-buffer) buffers))))
+                  'nomini (or frame 'visible))
+    (mapc (lambda (buffer) (image-size-buffer-change buffer))
+          (delq nil (delete-dups buffers)))))
+
+(defun image-size-buffer-change (&optional buffer)
+  (with-current-buffer (or buffer (current-buffer))
+    (when (derived-mode-p 'image-mode)
+      (let* ((spec (image-get-display-property))
+             (spec (and (eq (car-safe spec) 'image) spec))
+             (max-width  (plist-get (cdr spec) :max-width))
+             (max-height (plist-get (cdr spec) :max-height))
+             (windows (and (or max-width max-height)
+                           (get-buffer-window-list
+                            (current-buffer) 'nomini 'visible)))
+             min-width min-height min-window)
+        (mapc (lambda (window)
+                (let* ((edges (window-inside-pixel-edges window))
+                       (width  (- (nth 2 edges) (nth 0 edges)))
+		       (height (- (nth 3 edges) (nth 1 edges))))
+                  (setq min-width  (if min-width  (min min-width  width)  width))
+                  (setq min-height (if min-height (min min-height height) height))
+                  (when (and (= min-width width)
+                             (= min-height height))
+                    (setq min-window window))))
+              windows)
+        (when (or (and min-width  max-width  (not (= min-width  max-width)))
+                  (and min-height max-height (not (= min-height max-height))))
+          (with-selected-window min-window
+            (image-toggle-display-image)))))))
+
 \f
 ;;; Animated images
 

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >   	(add-hook 'change-major-mode-hook 'image-toggle-display-text nil t)
 >   	(add-hook 'after-revert-hook 'image-after-revert-hook nil t)
 > +	(add-hook 'window-size-change-functions (debounce 'image-size-window-change 1) nil t)
 > +	(add-hook 'buffer-list-update-hook 'image-size-buffer-change nil t)

Shouldn't you debounce the call to 'image-size-buffer-change' as well?
After all, the buffer list seems to change more often than the size of
windows.  In either case you probably want me to install the
'buffer-list-update-hook' changes.  Correct?

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>>   	(add-hook 'change-major-mode-hook 'image-toggle-display-text nil t)
>>   	(add-hook 'after-revert-hook 'image-after-revert-hook nil t)
>> +	(add-hook 'window-size-change-functions (debounce 'image-size-window-change 1) nil t)
>> +	(add-hook 'buffer-list-update-hook 'image-size-buffer-change nil t)
>
> Shouldn't you debounce the call to 'image-size-buffer-change' as well?
> After all, the buffer list seems to change more often than the size of
> windows.

When I tried debounce for buffer-list-update-hook, it used the last call
in sequence:

1 ... current: *scratch* ... window-buffer: *scratch*
2 ... current: *scratch* ... window-buffer: *info*

that was unsuitable for the logic of 'image-size-buffer-change'.

So maybe instead of 'debounce', we need to implement and use 'throttle'
or at least implement the 'immediate' option that uses the first call:

https://css-tricks.com/debouncing-throttling-explained-examples/

> In either case you probably want me to install the
> 'buffer-list-update-hook' changes.  Correct?

Yes, please install it, if you see no more regressions.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >> Shouldn't you debounce the call to 'image-size-buffer-change' as well?
 >> After all, the buffer list seems to change more often than the size of
 >> windows.
 >
 > When I tried debounce for buffer-list-update-hook, it used the last call
 > in sequence:
 >
 > 1 ... current: *scratch* ... window-buffer: *scratch*
 > 2 ... current: *scratch* ... window-buffer: *info*
 >
 > that was unsuitable for the logic of 'image-size-buffer-change'.

I'm afraid that 'buffer-list-update-hook' is too coarse for your
purpose - it gets called too often for unrelated events like creating
or killing some completely unrelated buffer.  If you think that
running a hook after selecting a window with NORECORD nil would fit
the bill better, let's try it.  Many people have asked for such a hook
and we always rejected it (Bug#7381, Bug#16436).  Maybe it is time to
reconsider.

 > So maybe instead of 'debounce', we need to implement and use 'throttle'
 > or at least implement the 'immediate' option that uses the first call:
 >
 > https://css-tricks.com/debouncing-throttling-explained-examples/

We'd probably need to come up with examples from our code base in
order to decide what to implement and how.

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

> I'm afraid that 'buffer-list-update-hook' is too coarse for your
> purpose - it gets called too often for unrelated events like creating
> or killing some completely unrelated buffer.

I agree that 'buffer-list-update-hook' is unsuitable for this task.

> If you think that running a hook after selecting a window with
> NORECORD nil would fit the bill better, let's try it.  Many people
> have asked for such a hook and we always rejected it (Bug#7381,
> Bug#16436).  Maybe it is time to reconsider.

In (info "(elisp) Selecting Windows") I see explanations why
there is no separate hook whenever a window gets selected.

But when I tried

  (advice-add 'select-window :before
              (lambda (window &optional norecord)
                (message "select-window %S" window)))

it's clear it's unsuitable either.  It's not called when needed,
e.g. when a window displays a new buffer after navigating to it
with 'C-x <C-left>' (previous-buffer).

However, I get exactly what is needed with

  (advice-add 'set-window-buffer :before
              (lambda (window buffer-or-name &optional keep-margins)
                (message "set-window-buffer %S %S" window buffer-or-name)))

It's called every time when a buffer is displayed in a window.

But unfortunately it has no hook, and (info "(elisp) Buffers and Windows") says
that set-window-buffer runs window-configuration-change-hook (too general
for this task since called too often) and window-scroll-functions
(also called too often).

Regarding window-scroll-functions, it would be too strange to use
it to catch set-window-buffer calls.

(info "(elisp) Window Hooks") says:

  There are three actions that can change this: scrolling the window,
  switching buffers in the window, and changing the size of the window.
  The first two actions run ‘window-scroll-functions’; the last runs
  ‘window-size-change-functions’.

Shouldn't the first two actions run separate hooks?

Moreover, it seems window-scroll-functions doesn't work even
for its purpose: it's not called after scrolling, e.g. not called
after 'C-l' (recenter-top-bottom) - tried with different prefix args.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > But when I tried
 >
 >    (advice-add 'select-window :before
 >                (lambda (window &optional norecord)
 >                  (message "select-window %S" window)))
 >
 > it's clear it's unsuitable either.  It's not called when needed,
 > e.g. when a window displays a new buffer after navigating to it
 > with 'C-x <C-left>' (previous-buffer).

Because the selected window does not change, obviously.

 > However, I get exactly what is needed with
 >
 >    (advice-add 'set-window-buffer :before
 >                (lambda (window buffer-or-name &optional keep-margins)
 >                  (message "set-window-buffer %S %S" window buffer-or-name)))
 >
 > It's called every time when a buffer is displayed in a window.
 >
 > But unfortunately it has no hook, and (info "(elisp) Buffers and Windows") says
 > that set-window-buffer runs window-configuration-change-hook (too general
 > for this task since called too often)

Because we run it also whenever a window changes size which is silly.
What else do we have 'window-size-change-functions' for?  I wanted to
change that but apparently ducked out.

 > and window-scroll-functions
 > (also called too often).

 > Regarding window-scroll-functions, it would be too strange to use
 > it to catch set-window-buffer calls.
 >
 > (info "(elisp) Window Hooks") says:
 >
 >    There are three actions that can change this: scrolling the window,
 >    switching buffers in the window, and changing the size of the window.
 >    The first two actions run ‘window-scroll-functions’; the last runs
 >    ‘window-size-change-functions’.
 >
 > Shouldn't the first two actions run separate hooks?
 >
 > Moreover, it seems window-scroll-functions doesn't work even
 > for its purpose: it's not called after scrolling, e.g. not called
 > after 'C-l' (recenter-top-bottom) - tried with different prefix args.

Maybe because the buffer of the scrolled window is not current when
you call it.

I don't know what to do.  We run hooks too often and do not provide
sufficient information when running them.  Sometimes we even hide
information when running a hook.  The problem is that changing the
current situation will have us either run even more hooks or cause
protests when a hook is no more run where it was run before.

martin

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> Date: Mon, 24 Sep 2018 10:22:45 +0200
> From: martin rudalics <rudalics@gmx.at>
> Cc: 32672@debbugs.gnu.org
> 
> I don't know what to do.  We run hooks too often and do not provide
> sufficient information when running them.  Sometimes we even hide
> information when running a hook.  The problem is that changing the
> current situation will have us either run even more hooks or cause
> protests when a hook is no more run where it was run before.

I agree.  I think Lisp programs that use hooks provided by
display-related code should generally expect to be called in many
unrelated situations, and do whatever it takes by themselves to detect
when it's "their" use case.  Expectations or requests for more focused
hooks are impractical or even not feasible to implement, because core
code knows very little about the Lisp application which uses the hook.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >> I don't know what to do.  We run hooks too often and do not provide
 >> sufficient information when running them.  Sometimes we even hide
 >> information when running a hook.  The problem is that changing the
 >> current situation will have us either run even more hooks or cause
 >> protests when a hook is no more run where it was run before.
 >
 > I agree.  I think Lisp programs that use hooks provided by
 > display-related code should generally expect to be called in many
 > unrelated situations, and do whatever it takes by themselves to detect
 > when it's "their" use case.  Expectations or requests for more focused
 > hooks are impractical or even not feasible to implement, because core
 > code knows very little about the Lisp application which uses the hook.

'window-configuration-change-hook' is a great mess and is not
display-related.  What users really need IMO is a single hook say
'window-state-change-functions' that we'd call in redisplay_internal
in lieu of 'window-size-change-functions'.  We would run it if
something in the state of a frame's root window changed (including
size changes, changes of the windows' start positions and the selected
window) and additionally provide a list of the differences in the
frame's previous window state and the one redisplay is about to use.
But such a change would be incompatible and the hook would
consequently have to be run in parallel with our present hooks.

martin

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> Date: Mon, 24 Sep 2018 14:25:15 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: juri@linkov.net, 32672@debbugs.gnu.org
> 
>  > I agree.  I think Lisp programs that use hooks provided by
>  > display-related code should generally expect to be called in many
>  > unrelated situations, and do whatever it takes by themselves to detect
>  > when it's "their" use case.  Expectations or requests for more focused
>  > hooks are impractical or even not feasible to implement, because core
>  > code knows very little about the Lisp application which uses the hook.
> 
> 'window-configuration-change-hook' is a great mess and is not
> display-related.

It is for the purposes of this discussion, since it's related to
changes in what windows display which buffers and which frames show
what windows.

> What users really need IMO is a single hook say
> 'window-state-change-functions' that we'd call in redisplay_internal
> in lieu of 'window-size-change-functions'.

redisplay_internal knows very little about changes in window
configurations, so I predict using this new hook will be as messy as
with the existing ones.

> We would run it if something in the state of a frame's root window
> changed (including size changes, changes of the windows' start
> positions and the selected window) and additionally provide a list
> of the differences in the frame's previous window state and the one
> redisplay is about to use.

We'd need to compute this list of changes first, and for that we will
need a whole slew of new variables and flags.  Currently, redisplay is
built on the opposite assumption: that each operation which could
potentially require redrawing some window(s) or frame(s) sets the
corresponding flags of the object that needs to be redrawn, without
any "explanation" of why that flag was set.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >> 'window-configuration-change-hook' is a great mess and is not
 >> display-related.
 >
 > It is for the purposes of this discussion, since it's related to
 > changes in what windows display which buffers and which frames show
 > what windows.

The point I tried to make was that currently running that hook is not
coupled to the eventual display of windows but to things that happen
before.

 >> What users really need IMO is a single hook say
 >> 'window-state-change-functions' that we'd call in redisplay_internal
 >> in lieu of 'window-size-change-functions'.
 >
 > redisplay_internal knows very little about changes in window
 > configurations, so I predict using this new hook will be as messy as
 > with the existing ones.

'redisplay_internal' per se should not have to know anything about
these changes.  It would call a simple function that calculates for
every window whether it already existed at the time of the last
redisplay and whether one of its sizes, its position, buffer, point,
or start position changed since then.  In addition we could trace the
state of its parameters and other things like its dedicatedness.

 >> We would run it if something in the state of a frame's root window
 >> changed (including size changes, changes of the windows' start
 >> positions and the selected window) and additionally provide a list
 >> of the differences in the frame's previous window state and the one
 >> redisplay is about to use.
 >
 > We'd need to compute this list of changes first, and for that we will
 > need a whole slew of new variables and flags.  Currently, redisplay is
 > built on the opposite assumption: that each operation which could
 > potentially require redrawing some window(s) or frame(s) sets the
 > corresponding flags of the object that needs to be redrawn, without
 > any "explanation" of why that flag was set.

As I said, redisplay would not have to care about that at all.  It
would simply call 'window-state-change-functions' where it calls
'window-size-change-functions' now.  And running
'window-state-change-functions' would use one boolean set (among
others) instead of where 'run-window-configuration-change-hook' gets
called now and which it resets.  Iff that boolean was set, it would
start to find all windows where a relevant change occurred and run the
functions.  Buffer-locally iff a window shows the buffer for which the
local hook was set and something changed for that window.

The great advantage for users and application programmers would be
that their functions would run once only and only if something really
changed since last redisplay.  Basically, it would extend the current
behavior of 'window-size-change-functions' to all window changing
functions.  And we could extend it in the sense that the users may
customize which changes should run their function without inventing
future hooks for them (admittedly 'add-hook' would then need a fifth
argument or some special interpretation of LOCAL for that).

martin

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> Date: Mon, 24 Sep 2018 19:37:08 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: juri@linkov.net, 32672@debbugs.gnu.org
> 
> As I said, redisplay would not have to care about that at all.  It
> would simply call 'window-state-change-functions' where it calls
> 'window-size-change-functions' now.  And running
> 'window-state-change-functions' would use one boolean set (among
> others) instead of where 'run-window-configuration-change-hook' gets
> called now and which it resets.  Iff that boolean was set, it would
> start to find all windows where a relevant change occurred and run the
> functions.  Buffer-locally iff a window shows the buffer for which the
> local hook was set and something changed for that window.

Those functions will need to keep track of the changes, or record the
previous state attributes somewhere, to do their job, right?  Are you
saying that these are already recorded/tracked?  If not, they will
need to be added, which was the point I was making.

> The great advantage for users and application programmers would be
> that their functions would run once only and only if something really
> changed since last redisplay.

Even the "once" part might be problematic, because redisplay_internal
sometimes re-runs its code more than once, as you know.  The "really
changed since last redisplay" is even trickier, because a given
redisplay cycle doesn't always finish all of its job, it can stop in
the middle and return after doing only part of what needs to be done,
in which case some of the windows are not updated, and we will have
lost our point of reference.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >> As I said, redisplay would not have to care about that at all.  It
 >> would simply call 'window-state-change-functions' where it calls
 >> 'window-size-change-functions' now.  And running
 >> 'window-state-change-functions' would use one boolean set (among
 >> others) instead of where 'run-window-configuration-change-hook' gets
 >> called now and which it resets.  Iff that boolean was set, it would
 >> start to find all windows where a relevant change occurred and run the
 >> functions.  Buffer-locally iff a window shows the buffer for which the
 >> local hook was set and something changed for that window.
 >
 > Those functions will need to keep track of the changes, or record the
 > previous state attributes somewhere, to do their job, right?  Are you
 > saying that these are already recorded/tracked?  If not, they will
 > need to be added, which was the point I was making.

I'm not sure what you mean with "Those functions".  Running
'window-size-change-functions' already records the current sizes in
preparation for the call during next redisplay.
'window-state-change-functions' would do the same for the values it
manages.

 >> The great advantage for users and application programmers would be
 >> that their functions would run once only and only if something really
 >> changed since last redisplay.
 >
 > Even the "once" part might be problematic, because redisplay_internal
 > sometimes re-runs its code more than once, as you know.  The "really
 > changed since last redisplay" is even trickier, because a given
 > redisplay cycle doesn't always finish all of its job, it can stop in
 > the middle and return after doing only part of what needs to be done,
 > in which case some of the windows are not updated, and we will have
 > lost our point of reference.

What you mention here would affect 'window-size-change-functions'
already now.  I see no great harm in that.  If running
'window-size-change-functions' succeeds, callers will have adapted to
the new sizes and no further calls are necessary.  If it fails, the
old sizes will continue to exist and the functions will be called
(maybe again) during the next internal redisplay.

martin

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> Date: Tue, 25 Sep 2018 09:26:47 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: juri@linkov.net, 32672@debbugs.gnu.org
> 
>  >> As I said, redisplay would not have to care about that at all.  It
>  >> would simply call 'window-state-change-functions' where it calls
>  >> 'window-size-change-functions' now.  And running
>  >> 'window-state-change-functions' would use one boolean set (among
>  >> others) instead of where 'run-window-configuration-change-hook' gets
>  >> called now and which it resets.  Iff that boolean was set, it would
>  >> start to find all windows where a relevant change occurred and run the
>  >> functions.  Buffer-locally iff a window shows the buffer for which the
>  >> local hook was set and something changed for that window.
>  >
>  > Those functions will need to keep track of the changes, or record the
>  > previous state attributes somewhere, to do their job, right?  Are you
>  > saying that these are already recorded/tracked?  If not, they will
>  > need to be added, which was the point I was making.
> 
> I'm not sure what you mean with "Those functions".

I mean window-state-change-functions, what else?

> Running 'window-size-change-functions' already records the current
> sizes in preparation for the call during next redisplay.
> 'window-state-change-functions' would do the same for the values it
> manages.

We are talking about hypothetical function(s), so it may well be that
there's some misunderstanding.  My point is that accurate recording of
window-size changes is hard, because the various variables used for
that might be outdated (e.g., due top a redisplay cycle that didn't
complete).  Also, redisplay_internal, which calls those functions,
will sometimes call them more than once in a redisplay cycle (see the
'retry' label and code that jumps back to it).

Bottom line is what I said up-thread: Lisp programs cannot expect
those hook calls to be too accurate and focused, they need to be
prepared to handle many irrelevant calls, and they had better have
their own bookkeeping regarding window dimensions etc.

>  > Even the "once" part might be problematic, because redisplay_internal
>  > sometimes re-runs its code more than once, as you know.  The "really
>  > changed since last redisplay" is even trickier, because a given
>  > redisplay cycle doesn't always finish all of its job, it can stop in
>  > the middle and return after doing only part of what needs to be done,
>  > in which case some of the windows are not updated, and we will have
>  > lost our point of reference.
> 
> What you mention here would affect 'window-size-change-functions'
> already now.  I see no great harm in that.

Neither do I; my point, once again, is that expectations should be
quite low.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >> I'm not sure what you mean with "Those functions".
 >
 > I mean window-state-change-functions, what else?

That would be a hook and would indeed have to do the work.  But why
the plural?

 > We are talking about hypothetical function(s), so it may well be that
 > there's some misunderstanding.  My point is that accurate recording of
 > window-size changes is hard, because the various variables used for
 > that might be outdated (e.g., due top a redisplay cycle that didn't
 > complete).  Also, redisplay_internal, which calls those functions,
 > will sometimes call them more than once in a redisplay cycle (see the
 > 'retry' label and code that jumps back to it).
 >
 > Bottom line is what I said up-thread: Lisp programs cannot expect
 > those hook calls to be too accurate and focused, they need to be
 > prepared to handle many irrelevant calls, and they had better have
 > their own bookkeeping regarding window dimensions etc.

'window-size-change-functions' is not hypothetical and guards itself
against running twice for unchanged window sizes.  I don't really
understand what you doubt here - I rewrote it in its current form
because you once said (when discussing Bug#21333) that

 > I believe window-size-change-functions is meant for taking notice of
 > resizes done by the user or some Lisp code, not for automated resizes
 > whose sole purpose is to allow some message be read in its entirety.
 > If you agree, then the current behavior will make sense to you.
 >
 > If anything, IMO we should _reduce_ the number of unrelated events
 > that trigger a call to these functions.  For example, currently any
 > command that reads from the minibuffer will trigger it, because when
 > read-from-minibuffer exits, it restores the window configuration by
 > calling set-window-configuration, which is documented to trigger these
 > functions.  That just doesn't make any sense to me, since most reads
 > from the minibuffer don't resize any windows!

and in a later post you said

 > I'd say, don't set the "size changed" flag unless the size really
 > changed.

and now it seems that you think that a similar argument does not apply
when running 'window-configuration-change-hook'.

I'd still need to see a hypothetical example where the same redisplay
cycle would run 'window-size-change-functions' functions twice when no
sizes actually changed.

martin

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> Date: Tue, 25 Sep 2018 19:56:09 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: juri@linkov.net, 32672@debbugs.gnu.org
> 
>  > Bottom line is what I said up-thread: Lisp programs cannot expect
>  > those hook calls to be too accurate and focused, they need to be
>  > prepared to handle many irrelevant calls, and they had better have
>  > their own bookkeeping regarding window dimensions etc.
> 
> 'window-size-change-functions' is not hypothetical and guards itself
> against running twice for unchanged window sizes.  I don't really
> understand what you doubt here - I rewrote it in its current form
> because you once said (when discussing Bug#21333) that
> 
>  > I believe window-size-change-functions is meant for taking notice of
>  > resizes done by the user or some Lisp code, not for automated resizes
>  > whose sole purpose is to allow some message be read in its entirety.
>  > If you agree, then the current behavior will make sense to you.
>  >
>  > If anything, IMO we should _reduce_ the number of unrelated events
>  > that trigger a call to these functions.  For example, currently any
>  > command that reads from the minibuffer will trigger it, because when
>  > read-from-minibuffer exits, it restores the window configuration by
>  > calling set-window-configuration, which is documented to trigger these
>  > functions.  That just doesn't make any sense to me, since most reads
>  > from the minibuffer don't resize any windows!
> 
> and in a later post you said
> 
>  > I'd say, don't set the "size changed" flag unless the size really
>  > changed.
> 
> and now it seems that you think that a similar argument does not apply
> when running 'window-configuration-change-hook'.

All true and agreed, but it doesn't contradict my main point.  We
could (and do) try to be as accurate as possible, but there are limits
to that that cannot be easily lifted, not without some serious
redesign.

> I'd still need to see a hypothetical example where the same redisplay
> cycle would run 'window-size-change-functions' functions twice when no
> sizes actually changed.

We have better things to do with our free time than discuss
hypothetical examples.

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> I don't know what to do.  We run hooks too often and do not provide
>> sufficient information when running them.  Sometimes we even hide
>> information when running a hook.  The problem is that changing the
>> current situation will have us either run even more hooks or cause
>> protests when a hook is no more run where it was run before.
>
> I agree.  I think Lisp programs that use hooks provided by
> display-related code should generally expect to be called in many
> unrelated situations, and do whatever it takes by themselves to detect
> when it's "their" use case.  Expectations or requests for more focused
> hooks are impractical or even not feasible to implement, because core
> code knows very little about the Lisp application which uses the hook.

I think window hook calls should be consistent at least with own inner logic,
e.g. as the semantics of the window-size-change-functions hook name suggests
it should be called when the window size is not the same as was before,
window-configuration-change-hook is called when the result of window-state-get
is not the same as it was before, etc.

This poses a question what a programmer is supposed to do when a hook is
more general, how to filter out the wanted notifications when the hook
is fired more often than needed.  We already have a working solution
in form of window-pixel-width-before-size-change, that could be extended
to other hooks, thus they could report the reason of their calls
exposing more their meta information, e.g. window-configuration-change-hook
to report that only one buffer in one particular window was switched.

So either we need more specific hooks, or allow general hooks report
the details inscluding the reasons of their calls.

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: martin rudalics <rudalics@gmx.at>,  32672@debbugs.gnu.org
> Date: Mon, 24 Sep 2018 21:38:59 +0300
> 
> I think window hook calls should be consistent at least with own inner logic,
> e.g. as the semantics of the window-size-change-functions hook name suggests
> it should be called when the window size is not the same as was before,
> window-configuration-change-hook is called when the result of window-state-get
> is not the same as it was before, etc.

My point is that these expectations are hard to meet with the current
design.  Most of those hooks are called from the display engine, which
normally has no idea what was changed since the last redisplay, in
terms of window dimensions and buffers displayed in each window.

You must keep in mind that Emacs has the MVC (model-view-controller)
design, where the View part -- redisplay -- basically just reflects on
the glass what it finds in the various Lisp data structures at the
moment when redisplay is invoked.  It doesn't track changes in
windows, frames, and buffers.  But the users of those hooks want to be
invoked when some such change happened.  And that is the crux of the
problem.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > I think window hook calls should be consistent at least with own inner logic,
 > e.g. as the semantics of the window-size-change-functions hook name suggests
 > it should be called when the window size is not the same as was before,
 > window-configuration-change-hook is called when the result of window-state-get
 > is not the same as it was before, etc.

Tying 'window-configuration-change-hook' to 'window-state-get' would
be considerably more than we do now or what I'd propose.  We probably
shouldn't care about a specific window's scroll bars or margins there.

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> I think window hook calls should be consistent at least with own inner logic,
>> e.g. as the semantics of the window-size-change-functions hook name suggests
>> it should be called when the window size is not the same as was before,
>> window-configuration-change-hook is called when the result of window-state-get
>> is not the same as it was before, etc.
>
> Tying 'window-configuration-change-hook' to 'window-state-get' would
> be considerably more than we do now or what I'd propose.  We probably
> shouldn't care about a specific window's scroll bars or margins there.

Your proposed window-state-change-functions would match window-state-get
very well, e.g. it could call the hook with an argument containing alist
of values that really changed, where elements of the alist could have
the same keys as in alist returned from window-state-get, for example:

  (add-hook 'window-state-change-functions (lambda (window alist) ...) nil t)

where 'alist' could have such keys and values of changes:

  (buffer "*scratch*") - means the buffer was switched in the window

  (selected) - the window was selected

  (start . #<marker at 146 in *scratch*>)  - the same meaning as for
                                             window-scroll-functions

  (pixel-width . 672) (pixel-height . 557) - the same meaning as for
                                             window-size-change-functions
maybe also include

  (pixel-width-before-size-change . 672)
  (pixel-height-before-size-change . 557)

with the same meaning as window-pixel-width-before-size-change
and window-pixel-height-before-size-change

or with shorter names

  (prev-pixel-width . 672)
  (prev-pixel-height . 557)

then it makes sense to add also

  (prev-buffer "*scratch*")

  (prev-start . #<marker at 146 in *scratch*>)

Or maybe simpler to call the hook with two arguments
containing the whole state data structures:

  (add-hook 'window-state-change-functions (lambda (window prev-state next-state) ...))

but then it's difficult for its consumer to find the differences.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > Your proposed window-state-change-functions would match window-state-get
 > very well, e.g. it could call the hook with an argument containing alist
 > of values that really changed, where elements of the alist could have
 > the same keys as in alist returned from window-state-get,

Retaining the nomenclature of 'window-state-get' is part of the plan.
But I'm not sure whether we want to retain all of what is recorded in
a window state.

 > for example:
 >
 >    (add-hook 'window-state-change-functions (lambda (window alist) ...) nil t)
 >
 > where 'alist' could have such keys and values of changes:
 >
 >    (buffer "*scratch*") - means the buffer was switched in the window

(buffer . #<buffer *scratch*>) rather

 >    (selected) - the window was selected

Yes.  But this would be stored on a per frame basis.

 >    (start . #<marker at 146 in *scratch*>)  - the same meaning as for
 >                                               window-scroll-functions
 >
 >    (pixel-width . 672) (pixel-height . 557) - the same meaning as for
 >                                               window-size-change-functions

Yes.  And maybe the body size in pixels as well.

 > maybe also include
 >
 >    (pixel-width-before-size-change . 672)
 >    (pixel-height-before-size-change . 557)
 >
 > with the same meaning as window-pixel-width-before-size-change
 > and window-pixel-height-before-size-change
 >
 > or with shorter names
 >
 >    (prev-pixel-width . 672)
 >    (prev-pixel-height . 557)
 >
 > then it makes sense to add also
 >
 >    (prev-buffer "*scratch*")
 >
 >    (prev-start . #<marker at 146 in *scratch*>)

That's one way to do that.  I'm not yet sure whether there's a need
for a function like 'window-prev-buffer' getting me the buffer shown
the last time redisplay called 'window-state-change-functions'.  If
not, we could encapsulate all information for the user in the alist
as you propose.

 > Or maybe simpler to call the hook with two arguments
 > containing the whole state data structures:
 >
 >    (add-hook 'window-state-change-functions (lambda (window prev-state next-state) ...))
 >
 > but then it's difficult for its consumer to find the differences.

That would be considerably simpler to implement.

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

>> Your proposed window-state-change-functions would match window-state-get
>> very well, e.g. it could call the hook with an argument containing alist
>> of values that really changed, where elements of the alist could have
>> the same keys as in alist returned from window-state-get,
>
> Retaining the nomenclature of 'window-state-get' is part of the plan.
> But I'm not sure whether we want to retain all of what is recorded in
> a window state.
>
>> for example:
>>
>>    (add-hook 'window-state-change-functions (lambda (window alist) ...) nil t)
>>
>> where 'alist' could have such keys and values of changes:
>>
>>    (buffer "*scratch*") - means the buffer was switched in the window
>
> (buffer . #<buffer *scratch*>) rather

Here is a complete implementation that works well when tested
on display-buffer-directionally in bug#32790 (it doesn't handle
window-point because then the hook is called too often):


[-- Attachment #2: window-state-change-functions.1.el --]
[-- Type: application/emacs-lisp, Size: 1429 bytes --]

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >>> Your proposed window-state-change-functions would match window-state-get
 >>> very well, e.g. it could call the hook with an argument containing alist
 >>> of values that really changed, where elements of the alist could have
 >>> the same keys as in alist returned from window-state-get,
 >>
 >> Retaining the nomenclature of 'window-state-get' is part of the plan.
 >> But I'm not sure whether we want to retain all of what is recorded in
 >> a window state.
...
 > Here is a complete implementation that works well when tested
 > on display-buffer-directionally in bug#32790

But tying this to the execution of commands misses the point that we
would have to react to state changes whenever a frame is redisplayed.
That is, we would miss all asynchronous, non-command-bound changes in
a frame.

What 'window-state-change-functions' would have to do IMO is:

(1) Record, for each frame, whether window states might have changed
     since the last redisplay of the frame.  That means to set a
     frame's flag whenever the buffer of a window, its selectedness or
     its start point changed.  The reason I'd do this is to avoid the
     costly pre-analysis in (2), for example, after each self-insertion
     or any other change of buffer text.

(2) During redisplay, check whether that flag was set and if so
     compare all windows on that frame with the state recorded during
     last redisplay.  As we do for 'window-size-change-functions'
     already now but also check all window related things like those
     from (1).  If and only if something changed, run the functions on
     'window-state-change-functions' providing them information of what
     has changed - a new window w was made, the buffer of window w has
     changed and b was its buffer during last redisplay, w was selected
     and ww was the window selected during last redisplay and so on.
     Or just give them the old state with, say, window buffers instead
     of window buffer names and the previous and next buffers of each
     window elided (these would be too costly to maintain and analyze,
     I think).  Thereafter, record the new state of the frame and clear
     the flag.

This would improve on the current 'window-configuration-change-hook'
in two regards: We'd run it less often (remember the number of calls
you cited earlier) and we'd run it more accurately (that is, only when
and always when something did really change).

The hard part to code such a thing is _how_ to provide the information
of what has changed.  There are three major elements:

(a) Windows that were added.  Trivial.

(b) Windows that were deleted.  Trivial.

(c) Windows where something changed (size, buffer, start position,
     dedicatedness, a window parameter, 'quit-restore' ???, ...).
     Non-trivial.

Note that not providing that information will mean that the function
run by the hook would have to manually compare the old state with the
present one in order to find out what has changed, much as what they
would do for size changes already now.

 > (it doesn't handle
 > window-point because then the hook is called too often):

I suppose we could but only when it's written back, that is, changing
point of the buffer shown in the selected window wouldn't count but
writing it back into 'window-point' when selecting another window
would.  But in principle I agree with what you mean here.

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> But unfortunately it has no hook, and (info "(elisp) Buffers and Windows") says
>> that set-window-buffer runs window-configuration-change-hook (too general
>> for this task since called too often)
>
> Because we run it also whenever a window changes size which is silly.
> What else do we have 'window-size-change-functions' for?  I wanted to
> change that but apparently ducked out.

Maybe because taking out window size change notifications from
window-configuration-change-hook is a backward-incompatible change?

>> and window-scroll-functions (also called too often).
>
>> Regarding window-scroll-functions, it would be too strange to use
>> it to catch set-window-buffer calls.
>>
>> (info "(elisp) Window Hooks") says:
>>
>>    There are three actions that can change this: scrolling the window,
>>    switching buffers in the window, and changing the size of the window.
>>    The first two actions run ‘window-scroll-functions’; the last runs
>>    ‘window-size-change-functions’.
>>
>> Shouldn't the first two actions run separate hooks?

Please also answer this question.  I believe this is the crucial question
for this request.

>> Moreover, it seems window-scroll-functions doesn't work even
>> for its purpose: it's not called after scrolling, e.g. not called
>> after 'C-l' (recenter-top-bottom) - tried with different prefix args.
>
> Maybe because the buffer of the scrolled window is not current when
> you call it.

Sorry, actually it works, I missed the logged messages because the content
of the *Messages* buffer is not refreshed to show new logged messages
from the calls of the window-scroll-functions hook such as

  (add-to-list 'window-scroll-functions
               (lambda (window display-start)
                 (message "window-scroll-functions %S %S %S"
                          window (window-buffer window) display-start)))

This means that C-l (recenter-top-bottom) doesn't redisplay the frame -
which is strange since it calls 'recenter' with non-nil arg REDISPLAY.
Only after switching to the *Messages* buffer with 'C-x o' it gets
redisplayed and all previously emitted messages appear in it.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > Maybe because taking out window size change notifications from
 > window-configuration-change-hook is a backward-incompatible change?

Certainly so.  But we now have the situation that only parts of the
size changes are reflected in calls of
'window-configuration-change-hook'.  I almost certainly will reinstate
the call for frame size changes because of Bug#32720.  But I won't
cover minibuffer window induced size changes.  Apparently there's no
harm in that because most callers of
'window-configuration-change-hook' are only interested in changes of
window widths (and as such will be affected when the body width of a
window changes with its total width unchanged).

 >>> and window-scroll-functions (also called too often).
 >>
 >>> Regarding window-scroll-functions, it would be too strange to use
 >>> it to catch set-window-buffer calls.
 >>>
 >>> (info "(elisp) Window Hooks") says:
 >>>
 >>>     There are three actions that can change this: scrolling the window,
 >>>     switching buffers in the window, and changing the size of the window.
 >>>     The first two actions run ‘window-scroll-functions’; the last runs
 >>>     ‘window-size-change-functions’.
 >>>
 >>> Shouldn't the first two actions run separate hooks?
 >
 > Please also answer this question.  I believe this is the crucial question
 > for this request.

I have no answer to this question.  But note that switching buffers
also runs 'window-configuration-change-hook'.  The latter is not run
when just the start position of that window's buffer changes.

 > Sorry, actually it works, I missed the logged messages because the content
 > of the *Messages* buffer is not refreshed to show new logged messages
 > from the calls of the window-scroll-functions hook such as
 >
 >    (add-to-list 'window-scroll-functions
 >                 (lambda (window display-start)
 >                   (message "window-scroll-functions %S %S %S"
 >                            window (window-buffer window) display-start)))
 >
 > This means that C-l (recenter-top-bottom) doesn't redisplay the frame -
 > which is strange since it calls 'recenter' with non-nil arg REDISPLAY.
 > Only after switching to the *Messages* buffer with 'C-x o' it gets
 > redisplayed and all previously emitted messages appear in it.

I never recenter so I have no idea what to expect from that function.
But from the doc I assume that you have to set 'recenter-redisplay'
appropriately.

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

Using new hooks I noticed only one problem that when the same image buffer is
displayed in several windows then using `other-window' to switch between them,
the selection-change hook is not always called.

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 92ba577b4f..0d6a0b8d04 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -574,6 +574,10 @@ image-mode
 
 	(add-hook 'change-major-mode-hook #'image-toggle-display-text nil t)
 	(add-hook 'after-revert-hook #'image-after-revert-hook nil t)
+	(add-hook 'window-size-change-functions (debounce #'image-window-change 1) nil t)
+	(add-hook 'window-state-change-functions (debounce #'image-window-change 1) nil t)
+	(add-hook 'window-selection-change-functions (debounce #'image-window-change 1) nil t)
+
 	(run-mode-hooks 'image-mode-hook)
 	(let ((image (image-get-display-property))
 	      (msg1 (substitute-command-keys
@@ -828,6 +832,19 @@ image-after-revert-hook
           (get-buffer-window-list (current-buffer) 'nomini 'visible))
     (image-toggle-display-image)))
 
+(defun image-window-change (window)
+  (when (and (eq window (selected-window)) (derived-mode-p 'image-mode))
+    (let ((spec (image-get-display-property)))
+      (when (eq (car-safe spec) 'image)
+        (let* ((image-width  (plist-get (cdr spec) :max-width))
+               (image-height (plist-get (cdr spec) :max-height))
+               (window-edges (window-inside-pixel-edges window))
+               (window-width  (- (nth 2 window-edges) (nth 0 window-edges)))
+               (window-height (- (nth 3 window-edges) (nth 1 window-edges))))
+          (when (or (not (= image-width  window-width))
+                    (not (= image-height window-height)))
+            (image-toggle-display-image)))))))
+
 \f
 ;;; Animated images
 

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > Using new hooks I noticed only one problem that when the same image buffer is
 > displayed in several windows then using `other-window' to switch between them,
 > the selection-change hook is not always called.

I can't imagine that to fail - it's too elementary.  Can you give
an 'other-window' scenario without images?

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> Using new hooks I noticed only one problem that when the same image buffer is
>> displayed in several windows then using `other-window' to switch between them,
>> the selection-change hook is not always called.
>
> I can't imagine that to fail - it's too elementary.  Can you give
> an 'other-window' scenario without images?

This is because of debounce: the selection-change hook is called twice,
so only the last call is processed.  But the problem is that the
selection-change hook is called not in the logical order, as would be
expected, i.e. first in unselected window, then in the selected window.
But actually it's called in the order of windows in the window list,
e.g. first the 'selected' event is called if the window was created
earlier, then the 'deselected' event if the window was created later.
IOW, the calling order depends on the window order.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 > This is because of debounce: the selection-change hook is called twice,
 > so only the last call is processed.  But the problem is that the
 > selection-change hook is called not in the logical order, as would be
 > expected, i.e. first in unselected window, then in the selected window.
 > But actually it's called in the order of windows in the window list,
 > e.g. first the 'selected' event is called if the window was created
 > earlier, then the 'deselected' event if the window was created later.
 > IOW, the calling order depends on the window order.

This is consistent with running the remaining hooks.  In either case
you can always use 'old-selected-window' to find out what changed.

martin

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 27 Dec 2018 01:42:54 +0200
> 
> +(defun image-window-change (window)
> +  (when (and (eq window (selected-window)) (derived-mode-p 'image-mode))
> +    (let ((spec (image-get-display-property)))
> +      (when (eq (car-safe spec) 'image)
> +        (let* ((image-width  (plist-get (cdr spec) :max-width))
> +               (image-height (plist-get (cdr spec) :max-height))
> +               (window-edges (window-inside-pixel-edges window))
> +               (window-width  (- (nth 2 window-edges) (nth 0 window-edges)))
> +               (window-height (- (nth 3 window-edges) (nth 1 window-edges))))
> +          (when (or (not (= image-width  window-width))
> +                    (not (= image-height window-height)))
> +            (image-toggle-display-image)))))))

What happoens in Emacs that was not built with Imagemagick?  Should we
perhaps do nothing in this hook in that case?

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>> +(defun image-window-change (window)
>> +  (when (and (eq window (selected-window)) (derived-mode-p 'image-mode))
>> +    (let ((spec (image-get-display-property)))
>> +      (when (eq (car-safe spec) 'image)
>> +        (let* ((image-width  (plist-get (cdr spec) :max-width))
>> +               (image-height (plist-get (cdr spec) :max-height))
>> +               (window-edges (window-inside-pixel-edges window))
>> +               (window-width  (- (nth 2 window-edges) (nth 0 window-edges)))
>> +               (window-height (- (nth 3 window-edges) (nth 1 window-edges))))
>> +          (when (or (not (= image-width  window-width))
>> +                    (not (= image-height window-height)))
>> +            (image-toggle-display-image)))))))
>
> What happens in Emacs that was not built with Imagemagick?  Should we
> perhaps do nothing in this hook in that case?

Let's hope that native image scaling will handle this properly.

bug#32672: 27.0.50; image resize on window resizing

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 32672@debbugs.gnu.org
> Date: Thu, 27 Dec 2018 22:58:04 +0200
> 
> >> +          (when (or (not (= image-width  window-width))
> >> +                    (not (= image-height window-height)))
> >> +            (image-toggle-display-image)))))))
> >
> > What happens in Emacs that was not built with Imagemagick?  Should we
> > perhaps do nothing in this hook in that case?
> 
> Let's hope that native image scaling will handle this properly.

We don't have that yet, so we should do something about that in the
meantime.

bug#32672: 27.0.50; image resize on window resizing

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Using new hooks I noticed only one problem that when the same image buffer is
> displayed in several windows then using `other-window' to switch between them,
> the selection-change hook is not always called.

[...]

> +	(add-hook 'window-size-change-functions (debounce #'image-window-change 1) nil t)
> +	(add-hook 'window-state-change-functions (debounce #'image-window-change 1) nil t)
> +	(add-hook 'window-selection-change-functions (debounce #'image-window-change 1) nil t)
> +

I think this sounds like a nice addition for image-mode...  but were the
remaining edge cases ever resolved?  I see that the patch hasn't been
applied.


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

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

tags 32672 fixed
close 32672 27.0.50
quit

>> Using new hooks I noticed only one problem that when the same image buffer is
>> displayed in several windows then using `other-window' to switch between them,
>> the selection-change hook is not always called.
>
> [...]
>
>> +	(add-hook 'window-size-change-functions (debounce #'image-window-change 1) nil t)
>> +	(add-hook 'window-state-change-functions (debounce #'image-window-change 1) nil t)
>> +	(add-hook 'window-selection-change-functions (debounce #'image-window-change 1) nil t)
>> +
>
> I think this sounds like a nice addition for image-mode...  but were the
> remaining edge cases ever resolved?  I see that the patch hasn't been
> applied.

This is installed now with the following patch and closed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: image--window-change.patch --]
[-- Type: text/x-diff, Size: 1866 bytes --]

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index db6864649d..09d7828047 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -599,6 +599,10 @@ image-mode--setup-mode
 
   (add-hook 'change-major-mode-hook #'image-toggle-display-text nil t)
   (add-hook 'after-revert-hook #'image-after-revert-hook nil t)
+  (add-hook 'window-size-change-functions #'image--window-change nil t)
+  (add-hook 'window-state-change-functions #'image--window-change nil t)
+  (add-hook 'window-selection-change-functions #'image--window-change nil t)
+
   (run-mode-hooks 'image-mode-hook)
   (let ((image (image-get-display-property))
 	(msg1 (substitute-command-keys
@@ -856,6 +860,27 @@ image-after-revert-hook
           (get-buffer-window-list (current-buffer) 'nomini 'visible))
     (image-toggle-display-image)))
 
+(defvar image--window-change-function
+  (debounce 1.0
+    (lambda (window)
+      (when (window-live-p window)
+        (with-current-buffer (window-buffer)
+          (when (derived-mode-p 'image-mode)
+            (let ((spec (image-get-display-property)))
+              (when (eq (car-safe spec) 'image)
+                (let* ((image-width  (plist-get (cdr spec) :max-width))
+                       (image-height (plist-get (cdr spec) :max-height))
+                       (edges (window-inside-pixel-edges window))
+                       (window-width  (- (nth 2 edges) (nth 0 edges)))
+                       (window-height (- (nth 3 edges) (nth 1 edges))))
+                  (when (and image-width image-height
+                             (or (not (= image-width  window-width))
+                                 (not (= image-height window-height))))
+                    (image-toggle-display-image)))))))))))
+
+(defun image--window-change (window)
+  (funcall image--window-change-function window))
+
 \f
 ;;; Animated images
 

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >>> +	(add-hook 'window-size-change-functions (debounce #'image-window-change 1) nil t)
 >>> +	(add-hook 'window-state-change-functions (debounce #'image-window-change 1) nil t)
 >>> +	(add-hook 'window-selection-change-functions (debounce #'image-window-change 1) nil t)

Why do you add this to 'window-size-change-functions' and
'window-selection-change-functions' too?

martin

bug#32672: 27.0.50; image resize on window resizing

[Juri Linkov]

>>>> +	(add-hook 'window-size-change-functions (debounce #'image-window-change 1) nil t)
>>>> +	(add-hook 'window-state-change-functions (debounce #'image-window-change 1) nil t)
>>>> +	(add-hook 'window-selection-change-functions (debounce #'image-window-change 1) nil t)
>
> Why do you add this to 'window-size-change-functions' and
> 'window-selection-change-functions' too?

Because it needs a call on window size changes and buffer switching.
Or do you mean that window-state-change-functions covers these two?
Then I'll try to remove them.

bug#32672: 27.0.50; image resize on window resizing

[martin rudalics]

 >> Why do you add this to 'window-size-change-functions' and
 >> 'window-selection-change-functions' too?
 >
 > Because it needs a call on window size changes and buffer switching.
 > Or do you mean that window-state-change-functions covers these two?

Yes.

 > Then I'll try to remove them.

Please do that.

Thanks, martin