unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#32777: 27.0.50; window-buffer gets wrong point
@ 2018-09-19 22:55 Juri Linkov
  2018-09-30  4:03 ` Federico Tedin
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2018-09-19 22:55 UTC (permalink / raw)
  To: 32777

This bug report concerns one particular usage of window-buffer in
read-extended-command, but the same issue might exist in other places:

0. emacs -Q

1. copy to *scratch* as text:

next-error
nonexistent-command

2.

2.1. put point on "next-error", type M-x M-n,
     it correctly inserts "next-error" to the minibuffer

2.2. put point on "nonexistent-command", type M-x M-n,
     it correctly displays "End of history; no default available"

3.

3.1. put point on "nonexistent-command"

3.2. split window C-x 2 or C-x 3 and switch to another window C-x o

3.3. put point on "next-error", type M-x M-n,
     it correctly inserts "next-error" to the minibuffer

4.

4.1. delete-other-windows with C-x 1

4.1. put point on "nonexistent-command"

4.2. create new frame with C-x 5 2

4.3. put point on "next-error", type M-x M-n,
     it displays "End of history; no default available",
     this is wrong





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-09-19 22:55 bug#32777: 27.0.50; window-buffer gets wrong point Juri Linkov
@ 2018-09-30  4:03 ` Federico Tedin
  2018-10-02  2:07   ` Federico Tedin
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Tedin @ 2018-09-30  4:03 UTC (permalink / raw)
  To: juri; +Cc: 32777

I am looking into this bug, and I've found a shorter way of reproducing it:

1) emacs -q

2) On *scratch*, enter:
next-error
nonexistent-command

3) Move the point to "nonexistent-command"

4) C-x 5 2

5) Move the point to "next-error"

6) M-x M-n shows "End of history" (but it should show "next-error")

It seems like the problem could be solved by modifying the code on
simple.el, line 1713 onward. I'll check and see if I can fix it.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-09-30  4:03 ` Federico Tedin
@ 2018-10-02  2:07   ` Federico Tedin
  2018-10-02  3:21     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Tedin @ 2018-10-02  2:07 UTC (permalink / raw)
  To: juri; +Cc: 32777

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

> It seems like the problem could be solved by modifying the code on
> simple.el, line 1713 onward. I'll check and see if I can fix it.

I've managed to fix the problem, I'm attaching a patch with my proposed changes.

[-- Attachment #2: minibuf.patch --]
[-- Type: text/x-patch, Size: 1409 bytes --]

From 6cb506a1f021888b5faf78090a4ab524fd8bb526 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Mon, 1 Oct 2018 23:03:36 -0300
Subject: [PATCH] Fix M-n command completion for read-extended-command

* lisp/simple.el (read-extended-command): Ensure the function uses the
  apropiate window when looking for a command to suggest when user
  enters M-n after initial prompt. (Bug#32777)
---
 lisp/simple.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index e41630d4ed..1d129c3c99 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1710,9 +1710,10 @@ read-extended-command
 	     (lambda ()
 	       ;; Get a command name at point in the original buffer
 	       ;; to propose it after M-n.
-	       (with-current-buffer (window-buffer (minibuffer-selected-window))
-		 (and (commandp (function-called-at-point))
-		      (format "%S" (function-called-at-point)))))))
+	       (with-selected-window (minibuffer-selected-window)
+                 (with-current-buffer (window-buffer (selected-window))
+		   (and (commandp (function-called-at-point))
+		        (format "%S" (function-called-at-point))))))))
     ;; Read a string, completing from and restricting to the set of
     ;; all defined commands.  Don't provide any initial input.
     ;; Save the command read on the extended-command history list.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-10-02  2:07   ` Federico Tedin
@ 2018-10-02  3:21     ` Eli Zaretskii
  2018-10-02 12:31       ` Federico Tedin
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-10-02  3:21 UTC (permalink / raw)
  To: Federico Tedin; +Cc: juri, 32777

