bug#19576: 24.4; Broken function in `window-size-change-functions' cause `write-file' to write the wrong buffer

bug#19576: 24.4; Broken function in `window-size-change-functions' cause `write-file' to write the wrong buffer

[Anders Lindgren]

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

Hi!

I have noticed that if a broken window size change function is installed,
`write-file' will save the content of the wrong buffer. One such broken
windows-size-change-function is `follow-window-size-change' of Follow Mode
(a package I wrote many years ago).

To verify this, place the following piece of code in a buffer and evaluate
it (lets call this the CODE buffer). Call M-x test-write-file RET. The code
first creates a new file with the content "Alpha", this works OK. It then
tries to overwrite the file with the content "Beta", answer "y" when asked
to confirm this. Now, the file contains the content of the CODE buffer, and
the CODE buffer is associated with the "test.txt" file name, which clearly
is incorrect.

What happens is that the `y-or-n-p' triggers a window size change callback.
`follow-window-size-change' walks through all buffers to check if anything
is needed to be done. It then restores the original frame, the original
buffer, and the original window. Unfortunately, when restoring the original
window, it also sets the buffer associated with the window as current, this
causes `write-file' to write the wrong buffer.

This will, most likely, affect other functions also using `y-or-n-p', or
otherwise trigger a window change callback.

Interestingly, the problem only is visible in Emacs 24.3 and 24.4, even
though `follow-window-size-change' has been broken much longer than that.
This makes me wonder if there has been a change done to the code that calls
the functions in `window-size-change-functions'.

Of course, this should be corrected in Follow Mode (maybe a simple
reordering of the restore expression would suffice). However, it might be a
good idea for Emacs to ensure that the correct frame, window, and buffer is
set, in case of other broken size change functions.

;;; write-file-bug.el

;; Works in 24.2, broken in 24.3.