> From: Federico Tedin <federicotedin@gmail.com>
> Date: Mon, 1 Oct 2018 23:07:41 -0300
> Cc: 32777@debbugs.gnu.org
> 
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -1710,9 +1710,10 @@ read-extended-command
>  	     (lambda ()
>  	       ;; Get a command name at point in the original buffer
>  	       ;; to propose it after M-n.
> -	       (with-current-buffer (window-buffer (minibuffer-selected-window))
> -		 (and (commandp (function-called-at-point))
> -		      (format "%S" (function-called-at-point)))))))
> +	       (with-selected-window (minibuffer-selected-window)
> +                 (with-current-buffer (window-buffer (selected-window))
> +		   (and (commandp (function-called-at-point))
> +		        (format "%S" (function-called-at-point))))))))
>      ;; Read a string, completing from and restricting to the set of
>      ;; all defined commands.  Don't provide any initial input.
>      ;; Save the command read on the extended-command history list.

Can you explain the change?  The minibuffer window is already the
selected window at this point (look at the implementation of
minibuffer-selected-window), so using with-selected-window, which
seems to be the only real change in the above, should be redundant.
Also, with-selected-window makes the window's buffer current, so why
did you need with-current-buffer in addition?  What am I missing?

Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-10-02  3:21     ` Eli Zaretskii
@ 2018-10-02 12:31       ` Federico Tedin
  2018-10-13  9:19         ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Federico Tedin @ 2018-10-02 12:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: juri, 32777

> Can you explain the change?  The minibuffer window is already the
> selected window at this point (look at the implementation of
> minibuffer-selected-window), so using with-selected-window, which
> seems to be the only real change in the above, should be redundant.

This was my reasoning:

Calling "minibuffer-selected-window" returns the last selected window
before switching to the minibuffer. Then, calling "window-buffer" with
that window will return that window's buffer.

The problem is that when "with-current-buffer" is called with the
resulting buffer, it that buffer has been opened on more than one
window, the active window will be set according to a criteria which I
haven't figured out yet, but not necessarily to the same exact window
"minibuffer-selected-window" returned.

The way I tested this was the following:
1) On a frame, open three windows. On the first two, open *scratch*.
On the third one, open
any other buffer.
2) Insert some content into buffer *scratch* ("hello").
3) Make sure the first window is selected, and move the point to (point-min).
4) M-x eval-expression (with-current-buffer "*scratch*" (message "%s"
(point))) should yield "1".
5) Select the second window, and move the point to (point-max).
6) M-x eval-expression (with-current-buffer "*scratch*" (message "%s"
(point))) should yield "7".
7) Now, select the third window.
8) M-x eval-expression (with-current-buffer "*scratch*" (message "%s" (point)))

The last point yields "1" in my case. If I wanted it to yield "7", I
would have to explicitly select the second window.  So from this, I
reasoned that using M-n when in read-extended-command, it will try to
read a command from the last selected window's buffer, but the value
of the point can vary if there's more than one window visiting that
buffer (like in the test case originally described by Juri). Please
correct me if I'm wrong.

> Also, with-selected-window makes the window's buffer current, so why
> did you need with-current-buffer in addition?  What am I missing?

This was an oversight on my part.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-10-02 12:31       ` Federico Tedin
@ 2018-10-13  9:19         ` Eli Zaretskii
  2018-10-13 13:08           ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-10-13  9:19 UTC (permalink / raw)
  To: Federico Tedin, martin rudalics; +Cc: juri, 32777