(require 'follow)

;; This simulates that there is at least one buffer with Follow Mode
;; enabled.
(add-hook 'window-size-change-functions 'follow-window-size-change t)

(defvar test-write-file--filename "test.txt")

(defun test-write-file--write-to-file (s)
  (with-temp-buffer
    (insert s)
    (write-file test-write-file--filename t)))

(defun test-write-file ()
  (interactive)
  (when (file-exists-p test-write-file--filename)
    (delete-file test-write-file--filename))
  (test-write-file--write-to-file "Alpha")
  (test-write-file--write-to-file "Beta"))

;;; white-file-bug.el ends here.

Sincerely,
    Anders Lindgren


In GNU Emacs 24.4.1 (x86_64-apple-darwin13.4.0, NS apple-appkit-1265.21)
 of 2014-10-21 on builder10-9.porkrind.org
Windowing system distributor `Apple', version 10.3.1265
Configured using:
 `configure --with-ns'

Important settings:
  value of $LC_CTYPE: UTF-8
  locale-coding-system: utf-8-unix

Major mode: Text

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<escape> x e v a l - b u f <tab> <return> <escape>
x e <backspace> t e s t - e <backspace> w <tab> <return>
y <escape> x r e p o <tab> o <backspace> r <tab> <
return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Saving file /Users/anders/emacs/src/bugs/test.txt...
Wrote /Users/anders/emacs/src/bugs/test.txt
File `test.txt' exists; overwrite? (y or n) y
Saving file /Users/anders/emacs/src/bugs/test.txt...
Wrote /Users/anders/emacs/src/bugs/test.txt
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
help-fns mail-prsvr mail-utils help-mode follow easymenu vc-dispatcher
vc-svn time-date tooltip electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process cocoa ns
multi-tty emacs)

Memory information:
((conses 16 74101 5798)
 (symbols 48 17581 0)
 (miscs 40 40 159)
 (strings 32 10646 4637)
 (string-bytes 1 280871)
 (vectors 16 9360)
 (vector-slots 8 377207 14310)
 (floats 8 54 183)
 (intervals 56 224 0)
 (buffers 960 13))

[-- Attachment #2: Type: text/html, Size: 6654 bytes --]

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Hi,

about a year ago I reported that `write-file' sometimes writes the wrong
buffer to the destination file. Unfortunately, I had no feedback regarding
this. When I checked today, the bug is still there.

The problem occurs when a function on the hook
`window-size-change-functions' change the current buffer. The functions in
this hook are executed when `y-or-n-p' is called, which is used by
`write-file' to verify that it is OK to overwrite an existing file. One
such function is `follow-window-size-change' in follow.el.

This problem is not limited to `write-file' -- all functions calling
`y-or-n-p' are affected by this!

Of course, it would be relatively straight forward to modify the offending
function (and all other similar functions). However, a more robust solution
would be for the code that calls the functions on the hook to ensure that
it isn't derailed when the buffer is changed.

Personally, I don't know that part of the code well enough to do this
change. Martin, is this something that you could look into, or suggest
someone who can?

See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19576 for more details.

    -- Anders Lindgren

[-- Attachment #2: Type: text/html, Size: 1484 bytes --]

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > about a year ago I reported that `write-file' sometimes writes the wrong
 > buffer to the destination file. Unfortunately, I had no feedback regarding
 > this. When I checked today, the bug is still there.

Sorry.  I could have sworn we fixed that then.

 > The problem occurs when a function on the hook
 > `window-size-change-functions' change the current buffer. The functions in
 > this hook are executed when `y-or-n-p' is called, which is used by
 > `write-file' to verify that it is OK to overwrite an existing file. One
 > such function is `follow-window-size-change' in follow.el.
 >
 > This problem is not limited to `write-file' -- all functions calling
 > `y-or-n-p' are affected by this!
 >
 > Of course, it would be relatively straight forward to modify the offending
 > function (and all other similar functions). However, a more robust solution
 > would be for the code that calls the functions on the hook to ensure that
 > it isn't derailed when the buffer is changed.
 >
 > Personally, I don't know that part of the code well enough to do this
 > change. Martin, is this something that you could look into, or suggest
 > someone who can?

Conceptually it should be easy to do that.  Save/restore current buffer,
selected window and frame.  But Alan (concerned about ‘follow-mode’),
Pip (who unfortunately disappeared) and Eli are currently discussing how
to fix ‘window-size-change-functions’ in various other ways as well.
I'll try to get the fix of this bug applied there too.

martin

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

>
> Conceptually it should be easy to do that.  Save/restore current buffer,
> selected window and frame.  But Alan (concerned about ‘follow-mode’),
> Pip (who unfortunately disappeared) and Eli are currently discussing how
> to fix ‘window-size-change-functions’ in various other ways as well.
> I'll try to get the fix of this bug applied there too.


This sounds good!

If Alan has any thoughts on follow mode, I'd be happy to discuss them. I
might need a bit of a refresher, though -- after all, I wrote it more than
twenty years ago.

    -- Anders

[-- Attachment #2: Type: text/html, Size: 867 bytes --]

bug#19576: write-file writes the wrong buffer

[Juri Linkov]

>> Conceptually it should be easy to do that.  Save/restore current buffer,
>> selected window and frame.  But Alan (concerned about ‘follow-mode’),
>> Pip (who unfortunately disappeared) and Eli are currently discussing how
>> to fix ‘window-size-change-functions’ in various other ways as well.
>> I'll try to get the fix of this bug applied there too.
>
> This sounds good!
>
> If Alan has any thoughts on follow mode, I'd be happy to discuss them. I
> might need a bit of a refresher, though -- after all, I wrote it more than
> twenty years ago.

I hope Alan (Cc:ed) could explain in details all improvements he's doing
in bug#17453.

Meanwhile, I'm taking an opportunity to ask you about your intriguing comment
in follow.el:

;; Almost like the real thing, except when the cursor ends up outside
;; the top or bottom...  In our case however, we end up outside the
;; window and hence we are recentered.  Should we let `recenter' handle
;; the point position we would never leave the selected window.  To do
;; it ourselves we would need to do our own redisplay, which is easier
;; said than done.  (Why didn't I do a real display abstraction from
;; the beginning?)

What a real display abstraction would you create if you designed
follow-mode today?

bug#19576: write-file writes the wrong buffer

[martin rudalics]

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

 >> I'll try to get the fix of this bug applied there too.

I attach a fix for restoring current buffer and selected window (the
latter hopefully taking care of selecting the right frame).  Please tell
me whether it's sufficient for your purposes.

martin

[-- Attachment #2: bug#19576.diff --]
[-- Type: text/plain, Size: 1536 bytes --]

diff --git a/src/window.c b/src/window.c
index 7c95ff9..06066fd 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3142,14 +3142,14 @@ run_funs (Lisp_Object funs)
       call0 (XCAR (funs));
 }

-static void
+void
 select_window_norecord (Lisp_Object window)
 {
   if (WINDOW_LIVE_P (window))
     Fselect_window (window, Qt);
 }

-static void
+void
 select_frame_norecord (Lisp_Object frame)
 {
   if (FRAME_LIVE_P (XFRAME (frame)))
diff --git a/src/window.h b/src/window.h
index 135f5de..9ee3f81 100644
--- a/src/window.h
+++ b/src/window.h
@@ -1013,6 +1013,8 @@ extern void grow_mini_window (struct window *, int, bool);
 extern void shrink_mini_window (struct window *, bool);
 extern int window_relative_x_coord (struct window *, enum window_part, int);

+void select_window_norecord (Lisp_Object);
+void select_frame_norecord (Lisp_Object);
 void run_window_configuration_change_hook (struct frame *f);

 /* Make WINDOW display BUFFER.  RUN_HOOKS_P means it's allowed
diff --git a/src/xdisp.c b/src/xdisp.c
index dbc2d84..5e0d4ca 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -11739,8 +11739,16 @@ prepare_menu_bars (void)

 	      while (CONSP (functions))
 		{
+		  ptrdiff_t count = SPECPDL_INDEX ();
+
+		  record_unwind_current_buffer ();
+		  record_unwind_protect (select_window_norecord, selected_window);
+
 		  if (!EQ (XCAR (functions), Qt))
 		    call1 (XCAR (functions), frame);
+
+		  unbind_to (count, Qnil);
+
 		  functions = XCDR (functions);
 		}
 	    }

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Hi,

I just tested the patch and I have verified that it works.

Thanks!

    -- Anders

On Tue, Nov 17, 2015 at 9:34 AM, martin rudalics <rudalics@gmx.at> wrote:

> >> I'll try to get the fix of this bug applied there too.
>
> I attach a fix for restoring current buffer and selected window (the
> latter hopefully taking care of selecting the right frame).  Please tell
> me whether it's sufficient for your purposes.
>
> martin
>

[-- Attachment #2: Type: text/html, Size: 912 bytes --]

bug#19576: write-file writes the wrong buffer

[Alan Mackenzie]

Hello, Anders and Juri.

On Tue, Nov 17, 2015 at 02:55:39AM +0200, Juri Linkov wrote:
> >> Conceptually it should be easy to do that.  Save/restore current buffer,
> >> selected window and frame.  But Alan (concerned about ‘follow-mode’),
> >> Pip (who unfortunately disappeared) and Eli are currently discussing how
> >> to fix ‘window-size-change-functions’ in various other ways as well.

I have a fix for the `window-size-change-functions' problem, which I
posted just over an hour ago (see bug #21869 or #21333).  The fix
consists of only invoking w-s-c-f after any change to the echo area size
has been done.  This might have some relevance for the current bug.  (I
haven't followed the current bug, I'm afraid.)  I really need the
go-ahead from Eli before I can commit the fix to the emacs-25 or master
branch.

> >> I'll try to get the fix of this bug applied there too.

> > This sounds good!

> > If Alan has any thoughts on follow mode, I'd be happy to discuss them. I
> > might need a bit of a refresher, though -- after all, I wrote it more than
> > twenty years ago.

In general, follow mode is wonderful (I use it all the time), but
(i) is not sufficiently integrated with the rest of Emacs, and
(ii) is too difficult to use in an emacs -Q.

By (ii), I mean that manually creating the side by side windows and
doing M-x follow-mode is too cumbersome.
follow-delete-other-windows-and-split is not bound to any key sequence
by default.  I have my own private commands bound to C-c 2, C-c 3, C-c
4, which enable follow mode in 2, 3, and 4 windows.  I also have C-c 0,
which disables follow mode.  I think Emacs should have something like
these in its global key map, say on C-x w f.  Maybe for Emacs 25.2, or
26.1.

By (i), I mean that other lisp programs cannot use follow mode.  For
example, many programs use `window-start' to get the start of the area
they want to work on, when really what they should get is the start of
the "lowest" follow window.

The most painful clash I know of is with Isearch.  It has several places
where it just doesn't work: for example, rather than point moving over
the entire set of windows when you input C-s / C-r, is stays in the one
window and scrolls that window.  I have fixed these problems in
isearch.el, and I gather Juri is reviewing these at the moment.  These
fixes depend on ....

I have enhanced follow mode with the six functions follow-window-start,
follow-window-end, follow-set-window-start,
follow-pos-visible-in-window-p, follow-recenter, and
follow-move-to-window-line, which do what their names suggest, but on
an entire follow mode group of windows.

The new follow-... functions need somehow to be connected with the rest
of the lisp world.  The idea is that another program can call just one
interface which will invoke either window-start or follow-window-start,
depending on whether follow mode is active.  I have, as yet, two
alternative implementations for this:
(i) New functions with names like window*-start (notice the "*"),
written in lisp in window.el;
(ii) Adding an extra parameter to the primitives (mainly in window.c),
so that instead of calling (window-start win), a function would call
(window-start win t).

Buffer local variables to perform the redirection are initialised at
follow-mode start up, and removed at follow-mode termination.

Of the above alternatives, Eli prefers (ii), but I think Juri prefers
(i).

> I hope Alan (Cc:ed) could explain in details all improvements he's doing
> in bug#17453.

> Meanwhile, I'm taking an opportunity to ask you about your intriguing comment
> in follow.el:

> ;; Almost like the real thing, except when the cursor ends up outside
> ;; the top or bottom...  In our case however, we end up outside the
> ;; window and hence we are recentered.  Should we let `recenter' handle
> ;; the point position we would never leave the selected window.  To do
> ;; it ourselves we would need to do our own redisplay, which is easier
> ;; said than done.  (Why didn't I do a real display abstraction from
> ;; the beginning?)

I must say, I've been intrigued by this comment for some while, too.

> What a real display abstraction would you create if you designed
> follow-mode today?

Indeed!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Hi Everybody!

(I took the liberty to add Eli to this as well, as this involves the
display engine.)


Meanwhile, I'm taking an opportunity to ask you about your intriguing
> comment
> in follow.el:
>
> ;; Almost like the real thing, except when the cursor ends up outside
> ;; the top or bottom...  In our case however, we end up outside the
> ;; window and hence we are recentered.  Should we let `recenter' handle
> ;; the point position we would never leave the selected window.  To do
> ;; it ourselves we would need to do our own redisplay, which is easier
> ;; said than done.  (Why didn't I do a real display abstraction from
> ;; the beginning?)
>
> What a real display abstraction would you create if you designed
> follow-mode today?
>


Unfortunately, I haven't got a clue what my twenty-years younger me was
thinking when writing this...


However, here are some thoughts on follow-mode I have accumulated over the
years:

* The core of follow-mode call `redisplay' and does all sorts of tricks to
keep up the illusion that a number of windows form a very tall windows. I
would like to do some investigations if this can be done more efficiently.
Keith David Bershatsky (a.k.a. lawlist) has analyzed under which
circumstances Emacs calls the different hooks and functions using a tool he
named `test-mode'. Maybe this information can be used to write a better
implementation of follow-mode. (Of course, one alternative would be to
implement this in the Emacs display engine, but I don't see that happening
anytime soon.)

* The Emacs display engine tries to recenter windows when window-start ==
buffer-end, to ensure that no window would appear empty. When follow-mode
is enabled, this is not what we want (except for the leftmost window). To
counter the recentering efforts of Emacs, follow-mode sets window-start
whenever it gets a chance. I would prefer if it was possible to tell the
display engine to leave some windows alone, preferably with some kind of
buffer-local `recenter-window-functions' variable. It could be made generic
so that the return value would be an integer telling Emacs to show this
many lines (if positive) or place the last line X lines from the bottom
(when negative). If such hook existed, follow-mode could return 0 for all
(except the leftmost) windows. Note that the user can change the content of
any window at any time, so this property isn't static.

* Make it easier for other packages to use it. By this I mean packages that
display information, like "grep", should be able to use follow mode to
display a buffer using side by side windows. Concretely, in
font-lock-studio (a debugger for font-lock keywords I wrote a while ago) I
do this, but I had to call `follow-post-command-hook' explicitly. Follow
mode should need a better interface for this, alternatively this should be
built into `display-buffer' so that it happens automatically.

* When follow-mode is enabled, there is a noticeable lag when updating the
region. You can see this by pressing C-c SPACE and holding the up or down
key. When follow-mode is disabled, the region is updated as fast as the
cursor moves, when enabled, it is updated in chunks.

* When written in the Emacs old era, the region was always highlighted in
all windows. Follow-mode went to great lengths to place the cursor in the
non-selected windows so that the region would look good. Today, one could
get the same effect by setting `highlight-nonselected-windows' (except that
the region is visible on the last line of windows to the left of the
selected window.)  I would love to give the user the option to 1)
automatically set this variable and move the cursor in the non-selected
windows or 2) don't move the cursors at all. (Alternatively, invent a new
way to highlight the region of the selected windows in the other windows,
without the cursor in the non-selected windows affecting it.)

* isearch: If I recall correctly, isearch used to worked across follow-mode
windows. Of course, this was before isearch highlighted matches etc. It
would be great if this could be restored once again!

* Process output: In early versions of follow-mode, it could be used with
any process. This was accomplished using `defadvice' on a handful of
process-related functions. At some point in time, this system was replaced
with a system specific to comint and compilation buffers -- as part of the
great defadvice sweep. Personally, I would like to Emacs to provide
`pre-process-output-functions' and `post-process-output-functions',
allowing packages like follow-mode to perform whatever action they would
like to the output of any process.

* If a user have columns with different widths, follow-mode can't correctly
display long lines stretching from one window to the next. The reason for
this is that the start position can't be placed at an arbitrary location on
a long line, only on positions that are a multiple of the column width. I
don't see any way to solve this without modifying the display engine.

* Automatic tests: These should verify the basic functionality, that the
windows are aligned properly. That moving the point down in one window
would move it to the next. All follow-mode-specific commands etc.


Anyway, this is a dire wish-list and I feel that I, unfortunately,
presently can't contribute to it. When it comes to Emacs responsibilities,
I have taken over the NextStep port after Jan Djärvs resignation, and on a
personal level I have a full time job and I have two small children and a
third due in about a week (if all goes well). Anyway, I'm glad that Alan
has taken this bull by the horn. However, feel free to ask questions or
discuss implementation ideas regarding this.

Sincerely,
    Anders Lindgren

[-- Attachment #2: Type: text/html, Size: 7200 bytes --]

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Hi Alan (and the rest of you)!

First, I'm really glad that you have taken on the challange to modernize
follow mode!


On Tue, Nov 17, 2015 at 02:55:39AM +0200, Juri Linkov wrote:
> > >> Conceptually it should be easy to do that.  Save/restore current
> buffer,
> > >> selected window and frame.  But Alan (concerned about ‘follow-mode’),
> > >> Pip (who unfortunately disappeared) and Eli are currently discussing
> how
> > >> to fix ‘window-size-change-functions’ in various other ways as well.
>
> I have a fix for the `window-size-change-functions' problem, which I
> posted just over an hour ago (see bug #21869 or #21333).  The fix
> consists of only invoking w-s-c-f after any change to the echo area size
> has been done.  This might have some relevance for the current bug.  (I
> haven't followed the current bug, I'm afraid.)  I really need the
> go-ahead from Eli before I can commit the fix to the emacs-25 or master
> branch.
>

Today Martin sent me a patch that solved the window-size-change-functions
problem i reported in bug#19576. I think this is different from the varying
echo area problems of the other bugs, though.


In general, follow mode is wonderful (I use it all the time),


I'm glad to hear it! I use it evert day too, and every time I do I'm glad
that I invested the time to write it.


but
> (i) is not sufficiently integrated with the rest of Emacs, and
> (ii) is too difficult to use in an emacs -Q.
>
> By (ii), I mean that manually creating the side by side windows and
> doing M-x follow-mode is too cumbersome.
> follow-delete-other-windows-and-split is not bound to any key sequence
> by default.  I have my own private commands bound to C-c 2, C-c 3, C-c
> 4, which enable follow mode in 2, 3, and 4 windows.  I also have C-c 0,
> which disables follow mode.  I think Emacs should have something like
> these in its global key map, say on C-x w f.  Maybe for Emacs 25.2, or
> 26.1.
>

I agree, I use an Emacs frame with six columns spread out across two
monitors, and it feels like many of the basic functions are missing.

I while a go I put together a companion program for follow mode which I
named "multicolumn". It provides functions to set up the frame to
accommodate a number of side-by-side windows. It also resizes the frame
(down to the pixel) for this. Also it defines a number of keys like "C-x <"
and "C-x >" for going to the leftmost and rightmost window, respectively.

You can find this at https://github.com/Lindydancer/multicolumn.


By (i), I mean that other lisp programs cannot use follow mode.  For
> example, many programs use `window-start' to get the start of the area
> they want to work on, when really what they should get is the start of
> the "lowest" follow window.
>

Agreed.

One thing that disturbs me (which I haven't fixed yet) is when a new buffer
is displayed in the middle of a group of follow-mode windows. It would be a
good idea to teach `display-buffer' to pick another window, like one the
first or last instead.


  I have, as yet, two
> alternative implementations for this:
> (i) New functions with names like window*-start (notice the "*"),
> written in lisp in window.el;
>

Maybe "window-group-start"? The "*" disappears easily and a window group
could actually be something else than a follow-mode group.


(ii) Adding an extra parameter to the primitives (mainly in window.c),
> so that instead of calling (window-start win), a function would call
> (window-start win t).
>
> Buffer local variables to perform the redirection are initialised at
> follow-mode start up, and removed at follow-mode termination.
>
> Of the above alternatives, Eli prefers (ii), but I think Juri prefers
> (i).


I can't say that it matters, really, but I think that I would prefer (i)
since 1) it's stands out more in the source and 2) it's an all-lisp
implementation.

Sincerely,
    Anders Lindgren

[-- Attachment #2: Type: text/html, Size: 5857 bytes --]

bug#19576: write-file writes the wrong buffer

[Alan Mackenzie]

Hello, Anders.

In article <mailman.9.1447720293.31583.bug-gnu-emacs@gnu.org> you wrote:
> [-- text/plain, encoding 7bit, charset: UTF-8, 28 lines --]

> Hi,

> about a year ago I reported that `write-file' sometimes writes the wrong
> buffer to the destination file. Unfortunately, I had no feedback regarding
> this. When I checked today, the bug is still there.

> The problem occurs when a function on the hook
> `window-size-change-functions' change the current buffer. The functions in
> this hook are executed when `y-or-n-p' is called, which is used by
> `write-file' to verify that it is OK to overwrite an existing file. One
> such function is `follow-window-size-change' in follow.el.

I've run the test code you posted back in January, and I can reproduce
the error.

But I don't see why `window-size-change-functions' are being called when
`y-or-n-p' is run.  It seems to me, all windows stay the same size.
Surely I'm missing something obvious, but what?

> This problem is not limited to `write-file' -- all functions calling
> `y-or-n-p' are affected by this!

> Of course, it would be relatively straight forward to modify the offending
> function (and all other similar functions). However, a more robust solution
> would be for the code that calls the functions on the hook to ensure that
> it isn't derailed when the buffer is changed.

> Personally, I don't know that part of the code well enough to do this
> change. Martin, is this something that you could look into, or suggest
> someone who can?

> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19576 for more details.

>     -- Anders Lindgren

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19576: write-file writes the wrong buffer

[Juri Linkov]

> One thing that disturbs me (which I haven't fixed yet) is when a new buffer
> is displayed in the middle of a group of follow-mode windows. It would be a
> good idea to teach `display-buffer' to pick another window, like one the
> first or last instead.

We have now a powerful system of display-buffer actions such as
display-buffer-reuse-window, display-buffer-below-selected, etc.
So follow-mode could add a new specific display action to
display-buffer-base-action that will exclude follow-mode windows
from consideration when deciding where to display the buffer.

bug#19576: write-file writes the wrong buffer

[Drew Adams]

> We have now a powerful system of display-buffer actions such as
> display-buffer-reuse-window, display-buffer-below-selected, etc.

Don't forget `display-buffer-full-moon-but-not-blue-moon-unless-southern-hemisphere'.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > But I don't see why `window-size-change-functions' are being called when
 > `y-or-n-p' is run.  It seems to me, all windows stay the same size.
 > Surely I'm missing something obvious, but what?

Probably ‘y-or-n-p’ somewhere in between saves and restores the window
configuration where the latter counts as a size change even if there was
none.

martin

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

>
> We have now a powerful system of display-buffer actions such as
> display-buffer-reuse-window, display-buffer-below-selected, etc.
> So follow-mode could add a new specific display action to
> display-buffer-base-action that will exclude follow-mode windows
> from consideration when deciding where to display the buffer.
>

It doesn't have to exclude all the windows. However, say that we have three
side-by-side windows that contains the same buffer and follow-mode is
active, then the middle window should not be picked.

    -- Anders

[-- Attachment #2: Type: text/html, Size: 810 bytes --]

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Tue, 17 Nov 2015 09:34:26 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: 19576@debbugs.gnu.org
> 
> I attach a fix for restoring current buffer and selected window (the
> latter hopefully taking care of selecting the right frame).  Please tell
> me whether it's sufficient for your purposes.

Thanks for working on this, Martin.  However, I don't think we should
install this change.  We call Lisp hooks from many places, including
maybe a dozen in the display engine.  It makes little sense to make
only one of them resistant to this kind of problems.  OTOH, if we do
this everywhere, I feel that we will unduly punish 99.999% percent of
legitimate users of these hooks just because one of them had a bug.

I think this is a clear bug in follow.el, and should be fixed there,
and nowhere else.  Perhaps we should also have some prominent warnings
in the documentation about this gotcha, so that the probability this
will happen again becomes lower.

Thanks.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Tue, 17 Nov 2015 20:02:04 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: 19576@debbugs.gnu.org
> 
> On Tue, Nov 17, 2015 at 02:55:39AM +0200, Juri Linkov wrote:
> > >> Conceptually it should be easy to do that.  Save/restore current buffer,
> > >> selected window and frame.  But Alan (concerned about ‘follow-mode’),
> > >> Pip (who unfortunately disappeared) and Eli are currently discussing how
> > >> to fix ‘window-size-change-functions’ in various other ways as well.
> 
> I have a fix for the `window-size-change-functions' problem, which I
> posted just over an hour ago (see bug #21869 or #21333).  The fix
> consists of only invoking w-s-c-f after any change to the echo area size
> has been done.  This might have some relevance for the current bug.  (I
> haven't followed the current bug, I'm afraid.)  I really need the
> go-ahead from Eli before I can commit the fix to the emacs-25 or master
> branch.

Thanks for working on this, and sorry for the delay in reviewing your
suggested changes.

> I propose the following strategy to fix the bug:
> 
> 1. Call resize_mini_window from redisplay_internal before the call to
>   prepare_menu_bars.
> 2. Remove the call to resize_mini_window from display_echo_area_1, and
>   make that function of type void.
> 3. Change the contract of echo_area_display, such that the echo area
>   must have been set to the correct height before calling it.
> 4. Adapt message3_nolog (the only other function which calls
>   echo_area_display) to call resize_mini_window.
> 
> As a result of these changes, any change in the size of the echo area
> would be taken into account when invoking window-size-change-functions.

I must say I prefer to avoid changes in the processing order of the
display engine, unless they are absolutely necessary and we understand
very well the effect of the order change.  Which IMO is not the case
here.

Could you try a simpler patch below?  It seems to fix both your test
case and the one originally reported in bug#21333.

Martin, is there any reason why window_resize_apply doesn't set the
frame's window_sizes_changed flag, but instead relies on its callers
to do that?

--- src/window.c~0	2015-11-11 07:57:56.000000000 +0200
+++ src/window.c	2015-11-18 18:47:51.875303700 +0200
@@ -4555,6 +4555,7 @@ grow_mini_window (struct window *w, int 
 	  /* Enforce full redisplay of the frame.  */
 	  /* FIXME: Shouldn't window--resize-root-window-vertically do it?  */
 	  fset_redisplay (f);
+	  FRAME_WINDOW_SIZES_CHANGED (f) = true;
 	  adjust_frame_glyphs (f);
 	  unblock_input ();
 	}
@@ -4594,6 +4595,7 @@ shrink_mini_window (struct window *w, bo
 	  /* Enforce full redisplay of the frame.  */
 	  /* FIXME: Shouldn't window--resize-root-window-vertically do it?  */
 	  fset_redisplay (f);
+	  FRAME_WINDOW_SIZES_CHANGED (f) = true;
 	  adjust_frame_glyphs (f);
 	  unblock_input ();
 	}
--- src/xdisp.c~0	2015-11-11 07:57:43.000000000 +0200
+++ src/xdisp.c	2015-11-18 18:53:32.333087700 +0200
@@ -13536,6 +13536,32 @@ redisplay_internal (void)
     {
       echo_area_display (false);
 
+      /* If echo_area_display resizes the mini-window, the redisplay and
+	 window_sizes_changed flags of the selected frame are set, but
+	 it's too late for the hooks in window-size-change-functions,
+	 which have been examined already in prepare_menu_bars.  So in
+	 that case we call the hooks here only for the selected frame.  */
+      if (sf->redisplay && FRAME_WINDOW_SIZES_CHANGED (sf))
+	{
+	  Lisp_Object functions;
+	  ptrdiff_t count1 = SPECPDL_INDEX ();
+
+	  record_unwind_save_match_data ();
+
+	  /* Clear flag first in case we get an error below.  */
+	  FRAME_WINDOW_SIZES_CHANGED (sf) = false;
+	  functions = Vwindow_size_change_functions;
+
+	  while (CONSP (functions))
+	    {
+	      if (!EQ (XCAR (functions), Qt))
+		call1 (XCAR (functions), selected_frame);
+	      functions = XCDR (functions);
+	    }
+
+	  unbind_to (count1, Qnil);
+	}
+
       if (message_cleared_p)
 	update_miniwindow_p = true;
 

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Tue, 17 Nov 2015 22:15:07 +0100
> From: Anders Lindgren <andlind@gmail.com>
> Cc: martin rudalics <rudalics@gmx.at>, Alan Mackenzie <acm@muc.de>, 19576@debbugs.gnu.org, 
> 	Eli Zaretskii <eliz@gnu.org>
> 
> * The Emacs display engine tries to recenter windows when window-start ==
> buffer-end, to ensure that no window would appear empty. When follow-mode
> is enabled, this is not what we want (except for the leftmost window). To
> counter the recentering efforts of Emacs, follow-mode sets window-start
> whenever it gets a chance. I would prefer if it was possible to tell the
> display engine to leave some windows alone, preferably with some kind of
> buffer-local `recenter-window-functions' variable. It could be made generic
> so that the return value would be an integer telling Emacs to show this
> many lines (if positive) or place the last line X lines from the bottom
> (when negative). If such hook existed, follow-mode could return 0 for all
> (except the leftmost) windows. Note that the user can change the content of
> any window at any time, so this property isn't static.

I don't understand why

    (set-window-start WINDOW POS t)

is not sufficient.  It does force the display engine to honor the
window-start position requested by the call; no recentering will take
place.  You say you "would prefer if it was possible to tell the
display engine to leave some windows alone", but that's exactly what
the above call does, wrt the starting position of the window.  So why
isn't it sufficient?

> * Make it easier for other packages to use it. By this I mean packages that
> display information, like "grep", should be able to use follow mode to
> display a buffer using side by side windows.

But Follow is a minor-mode, right?  So why cannot Grep etc. just turn
it on?

> * When follow-mode is enabled, there is a noticeable lag when updating the
> region. You can see this by pressing C-c SPACE and holding the up or down
> key. When follow-mode is disabled, the region is updated as fast as the
> cursor moves, when enabled, it is updated in chunks.

I guess follow-mode's post-command-hook interferes with
pre-redisplay-functions used to display the region nowadays.  Please
look into this, it would be good to fix this before Emacs 25.1 is out.

> * Process output: In early versions of follow-mode, it could be used with
> any process. This was accomplished using `defadvice' on a handful of
> process-related functions. At some point in time, this system was replaced
> with a system specific to comint and compilation buffers -- as part of the
> great defadvice sweep. Personally, I would like to Emacs to provide
> `pre-process-output-functions' and `post-process-output-functions',
> allowing packages like follow-mode to perform whatever action they would
> like to the output of any process.

Such hooks will be almost trivial to provide, I think.  But I don't
think I understand what problems would such hooks solve.  Could you
elaborate?

> * If a user have columns with different widths, follow-mode can't correctly
> display long lines stretching from one window to the next. The reason for
> this is that the start position can't be placed at an arbitrary location on
> a long line, only on positions that are a multiple of the column width. I
> don't see any way to solve this without modifying the display engine.

This is a tough nut, and the real problem is not necessarily obvious.
The real problem is that the display engine _assumes_ the lines above
and below the window edge are of the same pixel width.  It uses this
assumption in its layout code and decisions.  But when follow-mode us
used in windows of unequal width, that assumption breaks.  This is a
very serious problem, because the basic design of the Emacs display
engine is that it does its job one window at a time, i.e. it assumes
that displaying a window is possible by examining only the data of
that single window, and its associated buffer.

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Hi!

I don't understand why
>
>     (set-window-start WINDOW POS t)
>
> is not sufficient.  It does force the display engine to honor the
> window-start position requested by the call; no recentering will take
> place.  You say you "would prefer if it was possible to tell the
> display engine to leave some windows alone", but that's exactly what
> the above call does, wrt the starting position of the window.  So why
> isn't it sufficient?
>

If "pos" is point-max, the window will be recentered after a while. You can
try by entering the following in the *scratch* buffer:

 (set-window-start (selected-window) (point-max) t)

Place the cursor at the end of the buffer and evaluate the expression using
M-C-x.

If you work with Emacs for a while, you will notice that the content of the
*scratch* buffer will be visible after a while. This is what
`follow-avoid-tail-recenter' is designed to avoid. (For follow-mode, this
mean that the illusion of one tall window is broken, when the visible text
doesn't reach the rightmost window.)

As I have mentioned before, I would like to have a hook variable that the
display engine can call when doing this. Packaged like follow-mode can use
this to override the default behaviour.




> > * Make it easier for other packages to use it. By this I mean packages
> that
> > display information, like "grep", should be able to use follow mode to
> > display a buffer using side by side windows.
>
> But Follow is a minor-mode, right?  So why cannot Grep etc. just turn
> it on?
>

A package should never turn follow-mode on, but it should respect it when
it is activated in buffer!

Effectively, if a package wants to display a line in a buffer, and that
line is visible in any of the windows in a follow-mode window group, that
window should be used regardless if it was the window that was selected
before.

"Grep" does in fact do the right thing, the grep hit arrow move across the
visible windows nicely.

A package like "ispell" behaves behaves worse since it, somehow, prevent
follow-mode from doing it's job. The effect is that the windows are no
longer aligned (i.e. some lines are visible in more than one window, or
some lines between two windows are no longer shown.)

Another thing that makes things difficult is that the *selected* window
group is aligned by follow-mode. If a package wants to show (but not
select) another buffer, the windows of the other buffer will not be aligned
automatically, and there is no good interface for this. (In my package
`font-lock-studio', a control buffer is shown in the selected window, but I
wanted the source buffer to be displayed aligned (if follow-mode was
active). I ended up calling follow-post-command-hook directly, which really
isn't a good practice.)


> * When follow-mode is enabled, there is a noticeable lag when updating the
> > region. You can see this by pressing C-c SPACE and holding the up or down
> > key. When follow-mode is disabled, the region is updated as fast as the
> > cursor moves, when enabled, it is updated in chunks.
>
> I guess follow-mode's post-command-hook interferes with
> pre-redisplay-functions used to display the region nowadays.  Please
> look into this, it would be good to fix this before Emacs 25.1 is out.
>

Unfortunately, I won't be able to work on this. We are expecting a baby any
day now and when it comes to Emacs-related activitied I have picked up
where Jan Djärv left of regarding the NextStep port.

Besides, this is more of a minor irritation point rather than showstopper,
so it might just as well wait.


> * Process output: In early versions of follow-mode, it could be used with
> > any process. This was accomplished using `defadvice' on a handful of
> > process-related functions. At some point in time, this system was
> replaced
> > with a system specific to comint and compilation buffers -- as part of
> the
> > great defadvice sweep. Personally, I would like to Emacs to provide
> > `pre-process-output-functions' and `post-process-output-functions',
> > allowing packages like follow-mode to perform whatever action they would
> > like to the output of any process.
>
> Such hooks will be almost trivial to provide, I think.  But I don't
> think I understand what problems would such hooks solve.  Could you
> elaborate?
>

Follow mode can be used both in plain source buffers and in process
buffers. Concretely, you can have a *shell* buffer displayed in a number of
side by side windows, where the prompt is at the bottom of the rightmost
one, and the rest shows your recent activity.

Normally, follow-mode use the post-command-hook to ensure that the windows
are aligned. However, when you type something like "ls -lR" in your shell,
output will be coming in through the process system, which is not seen by
the post-command-hook.

In my original follow-mode implementation, the process filter functions
were advices so that all process output were passed through follow-modes
own filter functions that aligned the windows and passed the output the
real filter functions. The effect was that all process buffer, regardless
of which system they used, worked with follow mode. At some point in time
it was decided that `defavice' should not be used and this system was
replaced with a simpler system only working with compilation and comint
buffers.

If generic process output hooks follow-mode could once again work for all
processes, and in a much cleaner way. I can see other packages taking
advantage of this, like packages that would color output or strip away
parts of the output etc.


> * If a user have columns with different widths, follow-mode can't
> correctly
> > display long lines stretching from one window to the next. The reason for
> > this is that the start position can't be placed at an arbitrary location
> on
> > a long line, only on positions that are a multiple of the column width. I
> > don't see any way to solve this without modifying the display engine.
>
> This is a tough nut, and the real problem is not necessarily obvious.
> The real problem is that the display engine _assumes_ the lines above
> and below the window edge are of the same pixel width.  It uses this
> assumption in its layout code and decisions.  But when follow-mode us
> used in windows of unequal width, that assumption breaks.  This is a
> very serious problem, because the basic design of the Emacs display
> engine is that it does its job one window at a time, i.e. it assumes
> that displaying a window is possible by examining only the data of
> that single window, and its associated buffer.
>

In most cases this is the sane thing to do, I guess.

However, one way to handle this is to respect an explicit
`set-window-start' position even if the column isn't a multiple of the
screen width.


> Thanks for working on this, Martin.  However, I don't think we should
> install this change.  We call Lisp hooks from many places, including
> maybe a dozen in the display engine.  It makes little sense to make
> only one of them resistant to this kind of problems.  OTOH, if we do
> this everywhere, I feel that we will unduly punish 99.999% percent of
> legitimate users of these hooks just because one of them had a bug.
>
> I think this is a clear bug in follow.el, and should be fixed there,
> and nowhere else.  Perhaps we should also have some prominent warnings
> in the documentation about this gotcha, so that the probability this
> will happen again becomes lower.

I don't agree with you on this but I respect your opinion.

This is one of the most obscure bugs I have seen when working with Emacs --
trying to figure out why on earth `write-file' would save the wrong buffer
was no easy task, even with many years of Emacs experience under my belt.

There is a risk that other package writers will stumble upon similar
problems and give up, or write it of as "unexplainable". Ensuring that the
caller saves and restores the state is a very cheap life saver.

    -- Anders

[-- Attachment #2: Type: text/html, Size: 11326 bytes --]

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Wed, 18 Nov 2015 20:23:42 +0100
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>, Alan Mackenzie <acm@muc.de>, 
> 	19576@debbugs.gnu.org
> 
>     I don't understand why
>     
>     (set-window-start WINDOW POS t)
>     
>     is not sufficient. It does force the display engine to honor the
>     window-start position requested by the call; no recentering will take
>     place. You say you "would prefer if it was possible to tell the
>     display engine to leave some windows alone", but that's exactly what
>     the above call does, wrt the starting position of the window. So why
>     isn't it sufficient?
>     
> 
> If "pos" is point-max, the window will be recentered after a while. You can try
> by entering the following in the *scratch* buffer:
> 
> (set-window-start (selected-window) (point-max) t)
> 
> Place the cursor at the end of the buffer and evaluate the expression using
> M-C-x.
> 
> If you work with Emacs for a while, you will notice that the content of the
> *scratch* buffer will be visible after a while.

Is the M-C-x part important?  (I used M-: instead.)  If it isn't, then
I don't see any spontaneous recentering after that, so a reproducible
test case will be appreciated.  Maybe something that makes the
difference hides behind "work with Emacs for a while", I don't know.

> As I have mentioned before, I would like to have a hook variable that the
> display engine can call when doing this.

When doing what?

> Packaged like follow-mode can use this to override the default
> behaviour.

The default behavior in what aspect?

> "Grep" does in fact do the right thing, the grep hit arrow move across the
> visible windows nicely.
> 
> A package like "ispell" behaves behaves worse since it, somehow, prevent
> follow-mode from doing it's job. The effect is that the windows are no longer
> aligned (i.e. some lines are visible in more than one window, or some lines
> between two windows are no longer shown.)
> 
> Another thing that makes things difficult is that the *selected* window group
> is aligned by follow-mode. If a package wants to show (but not select) another
> buffer, the windows of the other buffer will not be aligned automatically, and
> there is no good interface for this. (In my package `font-lock-studio', a
> control buffer is shown in the selected window, but I wanted the source buffer
> to be displayed aligned (if follow-mode was active). I ended up calling
> follow-post-command-hook directly, which really isn't a good practice.)

These all sound like application-level problems to me, not issues with
the display engine.  At least not in the first approximation.

>     > * Process output: In early versions of follow-mode, it could be used with
>     > any process. This was accomplished using `defadvice' on a handful of
>     > process-related functions. At some point in time, this system was
>     replaced
>     > with a system specific to comint and compilation buffers -- as part of
>     the
>     > great defadvice sweep. Personally, I would like to Emacs to provide
>     > `pre-process-output-functions' and `post-process-output-functions',
>     > allowing packages like follow-mode to perform whatever action they would
>     > like to the output of any process.
>     
>     Such hooks will be almost trivial to provide, I think. But I don't
>     think I understand what problems would such hooks solve. Could you
>     elaborate?
>     
> 
> Follow mode can be used both in plain source buffers and in process buffers.
> Concretely, you can have a *shell* buffer displayed in a number of side by side
> windows, where the prompt is at the bottom of the rightmost one, and the rest
> shows your recent activity.
> 
> Normally, follow-mode use the post-command-hook to ensure that the windows are
> aligned. However, when you type something like "ls -lR" in your shell, output
> will be coming in through the process system, which is not seen by the
> post-command-hook.

Doesn't this mean that you need a way to hook buffer text changes?
Hooking processes is not necessarily what you want, since a process
filter could eat up the output completely and not show it in any
window, in which case follow-mode shouldn't be bothered.  Right?

> However, one way to handle this is to respect an explicit `set-window-start'
> position even if the column isn't a multiple of the screen width.

Ask Alain how easy that is.

I'm telling you: this is the tip of a huge iceberg.  The display
engine was never designed to handle windows whose redisplay depends on
other windows.

> > Thanks for working on this, Martin. However, I don't think we should
> > install this change. We call Lisp hooks from many places, including
> > maybe a dozen in the display engine. It makes little sense to make
> > only one of them resistant to this kind of problems. OTOH, if we do
> > this everywhere, I feel that we will unduly punish 99.999% percent of
> > legitimate users of these hooks just because one of them had a bug.
> >
> > I think this is a clear bug in follow.el, and should be fixed there,
> > and nowhere else. Perhaps we should also have some prominent warnings
> > in the documentation about this gotcha, so that the probability this
> > will happen again becomes lower.
> 
> I don't agree with you on this but I respect your opinion.
> 
> This is one of the most obscure bugs I have seen when working with Emacs --
> trying to figure out why on earth `write-file' would save the wrong buffer was
> no easy task, even with many years of Emacs experience under my belt.
> 
> There is a risk that other package writers will stumble upon similar problems
> and give up, or write it of as "unexplainable". Ensuring that the caller saves
> and restores the state is a very cheap life saver.

It's cheap for the "perpetrators", but it distributes the cost among
the "innocent".  Sorry, I simply cannot agree to such "re-balancing"
of guilt.

And yes, I know how much time debugging a tricky bug can consume.
been there, done that.  Still, once the reason is identified, we must
find the best place to fix it.  Choosing that place frequently
involves compromises.

bug#19576: write-file writes the wrong buffer

[Alan Mackenzie]

Hello, Eli.

On Wed, Nov 18, 2015 at 07:45:03PM +0200, Eli Zaretskii wrote:
> > Date: Tue, 17 Nov 2015 20:02:04 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: 19576@debbugs.gnu.org

[ .... ]

> > I propose the following strategy to fix the bug:

> > 1. Call resize_mini_window from redisplay_internal before the call to
> >   prepare_menu_bars.
> > 2. Remove the call to resize_mini_window from display_echo_area_1, and
> >   make that function of type void.
> > 3. Change the contract of echo_area_display, such that the echo area
> >   must have been set to the correct height before calling it.
> > 4. Adapt message3_nolog (the only other function which calls
> >   echo_area_display) to call resize_mini_window.

> > As a result of these changes, any change in the size of the echo area
> > would be taken into account when invoking window-size-change-functions.

> I must say I prefer to avoid changes in the processing order of the
> display engine, unless they are absolutely necessary and we understand
> very well the effect of the order change.  Which IMO is not the case
> here.

Yes.  This thought occurred to me too, this morning.  I identified the
function calls which were "missing" before doing the echo area size
change that I inserted.  These were (in order):

    1. reconsider_clip_changes
    2. bset_update_mode_line
    3. clear_garbaged_frames
    4. inhibit_garbage_collection
    5. forget_escape_and_glyphless_faces

.  Why not duplicate some of these calls on the way to the new echo area
size change code?  1. I don't know what "clip_changes" are.  2. is
surely harmless if done twice.  3. doesn't seem connected with the
purpose of the function, but surely is expensive enough only to be done
once.  4. seems like a good idea anyway, and 5. is cheap, and may have
some effect upon the calculation of the new size of the echo area.

> Could you try a simpler patch below?  It seems to fix both your test
> case and the one originally reported in bug#21333.

It does indeed fix my test case (I haven't tried it on #21333).  However
it violates the specification of window-size-change-functions, which
says that the hook is called _before_ redisplay, not after it has
started.  I suppose one could argue over what "redisplay" means here,
but intuitively I would say it is the putting of glyphs into matrices.

Also, there may be a possibility of w-s-c-f being invoked twice in a
single redisplay action.  I don't know whether this is bad or not, but
it doesn't seem good.

One way to "solve" these problems, although it is not pretty, is to put
an invocation of w-s-c-f into display_echo_area_1, just after the echo area
has (possibly) been resized.  This invocation would also test and reset
FRAME_WINDOW_SIZES_CHANGED (f).   There would have to be further
invocation(?s) of w-s-c-f in the other arms of the pertinent conditional in
redisplay_internal.  For that pain, we could take the w-s-c-f out of
prepare_menu_bars.

> Martin, is there any reason why window_resize_apply doesn't set the
> frame's window_sizes_changed flag, but instead relies on its callers
> to do that?

[ patch tested and snipped ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19576: write-file writes the wrong buffer

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> It's cheap for the "perpetrators", but it distributes the cost among the
> "innocent". Sorry, I simply cannot agree to such "re-balancing" of guilt.

> And yes, I know how much time debugging a tricky bug can consume. been
> there, done that. Still, once the reason is identified, we must find the
> best place to fix it. Choosing that place frequently involves compromises.

I have to agree with Eli on this one.

John

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Hi,


> I don't see any spontaneous recentering after that, so a reproducible
> test case will be appreciated.  Maybe something that makes the
> difference hides behind "work with Emacs for a while", I don't know.
>

It's hard to give an exact recipe, because the recentering occurs
stochastically. Sometimes, it occurs within a few seconds, sometimes within
a minute and sometimes the window stays like it is for a long period of
time.

The fact remains that a window where window-start equals point-max (i.e. a
window displaying nothing) do recenter itself from time to time.

Today, follow-mode actively prevents this by updating window-start of "tail
windows" whenever it gets a change (in the post-command-hook and in all
display-related hooks). Clearly, this is not an elegant solution, but in
practice it works.

My proposal is to modify the display engine so that instead of simply
recentering a window, it should call a hook to determine if the recentering
should take place. This can be made more of less fancy -- a simple solution
would be to return a boolean. A more advances solution could let a lisp
function in packages to decide how many lines should be visible.

Clearly, this is not something that would affect efficiency negatively as
this would be called relative seldom, maybe once a minute, and only if the
hook variable is non-nil. In fact, it will have a positive effect on
efficiency as the current solution isn't very efficient.

Anyway, this is not something that we have to change. The current solution,
albeit clumsy, have been working for 20 years.


> These all sound like application-level problems to me, not issues with
> the display engine.  At least not in the first approximation.

Yes, I agree on this. The list I originally mailed contained all
follow-mode related thoughts, not just the display-engine related.


Doesn't this mean that you need a way to hook buffer text changes?
> Hooking processes is not necessarily what you want, since a process
> filter could eat up the output completely and not show it in any
> window, in which case follow-mode shouldn't be bothered.  Right?
>

Right. However, the difference is rather academic since it would probably
few cases where a prior filter would eat all output.


> However, one way to handle this is to respect an explicit
> `set-window-start'
> > position even if the column isn't a multiple of the screen width.
>
> Ask Alain how easy that is.
>
> I'm telling you: this is the tip of a huge iceberg.  The display
> engine was never designed to handle windows whose redisplay depends on
> other windows.


OK, lets leave it as it is for now.


When it comes to bug#19576 (write-file saves the wrong buffer). As both
John and Eli think this shouldn't be fixed in the Emacs core, I will
correct the code in follow-mode and (if needed) update the documentation to
warn others of the dragons in these waters.

Anyway, I will pull out the this follow-mode discussion, partly because I
feel it has taken the focus away from the work Alan is doing, and partly
for personal reasons. The little time I have for Emacs-related work, I will
focus on NextStep issues.

Sincerely,
    Anders Lindgren

[-- Attachment #2: Type: text/html, Size: 4385 bytes --]

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > Thanks for working on this, Martin.  However, I don't think we should
 > install this change.  We call Lisp hooks from many places, including
 > maybe a dozen in the display engine.  It makes little sense to make
 > only one of them resistant to this kind of problems.  OTOH, if we do
 > this everywhere, I feel that we will unduly punish 99.999% percent of
 > legitimate users of these hooks just because one of them had a bug.

100% agreed.  But run_window_configuration_change_hook goes a long way
saving and restoring current buffer and selected window around each call
to a function on ‘window-configuration-change-hook’.

People who put their functions on ‘window-size-change-functions’ and
‘window-configuration-change-hook’ usually don't care about the precise
reason why these function get called.  They simply want to cover all
cases where a new window appears or a specific window changes size.  Do
we really expect them to add a ‘save-window-excursion’ in one case and
avoid it in the other because it would mean unnecessary extra work?

 > I think this is a clear bug in follow.el, and should be fixed there,
 > and nowhere else.  Perhaps we should also have some prominent warnings
 > in the documentation about this gotcha, so that the probability this
 > will happen again becomes lower.

We've broken the taboo in ‘window-configuration-change-hook’.  Anything
we add now will only increase confusion.

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > Martin, is there any reason why window_resize_apply doesn't set the
 > frame's window_sizes_changed flag, but instead relies on its callers
 > to do that?

No particular reason.  But note that ‘delete-other-windows-internal’ may
bypass window_resize_apply, so in that particular case we have to
manually make sure that the window_sizes_changed flag gets set.  I hope
this is the only case where it matters.

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Thu, 19 Nov 2015 07:54:32 +0100
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>, Alan Mackenzie <acm@muc.de>, 
> 	19576@debbugs.gnu.org
> 
>      I don't see any spontaneous recentering after that, so a reproducible
>     test case will be appreciated.  Maybe something that makes the
>     difference hides behind "work with Emacs for a while", I don't know.
> 
> It's hard to give an exact recipe, because the recentering occurs stochastically. Sometimes, it occurs within a few seconds, sometimes within a minute and sometimes the window stays like it is for a long period of time.

> The fact remains that a window where window-start equals point-max (i.e. a window displaying nothing) do recenter itself from time to time.

I could only understand that if whatever you do following a call to
set-window-start includes resizing of windows or creation/deletion of
windows.  Or maybe you meant editing in that window?  Even then at
least the simple commands I tried don't do that.

The display engine generally doesn't do anything unless the screen
should change.  So if you work outside of the window whose starting
point you forced, Emacs should never do anything with that window.

> My proposal is to modify the display engine so that instead of simply recentering a window, it should call a hook to determine if the recentering should take place.

The display engine doesn't recenter because it needs recentering.
Recentering is a means to achieve a specific goal, it isn't the goal
itself.  The goal is to determine the window's starting point when the
previous starting point cannot be used for some reason.  This is a
crucial part of the display of each window -- without determining
window-start, the display engine cannot proceed with displaying the
window.

So you cannot tell the display engine not to recenter, because it
won't know how to proceed.  I could understand if you'd ask for a way
to tell the display engine not to try redisplaying a certain window.
But disabling just the recentering is not in general possible, AFAIU.

(Actually, Emacs doesn't necessarily recenter: user options like
scroll-conservatively dictate how it finds a good candidate for
window-start, and recentering is just the simplest and the fastest
method.  But this doesn't seem to matter for the purposes of this
discussion, since you'd like to suppress _any_ kind of scrolling, I
believe.)

> This can be made more of less fancy -- a simple solution would be to return a boolean. A more advances solution could let a lisp function in packages to decide how many lines should be visible.

I'm not sure I understand: how many lines are visible is determined by
the window height and the height of the font(s) used by the text
displayed there.  Once these parameters are fixed, you cannot control
the number of lines visible in a window, except by changing the window
height.  What am I missing?

>     Doesn't this mean that you need a way to hook buffer text changes?
>     Hooking processes is not necessarily what you want, since a process
>     filter could eat up the output completely and not show it in any
>     window, in which case follow-mode shouldn't be bothered.  Right?
> 
> Right. However, the difference is rather academic since it would probably few cases where a prior filter would eat all output.

ispell and gdb-mi come to mind, and there are probably more examples.

> When it comes to bug#19576 (write-file saves the wrong buffer). As both John and Eli think this shouldn't be fixed in the Emacs core, I will correct the code in follow-mode and (if needed) update the documentation to warn others of the dragons in these waters.

Thanks.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Thu, 19 Nov 2015 09:12:50 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: andlind@gmail.com, 19576@debbugs.gnu.org
> 
>  > Thanks for working on this, Martin.  However, I don't think we should
>  > install this change.  We call Lisp hooks from many places, including
>  > maybe a dozen in the display engine.  It makes little sense to make
>  > only one of them resistant to this kind of problems.  OTOH, if we do
>  > this everywhere, I feel that we will unduly punish 99.999% percent of
>  > legitimate users of these hooks just because one of them had a bug.
> 
> 100% agreed.  But run_window_configuration_change_hook goes a long way
> saving and restoring current buffer and selected window around each call
> to a function on ‘window-configuration-change-hook’.

That function isn't called by the display engine, but only by a
handful of functions that react to changes in windows.  So I really
don't consider that to be an analogous case, sorry.

> People who put their functions on ‘window-size-change-functions’ and
> ‘window-configuration-change-hook’ usually don't care about the precise
> reason why these function get called.  They simply want to cover all
> cases where a new window appears or a specific window changes size.  Do
> we really expect them to add a ‘save-window-excursion’ in one case and
> avoid it in the other because it would mean unnecessary extra work?

Yes, we do.  Hooks called by the display engine should be coded very
carefully, because they are a large part of that proverbial rope that
Emacs gives us to hang ourselves.  If they don't write those hooks
with great care, they get what they deserve.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> X-Spam-Status: No, score=3.4 required=5.0 tests=BAYES_20,FREEMAIL_FROM,
> 	RCVD_IN_DNSWL_LOW,RCVD_IN_SBL_CSS,RCVD_IN_SORBS_WEB autolearn=disabled
> 	version=3.3.2
> Date: Thu, 19 Nov 2015 09:13:11 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: juri@linkov.net, andlind@gmail.com, 19576@debbugs.gnu.org
> 
>  > Martin, is there any reason why window_resize_apply doesn't set the
>  > frame's window_sizes_changed flag, but instead relies on its callers
>  > to do that?
> 
> No particular reason.  But note that ‘delete-other-windows-internal’ may
> bypass window_resize_apply, so in that particular case we have to
> manually make sure that the window_sizes_changed flag gets set.  I hope
> this is the only case where it matters.

Would you mind doing this on the master branch, please?

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Wed, 18 Nov 2015 23:23:04 +0000
> Cc: martin rudalics <rudalics@gmx.at>, juri@linkov.net, andlind@gmail.com,
>   19576@debbugs.gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > Could you try a simpler patch below?  It seems to fix both your test
> > case and the one originally reported in bug#21333.
> 
> It does indeed fix my test case (I haven't tried it on #21333).  However
> it violates the specification of window-size-change-functions, which
> says that the hook is called _before_ redisplay, not after it has
> started.  I suppose one could argue over what "redisplay" means here,
> but intuitively I would say it is the putting of glyphs into matrices.

"Redisplay" is indeed not defined well enough, but the only reasonable
interpretation of "before redisplay" is that it happens before the
call to redisplay_internal.  And this is false for your suggested
solution as well.  It is false even by your definition, because
prepare_menu_bars already manipulates the glyph matrices, the ones it
creates for the tool bar (and also menu bar on some display types).
And display_echo_area also manipulates glyph matrices (it calls
try_window).

Which is only logical for an event that by itself is triggered as part
of redisplay!  It's redisplay that decides to resize the mini-window,
so calling the hook after that decision _cannot_ possibly count as
being "before redisplay".

IOW, once we, by popular demand, decided to call
window-size-change-functions when the mini-window is resized, we
invalidated that specification.  All the other callers of this hook
are not part of a redisplay cycle, but this one is, and cannot be
anywhere else.

So no matter what change we eventually install, the documentation of
the hook needs to be amended to say that it's called "before redisplay
or at the beginning of a redisplay cycle", and maybe also mention that
the second case is when the mini-window is resized.

(Btw, the ELisp manual wisely doesn't say "before redisplay", only the
doc string does.)

> Also, there may be a possibility of w-s-c-f being invoked twice in a
> single redisplay action.

How do you see that as a possibility?  For that to happen, the flag
indicating that the mini-window was resized should be already set for
that window when we enter redisplay, but I don't see how that could
happen, and still lead to an additional resize as part of redisplay.

I also don't think it's a catastrophe if this hook is called more than
once in some rare situations.

> One way to "solve" these problems, although it is not pretty, is to put
> an invocation of w-s-c-f into display_echo_area_1, just after the echo area
> has (possibly) been resized.  This invocation would also test and reset
> FRAME_WINDOW_SIZES_CHANGED (f).   There would have to be further
> invocation(?s) of w-s-c-f in the other arms of the pertinent conditional in
> redisplay_internal.  For that pain, we could take the w-s-c-f out of
> prepare_menu_bars.

I don't see the need for such complications, certainly not on the
release branch.

Thanks.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 >> 100% agreed.  But run_window_configuration_change_hook goes a long way
 >> saving and restoring current buffer and selected window around each call
 >> to a function on ‘window-configuration-change-hook’.
 >
 > That function isn't called by the display engine, but only by a
 > handful of functions that react to changes in windows.  So I really
 > don't consider that to be an analogous case, sorry.

Reconsider.  ‘window-size-change-functions’ are processed by the display
engine because it's the one we expect to know that window resizing
activity has ceased.  And if you look at how packages use these two
hooks (and also ‘post-command-hook’) you will see that people should get
more help.  Just to quote from ‘linum-mode’

         ;; Using both window-size-change-functions and
         ;; window-configuration-change-hook seems redundant. --Stef

and

         (add-hook 'window-configuration-change-hook
                   ;; FIXME: If the buffer is shown in N windows, this
                   ;; will be called N times rather than once.  We should use
                   ;; something like linum-update-window instead.
                   'linum-update-current nil t)

 >> People who put their functions on ‘window-size-change-functions’ and
 >> ‘window-configuration-change-hook’ usually don't care about the precise
 >> reason why these function get called.  They simply want to cover all
 >> cases where a new window appears or a specific window changes size.  Do
 >> we really expect them to add a ‘save-window-excursion’ in one case and
 >> avoid it in the other because it would mean unnecessary extra work?
 >
 > Yes, we do.  Hooks called by the display engine should be coded very
 > carefully, because they are a large part of that proverbial rope that
 > Emacs gives us to hang ourselves.  If they don't write those hooks
 > with great care, they get what they deserve.

People do care.  But how would they know that in one case we save all
sorts of things while we don't in the other.

At least the OP got what he deserved: Waiting one year for being told
that the bug he reported is his own fault.

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 >> No particular reason.  But note that ‘delete-other-windows-internal’ may
 >> bypass window_resize_apply, so in that particular case we have to
 >> manually make sure that the window_sizes_changed flag gets set.  I hope
 >> this is the only case where it matters.
 >
 > Would you mind doing this on the master branch, please?

It might have interesting consequences.  Presumably, you expect that
this way we'll be finally able to run ‘window-size-change-functions’ if
and only if the size of windows really changed.  Correct?

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > IOW, once we, by popular demand, decided to call
 > window-size-change-functions when the mini-window is resized, we
 > invalidated that specification.  All the other callers of this hook
 > are not part of a redisplay cycle, but this one is, and cannot be
 > anywhere else.

I still wonder what will happen when the function called by
‘window-size-change-functions’ triggers a message that resizes the
mini-window.

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 09:22:15 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: andlind@gmail.com, 19576@debbugs.gnu.org
> 
>  >> People who put their functions on ‘window-size-change-functions’ and
>  >> ‘window-configuration-change-hook’ usually don't care about the precise
>  >> reason why these function get called.  They simply want to cover all
>  >> cases where a new window appears or a specific window changes size.  Do
>  >> we really expect them to add a ‘save-window-excursion’ in one case and
>  >> avoid it in the other because it would mean unnecessary extra work?
>  >
>  > Yes, we do.  Hooks called by the display engine should be coded very
>  > carefully, because they are a large part of that proverbial rope that
>  > Emacs gives us to hang ourselves.  If they don't write those hooks
>  > with great care, they get what they deserve.
> 
> People do care.  But how would they know that in one case we save all
> sorts of things while we don't in the other.

They don't need to.  We won't tell them that we do that even in a
single case.

> At least the OP got what he deserved: Waiting one year for being told
> that the bug he reported is his own fault.

Yes, our bug-handling routine can and should be improved.  I'm sorry
he had to wait for so long.  To be fair, the original report already
put the blame squarely on installing a "broken window size change
function", so I don't think what I wrote here 10 months later is a
revelation of any kind.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 09:22:28 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
>  19576@debbugs.gnu.org
> 
>  >> No particular reason.  But note that ‘delete-other-windows-internal’ may
>  >> bypass window_resize_apply, so in that particular case we have to
>  >> manually make sure that the window_sizes_changed flag gets set.  I hope
>  >> this is the only case where it matters.
>  >
>  > Would you mind doing this on the master branch, please?
> 
> It might have interesting consequences.  Presumably, you expect that
> this way we'll be finally able to run ‘window-size-change-functions’ if
> and only if the size of windows really changed.  Correct?

No, not really.  It's just a cleanup, IMO: instead of asking each
caller of window_resize_apply to set that flag, set it in that
function itself.  I didn't envision any changes in functionality due
to this change.  What did I miss?

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 09:22:41 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: juri@linkov.net, andlind@gmail.com, 19576@debbugs.gnu.org
> 
> I still wonder what will happen when the function called by
> ‘window-size-change-functions’ triggers a message that resizes the
> mini-window.

That was the test case in bug#21333.  I tried it after applying the
patch, and saw the expected resize messages.  Does that answer your
concerns?

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 >> It might have interesting consequences.  Presumably, you expect that
 >> this way we'll be finally able to run ‘window-size-change-functions’ if
 >> and only if the size of windows really changed.  Correct?
 >
 > No, not really.  It's just a cleanup, IMO: instead of asking each
 > caller of window_resize_apply to set that flag, set it in that
 > function itself.

In addition it would do that then for grow_mini_window and
shrink_mini_window too but I suppose that's what you want.

 > I didn't envision any changes in functionality due
 > to this change.  What did I miss?

"All" window size changes must pass through window_resize_apply.  So we
could easily check there if any size really changes and set that flag
only in that case.  This way you would get your check whether
‘set-window-configuration’ did change the size of any window for free.

It's not a minor change since I probably would remove the check done by
‘window--resize-apply-p’, have window_resize_apply return a boolean
indicating that some size really changed and have ‘window-resize-apply’
pass that value back to its caller so we can avoid calling
‘window--pixel-to-total’ and ‘window-configuration-change-hook’ if
nothing changed.  And, as I mentioned earlier, the one case in
‘delete-other-windows’ would have to be treated specially.  But I think
it's worth if we care about ‘set-window-configuration’.

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > That was the test case in bug#21333.  I tried it after applying the
 > patch, and saw the expected resize messages.  Does that answer your
 > concerns?

Does it run ‘window-size-change-functions’ again?

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 11:25:47 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
>  19576@debbugs.gnu.org
> 
>  >> It might have interesting consequences.  Presumably, you expect that
>  >> this way we'll be finally able to run ‘window-size-change-functions’ if
>  >> and only if the size of windows really changed.  Correct?
>  >
>  > No, not really.  It's just a cleanup, IMO: instead of asking each
>  > caller of window_resize_apply to set that flag, set it in that
>  > function itself.
> 
> In addition it would do that then for grow_mini_window and
> shrink_mini_window too but I suppose that's what you want.

Indeed, that was what triggered my question.

>  > I didn't envision any changes in functionality due
>  > to this change.  What did I miss?
> 
> "All" window size changes must pass through window_resize_apply.  So we
> could easily check there if any size really changes and set that flag
> only in that case.  This way you would get your check whether
> ‘set-window-configuration’ did change the size of any window for free.

Yes, that'd be an additional benefit.

> It's not a minor change since I probably would remove the check done by
> ‘window--resize-apply-p’, have window_resize_apply return a boolean
> indicating that some size really changed and have ‘window-resize-apply’
> pass that value back to its caller so we can avoid calling
> ‘window--pixel-to-total’ and ‘window-configuration-change-hook’ if
> nothing changed.  And, as I mentioned earlier, the one case in
> ‘delete-other-windows’ would have to be treated specially.  But I think
> it's worth if we care about ‘set-window-configuration’.

Yes, and we are allowed non-minor changes on master.

Thanks.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 11:48:00 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
>  19576@debbugs.gnu.org
> 
>  > That was the test case in bug#21333.  I tried it after applying the
>  > patch, and saw the expected resize messages.  Does that answer your
>  > concerns?
> 
> Does it run ‘window-size-change-functions’ again?

Yes, because I counted the number of those messages, and saw the
expected number of them.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > Yes, and we are allowed non-minor changes on master.

I can't test emacs-25 anyway.

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 >> Does it run ‘window-size-change-functions’ again?
 >
 > Yes, because I counted the number of those messages, and saw the
 > expected number of them.

Then it conceptually can run into a loop where it continuously grows and
shrinks the mini window?

martin

bug#19576: write-file writes the wrong buffer

[Alan Mackenzie]

Hello, Eli.

On Fri, Nov 20, 2015 at 01:15:39PM +0200, Eli Zaretskii wrote:
> > Date: Fri, 20 Nov 2015 11:25:47 +0100
> > From: martin rudalics <rudalics@gmx.at>
> > CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
> >  19576@debbugs.gnu.org

[ .... ]

> > It's not a minor change since I probably would remove the check done by
> > ‘window--resize-apply-p’, have window_resize_apply return a boolean
> > indicating that some size really changed and have ‘window-resize-apply’
> > pass that value back to its caller so we can avoid calling
> > ‘window--pixel-to-total’ and ‘window-configuration-change-hook’ if
> > nothing changed.  And, as I mentioned earlier, the one case in
> > ‘delete-other-windows’ would have to be treated specially.  But I think
> > it's worth if we care about ‘set-window-configuration’.

> Yes, and we are allowed non-minor changes on master.

Please don't decide that this bug won't be fixed for Emacs 25.1.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 12:25:44 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
>  19576@debbugs.gnu.org
> 
>  > Yes, and we are allowed non-minor changes on master.
> 
> I can't test emacs-25 anyway.

Because of loaddefs?  You should be able to produce them with a
different Emacs binary.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 12:25:51 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
>  19576@debbugs.gnu.org
> 
>  >> Does it run ‘window-size-change-functions’ again?
>  >
>  > Yes, because I counted the number of those messages, and saw the
>  > expected number of them.
> 
> Then it conceptually can run into a loop where it continuously grows and
> shrinks the mini window?

What loop?  Redisplay doesn't call 'message'; calling 'message' enters
another redisplay.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 11:32:14 +0000
> Cc: martin rudalics <rudalics@gmx.at>, juri@linkov.net, andlind@gmail.com,
>   19576@debbugs.gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> Hello, Eli.
> 
> On Fri, Nov 20, 2015 at 01:15:39PM +0200, Eli Zaretskii wrote:
> > > Date: Fri, 20 Nov 2015 11:25:47 +0100
> > > From: martin rudalics <rudalics@gmx.at>
> > > CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
> > >  19576@debbugs.gnu.org
> 
> [ .... ]
> 
> > > It's not a minor change since I probably would remove the check done by
> > > ‘window--resize-apply-p’, have window_resize_apply return a boolean
> > > indicating that some size really changed and have ‘window-resize-apply’
> > > pass that value back to its caller so we can avoid calling
> > > ‘window--pixel-to-total’ and ‘window-configuration-change-hook’ if
> > > nothing changed.  And, as I mentioned earlier, the one case in
> > > ‘delete-other-windows’ would have to be treated specially.  But I think
> > > it's worth if we care about ‘set-window-configuration’.
> 
> > Yes, and we are allowed non-minor changes on master.
> 
> Please don't decide that this bug won't be fixed for Emacs 25.1.

This issue doesn't affect this bug.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 >> Then it conceptually can run into a loop where it continuously grows and
 >> shrinks the mini window?
 >
 > What loop?  Redisplay doesn't call 'message';

I meant the code run by ‘window-size-change-functions’ might call
‘message’.

 > calling 'message' enters
 > another redisplay.

Then there should be no problems.

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > Because of loaddefs?  You should be able to produce them with a
 > different Emacs binary.

I'll rather wait a few days.  I'm in no hurry.

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Fri, 20 Nov 2015 15:21:07 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, juri@linkov.net, andlind@gmail.com, 
>  19576@debbugs.gnu.org
> 
>  >> Then it conceptually can run into a loop where it continuously grows and
>  >> shrinks the mini window?
>  >
>  > What loop?  Redisplay doesn't call 'message';
> 
> I meant the code run by ‘window-size-change-functions’ might call
> ‘message’.

In that case, redisplay won't be called by that invocation of
'message'.

Moreover, redisplay_internal doesn't allow to invoke itself
recursively (although I never saw it happening), it returns
immediately if that happens.

bug#19576: Fixed: bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

After discussing this with Eli and John, it was decided that the
responsibility to restore the current buffer lies on the lisp function and
not the Emacs code that calls `window-size-change-functions'.

I committed a patch to follow.el that ensures that the buffer is restored,
see
http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=75a1d009f747a220c7b9b1cfdbe7077082fe02d6
for details.

    -- Anders Lindgren

[-- Attachment #2: Type: text/html, Size: 645 bytes --]

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Thu, 19 Nov 2015 18:03:39 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net
> 
> > Date: Wed, 18 Nov 2015 23:23:04 +0000
> > Cc: martin rudalics <rudalics@gmx.at>, juri@linkov.net, andlind@gmail.com,
> >   19576@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>
> > 
> > > Could you try a simpler patch below?  It seems to fix both your test
> > > case and the one originally reported in bug#21333.
> > 
> > It does indeed fix my test case (I haven't tried it on #21333).  However
> > it violates the specification of window-size-change-functions, which
> > says that the hook is called _before_ redisplay, not after it has
> > started.  I suppose one could argue over what "redisplay" means here,
> > but intuitively I would say it is the putting of glyphs into matrices.
> 
> "Redisplay" is indeed not defined well enough, but the only reasonable
> interpretation of "before redisplay" is that it happens before the
> call to redisplay_internal.  And this is false for your suggested
> solution as well.  It is false even by your definition, because
> prepare_menu_bars already manipulates the glyph matrices, the ones it
> creates for the tool bar (and also menu bar on some display types).
> And display_echo_area also manipulates glyph matrices (it calls
> try_window).
> 
> Which is only logical for an event that by itself is triggered as part
> of redisplay!  It's redisplay that decides to resize the mini-window,
> so calling the hook after that decision _cannot_ possibly count as
> being "before redisplay".
> 
> IOW, once we, by popular demand, decided to call
> window-size-change-functions when the mini-window is resized, we
> invalidated that specification.  All the other callers of this hook
> are not part of a redisplay cycle, but this one is, and cannot be
> anywhere else.
> 
> So no matter what change we eventually install, the documentation of
> the hook needs to be amended to say that it's called "before redisplay
> or at the beginning of a redisplay cycle", and maybe also mention that
> the second case is when the mini-window is resized.

No further comments, so I've committed the changes I posted here
earlier.  I also modified the documentation to be consistent with what
the code does.

Please tell me if this bug and bug#21333 could now be closed, or if
there are any leftovers.

Thanks.

bug#19576: write-file writes the wrong buffer

[Alan Mackenzie]

Hello, Eli.

On Sat, Nov 21, 2015 at 01:35:02PM +0200, Eli Zaretskii wrote:
> > Date: Thu, 19 Nov 2015 18:03:39 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net

> > > Date: Wed, 18 Nov 2015 23:23:04 +0000
> > > Cc: martin rudalics <rudalics@gmx.at>, juri@linkov.net, andlind@gmail.com,
> > >   19576@debbugs.gnu.org
> > > From: Alan Mackenzie <acm@muc.de>

> > > > Could you try a simpler patch below?  It seems to fix both your test
> > > > case and the one originally reported in bug#21333.

> > > It does indeed fix my test case (I haven't tried it on #21333).  However
> > > it violates the specification of window-size-change-functions, which
> > > says that the hook is called _before_ redisplay, not after it has
> > > started.  I suppose one could argue over what "redisplay" means here,
> > > but intuitively I would say it is the putting of glyphs into matrices.

> > "Redisplay" is indeed not defined well enough, but the only reasonable
> > interpretation of "before redisplay" is that it happens before the
> > call to redisplay_internal.  And this is false for your suggested
> > solution as well.  It is false even by your definition, because
> > prepare_menu_bars already manipulates the glyph matrices, the ones it
> > creates for the tool bar (and also menu bar on some display types).
> > And display_echo_area also manipulates glyph matrices (it calls
> > try_window).

> > Which is only logical for an event that by itself is triggered as part
> > of redisplay!  It's redisplay that decides to resize the mini-window,
> > so calling the hook after that decision _cannot_ possibly count as
> > being "before redisplay".

> > IOW, once we, by popular demand, decided to call
> > window-size-change-functions when the mini-window is resized, we
> > invalidated that specification.  All the other callers of this hook
> > are not part of a redisplay cycle, but this one is, and cannot be
> > anywhere else.

> > So no matter what change we eventually install, the documentation of
> > the hook needs to be amended to say that it's called "before redisplay
> > or at the beginning of a redisplay cycle", and maybe also mention that
> > the second case is when the mini-window is resized.

> No further comments, so I've committed the changes I posted here
> earlier.  I also modified the documentation to be consistent with what
> the code does.

Thanks.

> Please tell me if this bug and bug#21333 could now be closed, or if
> there are any leftovers.

I don't think this fix concerns #19576.  The discussion of #21869 seems,
somehow, to have moved to here.

But I think that #21333 can be closed, and #21869 certainly can be; both
of these were about window-size-change-functions not getting called for
echo area size changes.

> Thanks.

Ditto!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sat, 21 Nov 2015 15:56:58 +0000
> Cc: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net
> From: Alan Mackenzie <acm@muc.de>
> 
> > Please tell me if this bug and bug#21333 could now be closed, or if
> > there are any leftovers.
> 
> I don't think this fix concerns #19576.  The discussion of #21869 seems,
> somehow, to have moved to here.

Right.  But then 19576 should be closed "for other reasons", right?

> But I think that #21333 can be closed, and #21869 certainly can be; both
> of these were about window-size-change-functions not getting called for
> echo area size changes.

Will do.

bug#21333: bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sat, 21 Nov 2015 15:56:58 +0000
> Cc: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net
> From: Alan Mackenzie <acm@muc.de>
> 
> Hello, Eli.
> 
> On Sat, Nov 21, 2015 at 01:35:02PM +0200, Eli Zaretskii wrote:
> > > Date: Thu, 19 Nov 2015 18:03:39 +0200
> > > From: Eli Zaretskii <eliz@gnu.org>
> > > Cc: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net
> 
> > > > Date: Wed, 18 Nov 2015 23:23:04 +0000
> > > > Cc: martin rudalics <rudalics@gmx.at>, juri@linkov.net, andlind@gmail.com,
> > > >   19576@debbugs.gnu.org
> > > > From: Alan Mackenzie <acm@muc.de>
> 
> > > > > Could you try a simpler patch below?  It seems to fix both your test
> > > > > case and the one originally reported in bug#21333.
> 
> > > > It does indeed fix my test case (I haven't tried it on #21333).  However
> > > > it violates the specification of window-size-change-functions, which
> > > > says that the hook is called _before_ redisplay, not after it has
> > > > started.  I suppose one could argue over what "redisplay" means here,
> > > > but intuitively I would say it is the putting of glyphs into matrices.
> 
> > > "Redisplay" is indeed not defined well enough, but the only reasonable
> > > interpretation of "before redisplay" is that it happens before the
> > > call to redisplay_internal.  And this is false for your suggested
> > > solution as well.  It is false even by your definition, because
> > > prepare_menu_bars already manipulates the glyph matrices, the ones it
> > > creates for the tool bar (and also menu bar on some display types).
> > > And display_echo_area also manipulates glyph matrices (it calls
> > > try_window).
> 
> > > Which is only logical for an event that by itself is triggered as part
> > > of redisplay!  It's redisplay that decides to resize the mini-window,
> > > so calling the hook after that decision _cannot_ possibly count as
> > > being "before redisplay".
> 
> > > IOW, once we, by popular demand, decided to call
> > > window-size-change-functions when the mini-window is resized, we
> > > invalidated that specification.  All the other callers of this hook
> > > are not part of a redisplay cycle, but this one is, and cannot be
> > > anywhere else.
> 
> > > So no matter what change we eventually install, the documentation of
> > > the hook needs to be amended to say that it's called "before redisplay
> > > or at the beginning of a redisplay cycle", and maybe also mention that
> > > the second case is when the mini-window is resized.
> 
> > No further comments, so I've committed the changes I posted here
> > earlier.  I also modified the documentation to be consistent with what
> > the code does.
> 
> Thanks.
> 
> > Please tell me if this bug and bug#21333 could now be closed, or if
> > there are any leftovers.
> 
> I don't think this fix concerns #19576.  The discussion of #21869 seems,
> somehow, to have moved to here.
> 
> But I think that #21333 can be closed, and #21869 certainly can be; both
> of these were about window-size-change-functions not getting called for
> echo area size changes.

Closing.

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

>
> Right.  But then 19576 should be closed "for other reasons", right?
>

Correct, I fixed and closed 19576 a couple of days ago. I did the change in
follow-mode as Eli suggested.


However, while we are discussing window-size-change-functions -- I noticed
that on OS X and Windows, it is not called when the user manually resized a
frame, whereas on X11 it is. It seems logical that is should be called --
should I file this as a bug (or one for each interface)?

    -- Anders

[-- Attachment #2: Type: text/html, Size: 780 bytes --]

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sat, 21 Nov 2015 17:49:45 +0100
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Alan Mackenzie <acm@muc.de>, 19576@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
> 
> Correct, I fixed and closed 19576 a couple of days ago. I did the change in
> follow-mode as Eli suggested.

Thanks.

> However, while we are discussing window-size-change-functions -- I noticed that
> on OS X and Windows, it is not called when the user manually resized a frame,
> whereas on X11 it is. It seems logical that is should be called -- should I
> file this as a bug (or one for each interface)?

Please file a single bug report for both platforms.

Thanks.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > However, while we are discussing window-size-change-functions -- I noticed
 > that on OS X and Windows, it is not called when the user manually resized a
 > frame, whereas on X11 it is. It seems logical that is should be called --
 > should I file this as a bug (or one for each interface)?

I can't imagine how this got called for X11 - something else must have
been intervening here.

Resizing a frame calls ‘window-configuration-change-hook’ instead.  In
Emacs 22 change_frame_size_1 still had

   /* This isn't quite a no-op: it runs window-configuration-change-hook.  */
   Fset_window_buffer (FRAME_SELECTED_WINDOW (f),
		      XWINDOW (FRAME_SELECTED_WINDOW (f))->buffer, Qt);

while in Emacs 23 change_frame_size_1 already uses

   run_window_configuration_change_hook (f);

Obviously ‘window-size-change-functions’ seems more logical here but
might break packages that expect the old behavior.  Running both hooks
for frame resizes doesn't seem overly clever but we do so already when
deleting and splitting windows.  Anyway, this is a can of worms.

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sat, 21 Nov 2015 19:27:38 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: Alan Mackenzie <acm@muc.de>, 19576@debbugs.gnu.org, 
>  Juri Linkov <juri@linkov.net>
> 
> Resizing a frame calls ‘window-configuration-change-hook’ instead.  In
> Emacs 22 change_frame_size_1 still had
> 
>    /* This isn't quite a no-op: it runs window-configuration-change-hook.  */
>    Fset_window_buffer (FRAME_SELECTED_WINDOW (f),
> 		      XWINDOW (FRAME_SELECTED_WINDOW (f))->buffer, Qt);
> 
> while in Emacs 23 change_frame_size_1 already uses
> 
>    run_window_configuration_change_hook (f);
> 
> Obviously ‘window-size-change-functions’ seems more logical here but
> might break packages that expect the old behavior.  Running both hooks
> for frame resizes doesn't seem overly clever but we do so already when
> deleting and splitting windows.  Anyway, this is a can of worms.

We could run both types of hooks, couldn't we?  The documentation
seems to suggest that both of them should be run in this situation.

As for "can of worms", we could make that change on master and see
what breaks.  WDYT?

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > We could run both types of hooks, couldn't we?

As I said we do that already for splitting and deleting windows.  So we
obviously could.  But this means that packages run the same function
twice because they are used to run the same function in both hooks.

 > The documentation
 > seems to suggest that both of them should be run in this situation.

‘window-configuration-change-hook’ shouldn't run since the window
configuration does not change.

 > As for "can of worms", we could make that change on master and see
 > what breaks.  WDYT?

It won't break anything.  But why should we OT1H make
‘window-size-change-functions’ more efficient when OTOH we call it after
we already called ‘window-configuration-change-hook’?

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sat, 21 Nov 2015 19:44:07 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: andlind@gmail.com, acm@muc.de, 19576@debbugs.gnu.org, 
>  juri@linkov.net
> 
>  > We could run both types of hooks, couldn't we?
> 
> As I said we do that already for splitting and deleting windows.  So we
> obviously could.  But this means that packages run the same function
> twice because they are used to run the same function in both hooks.

That'[s why I think we should do this on master, to see if this causes
any problems.

>  > The documentation
>  > seems to suggest that both of them should be run in this situation.
> 
> ‘window-configuration-change-hook’ shouldn't run since the window
> configuration does not change.

But the ELisp manual says it should:

   -- Variable: window-configuration-change-hook
       A normal hook that is run every time you change the window
       configuration of an existing frame.  This includes splitting or
       deleting windows, changing the sizes of windows, or displaying a
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       different buffer in a window.

> But why should we OT1H make ‘window-size-change-functions’ more
> efficient when OTOH we call it after we already called
> ‘window-configuration-change-hook’?

Sorry, I don't see the relevance of efficiency to this issue.

bug#19576: write-file writes the wrong buffer

[Anders Lindgren]

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

Auhm -- when starting to write the bug report I decided to make sure I got
all the facts right.

Unfortunately, it turned out that I had run an old Emacs 23 on X11 -- which
do run the window-size-change-functions. When testing on an Emacs 25 on
X11, it turns out that it doesn't. Back on the mac, I see the same pattern.

In other words: On Emacs 23, window-size-change-functions is called when
the user manually resized the frame. On Emacs 24 and Emacs 25 it isn't.

Sorry for the mixup.

Anyway, I just reported this regression as bug#21975.

I noticed this since Follow-mode no longer aligns its windows automatically
when the user resized the frame. In practice, this is not a big problem
since they will become aligned at the first user interaction instead.

    -- Anders


On Sat, Nov 21, 2015 at 8:08 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Sat, 21 Nov 2015 19:44:07 +0100
> > From: martin rudalics <rudalics@gmx.at>
> > CC: andlind@gmail.com, acm@muc.de, 19576@debbugs.gnu.org,
> >  juri@linkov.net
> >
> >  > We could run both types of hooks, couldn't we?
> >
> > As I said we do that already for splitting and deleting windows.  So we
> > obviously could.  But this means that packages run the same function
> > twice because they are used to run the same function in both hooks.
>
> That'[s why I think we should do this on master, to see if this causes
> any problems.
>
> >  > The documentation
> >  > seems to suggest that both of them should be run in this situation.
> >
> > ‘window-configuration-change-hook’ shouldn't run since the window
> > configuration does not change.
>
> But the ELisp manual says it should:
>
>    -- Variable: window-configuration-change-hook
>        A normal hook that is run every time you change the window
>        configuration of an existing frame.  This includes splitting or
>        deleting windows, changing the sizes of windows, or displaying a
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>        different buffer in a window.
>
> > But why should we OT1H make ‘window-size-change-functions’ more
> > efficient when OTOH we call it after we already called
> > ‘window-configuration-change-hook’?
>
> Sorry, I don't see the relevance of efficiency to this issue.
>

[-- Attachment #2: Type: text/html, Size: 3196 bytes --]

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > That'[s why I think we should do this on master, to see if this causes
 > any problems.

It shouldn't because, as Anders remarked, it already worked this way in
Emacs 23.  So I tend to install this on Emacs-25.

 >>   > The documentation
 >>   > seems to suggest that both of them should be run in this situation.
 >>
 >> ‘window-configuration-change-hook’ shouldn't run since the window
 >> configuration does not change.
 >
 > But the ELisp manual says it should:
 >
 >     -- Variable: window-configuration-change-hook
 >         A normal hook that is run every time you change the window
 >         configuration of an existing frame.  This includes splitting or
 >         deleting windows, changing the sizes of windows, or displaying a
 >                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 >         different buffer in a window.

Taken literally we'd then have to run ‘window-configuration-change-hook’
also when we resize windows without resizing the frame.  In particular,
we'd then have to run it in prepare_menu_bars and in redisplay_internal.
Do we really want to do that?  If so, we could easily obsolete
‘window-size-change-functions’ because then every case would have been
already covered by ‘window-configuration-change-hook’ before.

martin

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > I also modified the documentation to be consistent with what
 > the code does.

The Elisp manual now says

      The functions are called at the
      beginning of a redisplay cycle, and just once for each frame on
                                          ^^^^^^^^^
      which size changes have occurred.

IIUC the functions may now be called twice for the selected frame: A
first time when a "normal" size changed occurred and a second time when
the mini-window got resized.  Or am I missing something?

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sun, 22 Nov 2015 11:44:56 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: andlind@gmail.com, acm@muc.de, 19576@debbugs.gnu.org, 
>  juri@linkov.net
> 
>  > That'[s why I think we should do this on master, to see if this causes
>  > any problems.
> 
> It shouldn't because, as Anders remarked, it already worked this way in
> Emacs 23.  So I tend to install this on Emacs-25.

Fine with me.

>  >     -- Variable: window-configuration-change-hook
>  >         A normal hook that is run every time you change the window
>  >         configuration of an existing frame.  This includes splitting or
>  >         deleting windows, changing the sizes of windows, or displaying a
>  >                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  >         different buffer in a window.
> 
> Taken literally we'd then have to run ‘window-configuration-change-hook’
> also when we resize windows without resizing the frame.  In particular,
> we'd then have to run it in prepare_menu_bars and in redisplay_internal.
> Do we really want to do that?  If so, we could easily obsolete
> ‘window-size-change-functions’ because then every case would have been
> already covered by ‘window-configuration-change-hook’ before.

Fine with me, too, although this change is incompatible, and so should
be made on master, IMO.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sun, 22 Nov 2015 12:08:42 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net
> 
>  > I also modified the documentation to be consistent with what
>  > the code does.
> 
> The Elisp manual now says
> 
>       The functions are called at the
>       beginning of a redisplay cycle, and just once for each frame on
>                                           ^^^^^^^^^
>       which size changes have occurred.
> 
> IIUC the functions may now be called twice for the selected frame: A
> first time when a "normal" size changed occurred and a second time when
> the mini-window got resized.  Or am I missing something?

I couldn't think of any situation that would cause that.  It would
require a resize of windows outside redisplay that would also cause
redisplay to decide that the mini-window needs to be resized.  If you
can come up with a recipe for this, we should indeed remove the "just
once" promise.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > I couldn't think of any situation that would cause that.  It would
 > require a resize of windows outside redisplay that would also cause
 > redisplay to decide that the mini-window needs to be resized.  If you
 > can come up with a recipe for this, we should indeed remove the "just
 > once" promise.

A silly example with emacs -Q is

(defvar fun 0)

(defun fun (frame)
   "..."
   (message "\n%s\n" (setq fun (1+ fun))) (ding))

(add-hook 'window-size-change-functions 'fun)

This gets me complete erratic behavior when moving point.

Another example is

(defvar fun 0)

(defun fun (frame)
   "..."
   (message "%s" (setq fun (1+ fun))) (ding))

(add-hook 'window-size-change-functions 'fun)

(progn
   (tool-bar-mode -1)
   (sit-for 1)
   (tool-bar-mode 1))

With my patch for bug#21975 this runs ‘fun’ twice when the tool-bar is
reenabled.  Likely due to my calculations of the real tool bar height.

martin

bug#19576: write-file writes the wrong buffer

[Johan Bockgård]

Eli Zaretskii <eliz@gnu.org> writes:

>> The fact remains that a window where window-start equals point-max
>> (i.e. a window displaying nothing) do recenter itself from time to
>> time.
>
> I could only understand that if whatever you do following a call to
> set-window-start includes resizing of windows or creation/deletion of
> windows.  Or maybe you meant editing in that window?  Even then at
> least the simple commands I tried don't do that.
>
> The display engine generally doesn't do anything unless the screen
> should change.  So if you work outside of the window whose starting
> point you forced, Emacs should never do anything with that window.

E.g. the following cause a recenter of *scratch*:

emacs -Q
M-: (set-window-start (selected-window) (point-max) t) RET
C-x 2
Click mouse in other window

or

emacs -Q
M-: (set-window-start (selected-window) (point-max) t) RET
C-x 2
C-x o
C-h i

or

emacs -Q
M-: (set-window-start (selected-window) (point-max) t) RET
Resize frame

or...

bug#19576: write-file writes the wrong buffer

[Johan Bockgård]

Johan Bockgård <bojohan@gnu.org> writes:

> or...

emacs -Q
M-: (set-window-start (selected-window) (point-max) t) RET
C-x 2
C-x o
C-s

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> From: Johan Bockgård <bojohan@gnu.org>
> Cc: Anders Lindgren <andlind@gmail.com>,  19576@debbugs.gnu.org
> Date: Sun, 22 Nov 2015 19:44:04 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I could only understand that if whatever you do following a call to
> > set-window-start includes resizing of windows or creation/deletion of
> > windows.  Or maybe you meant editing in that window?  Even then at
> > least the simple commands I tried don't do that.
> >
> > The display engine generally doesn't do anything unless the screen
> > should change.  So if you work outside of the window whose starting
> > point you forced, Emacs should never do anything with that window.
> 
> E.g. the following cause a recenter of *scratch*:
> 
> emacs -Q
> M-: (set-window-start (selected-window) (point-max) t) RET
> C-x 2
> Click mouse in other window
> 
> or
> 
> emacs -Q
> M-: (set-window-start (selected-window) (point-max) t) RET
> C-x 2
> C-x o
> C-h i
> 
> or
> 
> emacs -Q
> M-: (set-window-start (selected-window) (point-max) t) RET
> Resize frame

That's what I said (see above): resizing of windows or
creation/deletion of windows can indeed cause recentering.

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sun, 22 Nov 2015 18:46:19 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, 19576@debbugs.gnu.org, andlind@gmail.com, 
>  juri@linkov.net
> 
>  > I couldn't think of any situation that would cause that.  It would
>  > require a resize of windows outside redisplay that would also cause
>  > redisplay to decide that the mini-window needs to be resized.  If you
>  > can come up with a recipe for this, we should indeed remove the "just
>  > once" promise.
> 
> A silly example with emacs -Q is
> 
> (defvar fun 0)
> 
> (defun fun (frame)
>    "..."
>    (message "\n%s\n" (setq fun (1+ fun))) (ding))
> 
> (add-hook 'window-size-change-functions 'fun)
> 
> This gets me complete erratic behavior when moving point.

Maybe I'm blind, but I don't see anything erratic.

> Another example is
> 
> (defvar fun 0)
> 
> (defun fun (frame)
>    "..."
>    (message "%s" (setq fun (1+ fun))) (ding))
> 
> (add-hook 'window-size-change-functions 'fun)
> 
> (progn
>    (tool-bar-mode -1)
>    (sit-for 1)
>    (tool-bar-mode 1))
> 
> With my patch for bug#21975 this runs ‘fun’ twice when the tool-bar is
> reenabled.  Likely due to my calculations of the real tool bar height.

I see that, but does that mean we should remove the "once only"
promise?

bug#19576: write-file writes the wrong buffer

[Alan Mackenzie]

Hello, Eli and Martin.

On Mon, Nov 23, 2015 at 08:21:14PM +0200, Eli Zaretskii wrote:
> > Date: Sun, 22 Nov 2015 18:46:19 +0100
> > From: martin rudalics <rudalics@gmx.at>
> > CC: acm@muc.de, 19576@debbugs.gnu.org, andlind@gmail.com, 
> >  juri@linkov.net
> > 
> >  > I couldn't think of any situation that would cause that.  It would
> >  > require a resize of windows outside redisplay that would also cause
> >  > redisplay to decide that the mini-window needs to be resized.  If you
> >  > can come up with a recipe for this, we should indeed remove the "just
> >  > once" promise.
> > 
> > A silly example with emacs -Q is
> > 
> > (defvar fun 0)
> > 
> > (defun fun (frame)
> >    "..."
> >    (message "\n%s\n" (setq fun (1+ fun))) (ding))
> > 
> > (add-hook 'window-size-change-functions 'fun)
> > 
> > This gets me complete erratic behavior when moving point.

> Maybe I'm blind, but I don't see anything erratic.

> > Another example is
> > 
> > (defvar fun 0)
> > 
> > (defun fun (frame)
> >    "..."
> >    (message "%s" (setq fun (1+ fun))) (ding))
> > 
> > (add-hook 'window-size-change-functions 'fun)
> > 
> > (progn
> >    (tool-bar-mode -1)
> >    (sit-for 1)
> >    (tool-bar-mode 1))
> > 
> > With my patch for bug#21975 this runs ‘fun’ twice when the tool-bar is
> > reenabled.  Likely due to my calculations of the real tool bar height.

> I see that, but does that mean we should remove the "once only"
> promise?

Or, alternatively, perhaps reconsider my proposed patch from a few days
ago which has w-s-c-functions run at most once for any redisplay
operation.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 >> A silly example with emacs -Q is
 >>
 >> (defvar fun 0)
 >>
 >> (defun fun (frame)
 >>     "..."
 >>     (message "\n%s\n" (setq fun (1+ fun))) (ding))
 >>
 >> (add-hook 'window-size-change-functions 'fun)
 >>
 >> This gets me complete erratic behavior when moving point.
 >
 > Maybe I'm blind, but I don't see anything erratic.

OK.  Here the erratic behavior is caused by ‘eldoc-mode’.  In any case
‘window-size-change-functions’ is executed twice per redisplay IIUC.

 > I see that, but does that mean we should remove the "once only"
 > promise?

I'd just remove the qualifier "just".

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Tue, 24 Nov 2015 09:27:52 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, 19576@debbugs.gnu.org, andlind@gmail.com, 
>  juri@linkov.net
> 
>  > I see that, but does that mean we should remove the "once only"
>  > promise?
> 
> I'd just remove the qualifier "just".

Feel free.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

 > Please tell me if this bug and bug#21333 could now be closed, or if
 > there are any leftovers.

Here with emacs -Q evaluating

(message "...\n")

runs ‘window-size-change-functions’.  Evaluating

(read-file-name "...\n")

doesn't.  If this is intentional it should be documented.

martin

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Sat, 28 Nov 2015 11:26:18 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 19576@debbugs.gnu.org, andlind@gmail.com, juri@linkov.net
> 
>  > Please tell me if this bug and bug#21333 could now be closed, or if
>  > there are any leftovers.
> 
> Here with emacs -Q evaluating
> 
> (message "...\n")
> 
> runs ‘window-size-change-functions’.  Evaluating
> 
> (read-file-name "...\n")
> 
> doesn't.  If this is intentional it should be documented.

I don't know if it's intentional.  Could you please look into this and
publish what you find?  I really could use some help here.

Thanks.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

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

 >> Here with emacs -Q evaluating
 >>
 >> (message "...\n")
 >>
 >> runs ‘window-size-change-functions’.  Evaluating
 >>
 >> (read-file-name "...\n")
 >>
 >> doesn't.  If this is intentional it should be documented.
 >
 > I don't know if it's intentional.  Could you please look into this and
 > publish what you find?  I really could use some help here.

The attached patch runs ‘window-size-change-functions’ for any call of
‘read-from-minibuffer’ that changes the size of the minibuffer window
either via the prompt or user input.  I have no idea whether it handles
all possible cases nor whether it might baldy interact with the part
that traces messages resizing the echo area.

Also, saving match data around the ‘window-size-change-functions’ calls
and not saving current buffer and selected window constitutes a gross
inconsistency IMHO.  So I'd rather prefer to close my eyes to this part
of the code, if possible.

martin

[-- Attachment #2: xdisp.diff --]
[-- Type: text/plain, Size: 848 bytes --]

--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13581,6 +13581,27 @@ redisplay_internal (void)
 	   && (current_buffer->clip_changed || window_outdated (w))
 	   && resize_mini_window (w, false))
     {
+      if (sf->redisplay)
+	{
+	  Lisp_Object functions;
+	  ptrdiff_t count1 = SPECPDL_INDEX ();
+
+	  record_unwind_save_match_data ();
+
+	  /* Clear flag first in case we get an error below.  */
+	  FRAME_WINDOW_SIZES_CHANGED (sf) = false;
+	  functions = Vwindow_size_change_functions;
+
+	  while (CONSP (functions))
+	    {
+	      if (!EQ (XCAR (functions), Qt))
+		call1 (XCAR (functions), selected_frame);
+	      functions = XCDR (functions);
+	    }
+
+	  unbind_to (count1, Qnil);
+	}
+
       /* Resized active mini-window to fit the size of what it is
          showing if its contents might have changed.  */
       must_finish = true;

bug#19576: write-file writes the wrong buffer

[Eli Zaretskii]

> Date: Mon, 30 Nov 2015 14:30:31 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, 19576@debbugs.gnu.org, andlind@gmail.com, 
>  juri@linkov.net
> 
>  >> Here with emacs -Q evaluating
>  >>
>  >> (message "...\n")
>  >>
>  >> runs ‘window-size-change-functions’.  Evaluating
>  >>
>  >> (read-file-name "...\n")
>  >>
>  >> doesn't.  If this is intentional it should be documented.
>  >
>  > I don't know if it's intentional.  Could you please look into this and
>  > publish what you find?  I really could use some help here.
> 
> The attached patch runs ‘window-size-change-functions’ for any call of
> ‘read-from-minibuffer’ that changes the size of the minibuffer window
> either via the prompt or user input.  I have no idea whether it handles
> all possible cases nor whether it might baldy interact with the part
> that traces messages resizing the echo area.

Thanks, please push to the emacs-25 branch.

bug#19576: write-file writes the wrong buffer

[martin rudalics]

> Thanks, please push to the emacs-25 branch.

Done.

martin