> From: Federico Tedin <federicotedin@gmail.com>
> Date: Tue, 2 Oct 2018 09:31:25 -0300
> Cc: juri@linkov.net, 32777@debbugs.gnu.org
> 
> > Can you explain the change?  The minibuffer window is already the
> > selected window at this point (look at the implementation of
> > minibuffer-selected-window), so using with-selected-window, which
> > seems to be the only real change in the above, should be redundant.
> 
> This was my reasoning:
> 
> Calling "minibuffer-selected-window" returns the last selected window
> before switching to the minibuffer. Then, calling "window-buffer" with
> that window will return that window's buffer.
> 
> The problem is that when "with-current-buffer" is called with the
> resulting buffer, it that buffer has been opened on more than one
> window, the active window will be set according to a criteria which I
> haven't figured out yet, but not necessarily to the same exact window
> "minibuffer-selected-window" returned.
> 
> The way I tested this was the following:
> 1) On a frame, open three windows. On the first two, open *scratch*.
> On the third one, open
> any other buffer.
> 2) Insert some content into buffer *scratch* ("hello").
> 3) Make sure the first window is selected, and move the point to (point-min).
> 4) M-x eval-expression (with-current-buffer "*scratch*" (message "%s"
> (point))) should yield "1".
> 5) Select the second window, and move the point to (point-max).
> 6) M-x eval-expression (with-current-buffer "*scratch*" (message "%s"
> (point))) should yield "7".
> 7) Now, select the third window.
> 8) M-x eval-expression (with-current-buffer "*scratch*" (message "%s" (point)))
> 
> The last point yields "1" in my case. If I wanted it to yield "7", I
> would have to explicitly select the second window.  So from this, I
> reasoned that using M-n when in read-extended-command, it will try to
> read a command from the last selected window's buffer, but the value
> of the point can vary if there's more than one window visiting that
> buffer (like in the test case originally described by Juri). Please
> correct me if I'm wrong.

Thanks.

Martin, any comments?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-10-13  9:19         ` Eli Zaretskii
@ 2018-10-13 13:08           ` martin rudalics
  2018-10-15  7:56             ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-10-13 13:08 UTC (permalink / raw)
  To: Eli Zaretskii, Federico Tedin; +Cc: juri, 32777

 > Martin, any comments?

+	       (with-selected-window (minibuffer-selected-window)
+                 (with-current-buffer (window-buffer (selected-window))
+		   (and (commandp (function-called-at-point))
+		        (format "%S" (function-called-at-point))))))))

The

+                 (with-current-buffer (window-buffer (selected-window))

should be superfluous by all our rules but we know too well how valid
these rules sometimes are.  Any other patch (for example, using an
idiom like (window-point (minibuffer-selected-window))) would be likely
longer than Federico's solution.

It would be interesting to find out why the patch works.  Federico's
earlier remark

   The problem is that when "with-current-buffer" is called with the
   resulting buffer, it that buffer has been opened on more than one
   window, the active window will be set according to a criteria which I
   haven't figured out yet, but not necessarily to the same exact window
   "minibuffer-selected-window" returned.

can't be the cause IMO because 'with-current-buffer' does not set the
"active window" and the value of point of the buffer made current
remains the previous value of point in that buffer and that should be
the value assigned by this step

4.3. put point on "next-error", type M-x M-n,

of Juri's scenario.  So far I'm clueless.

martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-10-13 13:08           ` martin rudalics
@ 2018-10-15  7:56             ` martin rudalics
  2018-12-21  0:18               ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-10-15  7:56 UTC (permalink / raw)
  To: Eli Zaretskii, Federico Tedin; +Cc: juri, 32777

 > So far I'm clueless.

Here are the clues: FWIW the problem we see here is a redisplay
problem exemplified by the backtrace below.  Recall that Juri did a
M-x M-n with *scratch* selected in a window W1 on a frame F1 and
*scratch* also shown with a different value of window point in a
window W2 on another frame say F2.

Now the M-x makes the minibuffer window on frame F1 the selected
window.  When Juri now types M-n the backtrace below kicks in.  In
particular, the x_consider_frame_title call in frame #10 does

       record_unwind_protect (unwind_format_mode_line,
			     format_mode_line_unwind_data
			       (f, current_buffer, selected_window, false));

and then continues to select window W2 on F2 from frame #9 to frame #3
and, since the window point of W2 differs from that of W1, sets the
point of *scratch* to the value of W2's point from frame #2 to frame
#0.  Everything normal, it seems.

But now the unwind form above kicks and, since the selected window of
F1 is not the window on *scratch* but the minibuffer window, it will
reselect the minibuffer window on F1.  This will, however, fail to
restore the position of point in *scratch* from the window point of W1
with the ensuing problem described by Juri.

I hope this explains now why Federico's patch fixes Juri's scenario:
Selecting minibuf_selected_window sets the point of *scratch* to the
window point of W1 with the desired effect.

I can see two possible fixes for the underlying problem:

(1) Handle the minibuf_selected_window case specially.  This means
that, if on the target frame of the unwind form this variable is
non-nil, we restore the value of point of the buffer shown in
minibuf_selected_window to that window's point.

(2) Ignore minibuf_selected_window and handle point of the buffer
shown in the selected window of the frame whose title we consider for
drawing its title.  That is, record the buffer shown in
f->selected_window together with its position of point before
selecting f->selected_window in x_consider_frame_title and restore the
buffer's position of point in the unwind form.

I think (1) is more conservative because it strictly handles just
scenarios like Juri's.  But (1) does not handle the case where the
buffer of W2 is not shown anywhere else when x_consider_frame_title is
called.  So IMHO (2) should be the correct approach but I have no idea
what further consequences it could have.

martin

#0  temp_set_point_both (buffer=0x161ce10 <dumped_data+97936>, charpos=4141, bytepos=4141) at ../../src/intervals.c:1737
#1  0x01210b80 in set_point_both (charpos=4141, bytepos=4141) at ../../src/intervals.c:1895
#2  0x012106a1 in set_point_from_marker (marker=XIL(0x5e09b15)) at ../../src/intervals.c:1774
#3  0x0108a363 in select_window_1 (window=XIL(0x5dfcf25), inhibit_point_swap=false) at ../../src/window.c:565
#4  0x0108a17b in select_window (window=XIL(0x5dfcf25), norecord=XIL(0x8720), inhibit_point_swap=false) at ../../src/window.c:523
#5  0x0108a38a in Fselect_window (window=XIL(0x5dfcf25), norecord=XIL(0x8720)) at ../../src/window.c:593
#6  0x01013d60 in do_switch_frame (frame=XIL(0x5dfcda5), track=1, for_deletion=0, norecord=XIL(0x8720)) at ../../src/frame.c:1418
#7  0x01013dba in Fselect_frame (frame=XIL(0x5dfcda5), norecord=XIL(0x8720)) at ../../src/frame.c:1451
#8  0x0108a11c in select_window (window=XIL(0x5dfcf25), norecord=XIL(0x8720), inhibit_point_swap=false) at ../../src/window.c:515
#9  0x0108a38a in Fselect_window (window=XIL(0x5dfcf25), norecord=XIL(0x8720)) at ../../src/window.c:593
#10 0x0104651b in x_consider_frame_title (frame=XIL(0x5dfcda5)) at ../../src/xdisp.c:12032
#11 0x01046a62 in prepare_menu_bars () at ../../src/xdisp.c:12134
#12 0x0104afd2 in redisplay_internal () at ../../src/xdisp.c:14042
#13 0x01049d46 in redisplay () at ../../src/xdisp.c:13620
#14 0x0110ae22 in read_char (commandflag=1, map=XIL(0x1776a33), prev_event=XIL(0), used_mouse_menu=0x82e323, end_time=0x0) at ../../src/keyboard.c:2451
#15 0x0111969a in read_key_sequence (keybuf=0x82e464, prompt=XIL(0), dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=false) at ../../src/keyboard.c:9117
#16 0x0110844d in command_loop_1 () at ../../src/keyboard.c:1338
#17 0x011a39d2 in internal_condition_case (bfun=0x1108000 <command_loop_1>, handlers=XIL(0x35c0), hfun=0x11076a9 <cmd_error>) at ../../src/eval.c:1373
#18 0x01107c29 in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1079
#19 0x011a2fba in internal_catch (tag=XIL(0x37a0), func=0x1107bfe <command_loop_2>, arg=XIL(0)) at ../../src/eval.c:1136
#20 0x01107b4b in command_loop () at ../../src/keyboard.c:1050
#21 0x01107221 in recursive_edit_1 () at ../../src/keyboard.c:703
#22 0x0114637b in read_minibuf (map=XIL(0x17862bb), initial=XIL(0), prompt=XIL(0x1a82b3c), expflag=false, histvar=XIL(0x2723e0), histpos=make_number(0), defalt=XIL(0), allow_props=false, inherit_input_method=false) at ../../src/minibuf.c:669
#23 0x01146df2 in Fread_from_minibuffer (prompt=XIL(0x1a82b3c), initial_contents=XIL(0), keymap=XIL(0x17862bb), sys_read=XIL(0), hist=XIL(0x2723e0), default_value=XIL(0), inherit_input_method=XIL(0)) at ../../src/minibuf.c:940
#24 0x011a7982 in funcall_subr (subr=0x12c1dc0 <Sread_from_minibuffer>, numargs=7, args=0x82e810) at ../../src/eval.c:2954
#25 0x011a73bb in Ffuncall (nargs=8, args=0x82e80c) at ../../src/eval.c:2859
#26 0x011efd21 in exec_byte_code (bytestr=XIL(0x1322e14), vector=XIL(0x1322e25), maxdepth=make_number(18), args_template=make_number(2050), nargs=8, args=0x82eb20) at ../../src/bytecode.c:632
#27 0x011a7ed7 in funcall_lambda (fun=XIL(0x1322dfd), nargs=8, arg_vector=0x82eb00) at ../../src/eval.c:3060
#28 0x011a73fd in Ffuncall (nargs=9, args=0x82eafc) at ../../src/eval.c:2861
#29 0x011486b1 in Fcompleting_read (prompt=XIL(0x1a82b3c), collection=XIL(0x1a0385b), predicate=XIL(0x2800), require_match=XIL(0x8720), initial_input=XIL(0), hist=XIL(0x2723e0), def=XIL(0), inherit_input_method=XIL(0)) at ../../src/minibuf.c:1644
#30 0x011a605d in eval_sub (form=XIL(0x1a03833)) at ../../src/eval.c:2354
#31 0x011a09dd in Fprogn (body=XIL(0)) at ../../src/eval.c:481
#32 0x011a5b12 in eval_sub (form=XIL(0x1a037db)) at ../../src/eval.c:2276
#33 0x011a328e in Funwind_protect (args=XIL(0x1a037ab)) at ../../src/eval.c:1230
#34 0x011a5b12 in eval_sub (form=XIL(0x1a037b3)) at ../../src/eval.c:2276
#35 0x011a09dd in Fprogn (body=XIL(0)) at ../../src/eval.c:481
#36 0x011a2b5f in Flet (args=XIL(0x1a0377b)) at ../../src/eval.c:1009
#37 0x011a5b12 in eval_sub (form=XIL(0x1a0376b)) at ../../src/eval.c:2276
#38 0x011a09dd in Fprogn (body=XIL(0)) at ../../src/eval.c:481
#39 0x011a8287 in funcall_lambda (fun=XIL(0x1a0375b), nargs=0, arg_vector=0x0) at ../../src/eval.c:3131
#40 0x011a7503 in Ffuncall (nargs=1, args=0x82f048) at ../../src/eval.c:2873
#41 0x011efd21 in exec_byte_code (bytestr=XIL(0x13270dc), vector=XIL(0x13270f5), maxdepth=make_number(3), args_template=XIL(0), nargs=0, args=0x0) at ../../src/bytecode.c:632
#42 0x011ef157 in Fbyte_code (bytestr=XIL(0x13270dc), vector=XIL(0x13270f5), maxdepth=make_number(3)) at ../../src/bytecode.c:322
#43 0x011a5ed3 in eval_sub (form=XIL(0x13270cb)) at ../../src/eval.c:2330
#44 0x011a55ca in Feval (form=XIL(0x13270cb), lexical=XIL(0)) at ../../src/eval.c:2144
#45 0x0119db0d in Fcall_interactively (function=XIL(0x4bea0), record_flag=XIL(0), keys=XIL(0x161a30d)) at ../../src/callint.c:321
#46 0x011a7842 in funcall_subr (subr=0x151f0c0 <Scall_interactively>, numargs=3, args=0x82f550) at ../../src/eval.c:2939
#47 0x011a73bb in Ffuncall (nargs=4, args=0x82f54c) at ../../src/eval.c:2859
#48 0x011efd21 in exec_byte_code (bytestr=XIL(0x1327124), vector=XIL(0x1327135), maxdepth=make_number(13), args_template=make_number(1025), nargs=1, args=0x82f870) at ../../src/bytecode.c:632
#49 0x011a7ed7 in funcall_lambda (fun=XIL(0x132710d), nargs=1, arg_vector=0x82f86c) at ../../src/eval.c:3060
#50 0x011a73fd in Ffuncall (nargs=2, args=0x82f868) at ../../src/eval.c:2861
#51 0x011a6dfd in call1 (fn=XIL(0x27e0), arg1=XIL(0x4bea0)) at ../../src/eval.c:2710
#52 0x011087c2 in command_loop_1 () at ../../src/keyboard.c:1451
#53 0x011a39d2 in internal_condition_case (bfun=0x1108000 <command_loop_1>, handlers=XIL(0x35c0), hfun=0x11076a9 <cmd_error>) at ../../src/eval.c:1373
#54 0x01107c29 in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1079
#55 0x011a2fba in internal_catch (tag=XIL(0x8cc0), func=0x1107bfe <command_loop_2>, arg=XIL(0)) at ../../src/eval.c:1136
#56 0x01107bc3 in command_loop () at ../../src/keyboard.c:1058
#57 0x01107221 in recursive_edit_1 () at ../../src/keyboard.c:703
#58 0x011073f7 in Frecursive_edit () at ../../src/keyboard.c:774
#59 0x01105384 in main (argc=2, argv=0xa33f88) at ../../src/emacs.c:1731

Lisp Backtrace:
"redisplay_internal (C function)" (0x0)
"read-from-minibuffer" (0x82e810)
"completing-read-default" (0x82eb00)
"completing-read" (0x82eb68)
"progn" (0x82ec78)
"unwind-protect" (0x82ed68)
"let" (0x82eed8)
"read-extended-command" (0x82f04c)
"byte-code" (0x82f268)
"call-interactively" (0x82f550)
"command-execute" (0x82f86c)





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-10-15  7:56             ` martin rudalics
@ 2018-12-21  0:18               ` Juri Linkov
  2018-12-21  9:15                 ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2018-12-21  0:18 UTC (permalink / raw)
  To: martin rudalics; +Cc: Federico Tedin, 32777

> I can see two possible fixes for the underlying problem:
>
> (1) Handle the minibuf_selected_window case specially.  This means
> that, if on the target frame of the unwind form this variable is
> non-nil, we restore the value of point of the buffer shown in
> minibuf_selected_window to that window's point.
>
> (2) Ignore minibuf_selected_window and handle point of the buffer
> shown in the selected window of the frame whose title we consider for
> drawing its title.  That is, record the buffer shown in
> f->selected_window together with its position of point before
> selecting f->selected_window in x_consider_frame_title and restore the
> buffer's position of point in the unwind form.
>
> I think (1) is more conservative because it strictly handles just
> scenarios like Juri's.  But (1) does not handle the case where the
> buffer of W2 is not shown anywhere else when x_consider_frame_title is
> called.  So IMHO (2) should be the correct approach but I have no idea
> what further consequences it could have.

If the underlying problem can't be fixed, then maybe at least fix
this particular case like Federico proposed?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-12-21  0:18               ` Juri Linkov
@ 2018-12-21  9:15                 ` martin rudalics
  2018-12-21 16:02                   ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-12-21  9:15 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Federico Tedin, 32777

 > If the underlying problem can't be fixed, then maybe at least fix
 > this particular case like Federico proposed?

I would like to hear Eli's opinion first.  Eli can you please have a
look at

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32777#26

and tell us whether my analysis there is correct?  If so, do you have
any ideas how to proceed?

Thanks, martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-12-21  9:15                 ` martin rudalics
@ 2018-12-21 16:02                   ` Eli Zaretskii
  2018-12-23  9:40                     ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-12-21 16:02 UTC (permalink / raw)
  To: martin rudalics; +Cc: juri, federicotedin, 32777

> Date: Fri, 21 Dec 2018 10:15:28 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: Eli Zaretskii <eliz@gnu.org>, 
>  Federico Tedin <federicotedin@gmail.com>,
>  32777@debbugs.gnu.org
> 
> I would like to hear Eli's opinion first.  Eli can you please have a
> look at
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32777#26
> 
> and tell us whether my analysis there is correct?  If so, do you have
> any ideas how to proceed?

I think your analysis is correct.

As for how to proceed: I would love for us to have a way of selecting
a window temporarily, in way that doesn't involve saving/restoring its
window-point.  But maybe this is a pipe dream, so I think we should
try your alternative #2 on master and see what it breaks.

Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-12-21 16:02                   ` Eli Zaretskii
@ 2018-12-23  9:40                     ` martin rudalics
  2018-12-29  9:59                       ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-12-23  9:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: juri, federicotedin, 32777

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

 > As for how to proceed: I would love for us to have a way of selecting
 > a window temporarily, in way that doesn't involve saving/restoring its
 > window-point.

It depends on what your semantics of "selecting a window temporarily"
are.  The only invariance frame/window management requires is that
(frame-selected-window (selected-frame)) equals (selected-window).
Otherwise you can freely set things like selected_window and
selected_frame directly.

Things get problematic when you want to, for example, show the line
number of a buffer in the mode line since in that case you need the
corresponding window's point which is stored in the window structure
only if the window is not the selected one.  So the most important
prerequisite for what you want is that redisplay itself always has a
very strong opinion of which window is the selected one and which
buffer is current.

One of the most unexpected things redisplay currently does is to run
Fselect_frame from unwind_format_mode_line which if I'm not mistaken
could wind up calling do_switch_frame with TRACK non-zero and thus
cause a Fredirect_frame_focus from within redisplay.  I think that
redisplay should not call 'select-frame' or 'select-window' for such
reasons.

 > But maybe this is a pipe dream, so I think we should
 > try your alternative #2 on master and see what it breaks.

To elaborate further on what I stated there as

 >> But (1) does not handle the case where the
 >> buffer of W2 is not shown anywhere else when x_consider_frame_title is
 >> called.  So IMHO (2) should be the correct approach but I have no idea
 >> what further consequences it could have.

consider the following scenario with emacs -Q:

C-x 5 2

C-x 5 o

C-x b RET

M-: (with-current-buffer "*scratch*" (goto-char (point-min)))

M-: (with-current-buffer "*scratch*" (point))

The value of evaluating the second form depends on the position of
point of the window displaying *scratch* and not on where the
'goto-char' went before.  So this is a quite nasty bug IMHO.

Unfortunately, curing it is non-trivial to avoid that some window
point gets moved unexpectedly, see the attached patch.  People using
multiple frames pretty please try to run it and report any bad
experiences here ASAP.

Thanks, martin

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

diff --git a/src/xdisp.c b/src/xdisp.c
index 94742c2..315dce2 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -11860,7 +11860,7 @@ static void ATTRIBUTE_FORMAT_PRINTF (1, 0)
   Vmode_line_unwind_vector = Qnil;
 
   if (NILP (vector))
-    vector = make_nil_vector (10);
+    vector = make_nil_vector (12);
 
   ASET (vector, 0, make_fixnum (mode_line_target));
   ASET (vector, 1, make_fixnum (MODE_LINE_NOPROP_LEN (0)));
@@ -11877,12 +11877,24 @@ static void ATTRIBUTE_FORMAT_PRINTF (1, 0)
   ASET (vector, 7, owin);
   if (target_frame)
     {
+      Lisp_Object buffer = XWINDOW (target_frame->selected_window)->contents;
+      struct buffer *b = XBUFFER (buffer);
+      struct buffer *cb = current_buffer;
+
       /* Similarly to `with-selected-window', if the operation selects
 	 a window on another frame, we must restore that frame's
 	 selected window, and (for a tty) the top-frame.  */
       ASET (vector, 8, target_frame->selected_window);
       if (FRAME_TERMCAP_P (target_frame))
 	ASET (vector, 9, FRAME_TTY (target_frame)->top_frame);
+
+      /* If we select a window on another frame, make sure that that
+	 selection does not leave its buffer's point modified when
+	 unwinding (Bug#32777).  */
+      ASET (vector, 10, buffer);
+      current_buffer = b;
+      ASET (vector, 11, build_marker (current_buffer, PT, PT_BYTE));
+      current_buffer = cb;
     }
 
   return vector;
@@ -11913,12 +11925,26 @@ static void ATTRIBUTE_FORMAT_PRINTF (1, 0)
 	{
 	  Lisp_Object frame
 	    = WINDOW_FRAME (XWINDOW (target_frame_window));
+	  Lisp_Object buffer = AREF (vector, 10);
 
 	  if (!EQ (frame, WINDOW_FRAME (XWINDOW (old_window))))
 	    Fselect_window (target_frame_window, Qt);
 
 	  if (!NILP (old_top_frame) && !EQ (old_top_frame, frame))
 	    Fselect_frame (old_top_frame, Qt);
+
+	  if (BUFFER_LIVE_P (XBUFFER (buffer))
+	      && !EQ (XWINDOW (old_window)->contents, buffer))
+	    {
+	      /* Restore point of target_frame_window's buffer
+		 (Bug#32777).  */
+	      struct buffer *cb = current_buffer;
+
+	      current_buffer = XBUFFER (buffer);
+	      set_point_from_marker (AREF (vector, 11));
+	      ASET (vector, 11, Qnil);
+	      current_buffer = cb;
+	    }
 	}
 
       Fselect_window (old_window, Qt);


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-12-23  9:40                     ` martin rudalics
@ 2018-12-29  9:59                       ` martin rudalics
  2018-12-29 23:10                         ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-12-29  9:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: juri, federicotedin, 32777

 > Unfortunately, curing it is non-trivial to avoid that some window
 > point gets moved unexpectedly, see the attached patch.

I now installed on master a slightly modified version.  Kindly check
whether all bugs mentioned in this thread have been fixed and report
any anomalies immediately.

Thanks, martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-12-29  9:59                       ` martin rudalics
@ 2018-12-29 23:10                         ` Juri Linkov
  2018-12-30  9:49                           ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2018-12-29 23:10 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32777-done

> I now installed on master a slightly modified version.  Kindly check
> whether all bugs mentioned in this thread have been fixed and report
> any anomalies immediately.

Thanks.  I confirm that all reported anomalies are fixed now.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#32777: 27.0.50; window-buffer gets wrong point
  2018-12-29 23:10                         ` Juri Linkov
@ 2018-12-30  9:49                           ` martin rudalics
  0 siblings, 0 replies; 15+ messages in thread
From: martin rudalics @ 2018-12-30  9:49 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32777-done

 > Thanks.  I confirm that all reported anomalies are fixed now.

Thanks for checking, martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-12-30  9:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 22:55 bug#32777: 27.0.50; window-buffer gets wrong point Juri Linkov
2018-09-30  4:03 ` Federico Tedin
2018-10-02  2:07   ` Federico Tedin
2018-10-02  3:21     ` Eli Zaretskii
2018-10-02 12:31       ` Federico Tedin
2018-10-13  9:19         ` Eli Zaretskii
2018-10-13 13:08           ` martin rudalics
2018-10-15  7:56             ` martin rudalics
2018-12-21  0:18               ` Juri Linkov
2018-12-21  9:15                 ` martin rudalics
2018-12-21 16:02                   ` Eli Zaretskii
2018-12-23  9:40                     ` martin rudalics
2018-12-29  9:59                       ` martin rudalics
2018-12-29 23:10                         ` Juri Linkov
2018-12-30  9:49                           ` martin rudalics

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).