bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Antoine Levitt]

Tested on git master, the bug is not present on 25.

Recipe from emacs -Q:

(setq dired-auto-revert-buffer t)

open dired, open a folder, C-x b back to the previous dired buffer, open
the same folder again, see point jump back to the beginning of buffer.

I'm not sure why, since dired-auto-revert-buffer just seems to call
revert-buffer, which does not jump point.

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

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

On Sun, 04 Jun 2017 16:45:05 -0700 Antoine Levitt <antoine.levitt@gmail.com> wrote:

> Tested on git master, the bug is not present on 25.
>
> Recipe from emacs -Q:
>
> (setq dired-auto-revert-buffer t)
>
> open dired, open a folder, C-x b back to the previous dired buffer, open
> the same folder again, see point jump back to the beginning of buffer.
>
> I'm not sure why, since dired-auto-revert-buffer just seems to call
> revert-buffer, which does not jump point.

I think I see why this happens.  I assume when you opened "the same
folder again" you pressed RET on the line in the Dired listing.  That
calls dired-find-file, which calls find-file, which first calls
find-file-noselect (to see if it's a live buffer), which calls (via
run-hook-with-args-until-success) dired-noselect, which calls (via
dired-internal-noselect) revert-buffer (since dired-auto-revert-buffer
is t), which calls dired-revert, which saves point (via
dired-save-positions) but then erases the buffer and inserts it anew --
and this is the problem, because now continuing in find-file, it calls
switch-to-buffer (since it's a live buffer that you're revisiting),
which sets window-point by consulting window-prev-buffers.  Prior to the
invocation of erase-buffer, window-prev-buffers contained this entry for
the buffer being reverted ("test" in my test case):

(#<buffer test> #<marker at 1 in test> #<marker at 232 in test>)

which shows window-point on the first file name in the Dired listing,
but after erase-buffer it's now this:

(#<buffer test> #<marker at 1 in test> #<marker at 1 in test>)

Since dired-revert restored the saved point (232) but does not return it
to the caller, switch-to-buffer does not have this information, but only
what window-prev-buffers shows, which is now 1.  So that's what it sets
window-point to.

One way to fix this is to make dired-restore-positions restore not only
point but also window-point.  The attached patch does this.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dired-restore-positions patch --]
[-- Type: text/x-patch, Size: 1247 bytes --]

diff --git a/lisp/dired.el b/lisp/dired.el
index 8396652d50..26d3d76817 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1382,9 +1382,13 @@ dired-restore-positions
   (let* ((buf-file-pos (nth 0 positions))
 	 (buffer (nth 0 buf-file-pos)))
     (unless (and (nth 1 buf-file-pos)
-		 (dired-goto-file (nth 1 buf-file-pos)))
+		 (prog1 (dired-goto-file (nth 1 buf-file-pos))
+		   (set-window-buffer nil buffer)
+		   (set-window-point (selected-window) (nth 2 buf-file-pos))))
       (goto-char (nth 2 buf-file-pos))
-      (dired-move-to-filename))
+      (dired-move-to-filename)
+      (set-window-buffer nil buffer)
+      (set-window-point (selected-window) (nth 2 buf-file-pos)))
     (dolist (win-file-pos (nth 1 positions))
       ;; Ensure that window still displays the original buffer.
       (when (eq (window-buffer (nth 0 win-file-pos)) buffer)
@@ -1392,7 +1396,8 @@ dired-restore-positions
 	  (unless (and (nth 1 win-file-pos)
 		       (dired-goto-file (nth 1 win-file-pos)))
 	    (goto-char (nth 2 win-file-pos))
-	    (dired-move-to-filename)))))))
+	    (dired-move-to-filename)
+	    (set-window-point nil (point))))))))
 
 (defun dired-remember-marks (beg end)
   "Return alist of files and their marks, from BEG to END."

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Mon, 05 Jun 2017 15:37:47 +0200
> Cc: 27243@debbugs.gnu.org
> 
> I think I see why this happens.  I assume when you opened "the same
> folder again" you pressed RET on the line in the Dired listing.  That
> calls dired-find-file, which calls find-file, which first calls
> find-file-noselect (to see if it's a live buffer), which calls (via
> run-hook-with-args-until-success) dired-noselect, which calls (via
> dired-internal-noselect) revert-buffer (since dired-auto-revert-buffer
> is t), which calls dired-revert, which saves point (via
> dired-save-positions) but then erases the buffer and inserts it anew --
> and this is the problem, because now continuing in find-file, it calls
> switch-to-buffer (since it's a live buffer that you're revisiting),
> which sets window-point by consulting window-prev-buffers.  Prior to the
> invocation of erase-buffer, window-prev-buffers contained this entry for
> the buffer being reverted ("test" in my test case):
> 
> (#<buffer test> #<marker at 1 in test> #<marker at 232 in test>)
> 
> which shows window-point on the first file name in the Dired listing,
> but after erase-buffer it's now this:
> 
> (#<buffer test> #<marker at 1 in test> #<marker at 1 in test>)
> 
> Since dired-revert restored the saved point (232) but does not return it
> to the caller, switch-to-buffer does not have this information, but only
> what window-prev-buffers shows, which is now 1.  So that's what it sets
> window-point to.

Right, I see the same.

> One way to fix this is to make dired-restore-positions restore not only
> point but also window-point.  The attached patch does this.

An alternative is to bind switch-to-buffer-preserve-window-point to
nil (the whole problem was caused by the fact that this variable is
now non-nil by default, and dired-revert runs afoul of it, as you
point out):

--- lisp/dired.el~0	2017-02-28 06:27:41.000000000 +0200
+++ lisp/dired.el	2017-06-05 17:51:50.633645200 +0300
@@ -2126,7 +2126,11 @@
   (interactive)
   ;; Bind `find-file-run-dired' so that the command works on directories
   ;; too, independent of the user's setting.
-  (let ((find-file-run-dired t))
+  (let ((find-file-run-dired t)
+        (switch-to-buffer-preserve-window-point
+         (if dired-auto-revert-buffer
+             nil
+           switch-to-buffer-preserve-window-point)))
     (find-file (dired-get-file-for-visit))))
 
 (defun dired-find-alternate-file ()


I'm not sure which solution is better.  Maybe someone will come up
with a more elegant one.

Thanks.

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

On Mon, 05 Jun 2017 18:02:14 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> One way to fix this is to make dired-restore-positions restore not only
>> point but also window-point.  The attached patch does this.
>
> An alternative is to bind switch-to-buffer-preserve-window-point to
> nil (the whole problem was caused by the fact that this variable is
> now non-nil by default, and dired-revert runs afoul of it, as you
> point out):

Oh, I didn't even know about switch-to-buffer-preserve-window-point.

> I'm not sure which solution is better.  Maybe someone will come up
> with a more elegant one.

Your fix is certainly simpler and I think cleaner than mine, so I would
say install it.

Steve Berman

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: antoine.levitt@gmail.com,  27243@debbugs.gnu.org
> Date: Mon, 05 Jun 2017 17:15:54 +0200
> 
> > An alternative is to bind switch-to-buffer-preserve-window-point to
> > nil (the whole problem was caused by the fact that this variable is
> > now non-nil by default, and dired-revert runs afoul of it, as you
> > point out):
> 
> Oh, I didn't even know about switch-to-buffer-preserve-window-point.
> 
> > I'm not sure which solution is better.  Maybe someone will come up
> > with a more elegant one.
> 
> Your fix is certainly simpler and I think cleaner than mine, so I would
> say install it.

OK, thanks.  I will wait for a couple of days to let others a chance
to chime in, and will install if no further comments come up.

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> Date: Mon, 05 Jun 2017 19:20:41 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 27243@debbugs.gnu.org, antoine.levitt@gmail.com
> 
> > Your fix is certainly simpler and I think cleaner than mine, so I would
> > say install it.
> 
> OK, thanks.  I will wait for a couple of days to let others a chance
> to chime in, and will install if no further comments come up.

No further comments, so I pushed the change, and I'm marking this bug
done.

bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer

[Antoine Levitt]

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

Thanks for the quick fix!

Best,
Antoine

On 10 Jun 2017 10:25 am, "Eli Zaretskii" <eliz@gnu.org> wrote:

> Date: Mon, 05 Jun 2017 19:20:41 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 27243@debbugs.gnu.org, antoine.levitt@gmail.com
>
> > Your fix is certainly simpler and I think cleaner than mine, so I would
> > say install it.
>
> OK, thanks.  I will wait for a couple of days to let others a chance
> to chime in, and will install if no further comments come up.

No further comments, so I pushed the change, and I'm marking this bug
done.

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

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Antoine Levitt]

I just noticed this is not yet completely fixed: starting from emacs -Q,
(setq dired-auto-revert-buffer t), open dired, open any file in that
directory, C-x d RET to run dired again, the point jumps back to the
beginning of the buffer.

Best,
Antoine

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Stephen Berman]

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

On Sat, 17 Jun 2017 14:16:01 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:

> I just noticed this is not yet completely fixed: starting from emacs -Q,
> (setq dired-auto-revert-buffer t), open dired, open any file in that
> directory, C-x d RET to run dired again, the point jumps back to the
> beginning of the buffer.

I suppose the command `dired' should not use switch-to-buffer.  With the
following patch, executing the above recipe does not move point.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dired patch --]
[-- Type: text/x-patch, Size: 583 bytes --]

diff --git a/lisp/dired.el b/lisp/dired.el
index 8396652d50..aa59f01af9 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -786,7 +786,8 @@ dired
 If DIRNAME is already in a Dired buffer, that buffer is used without refresh."
   ;; Cannot use (interactive "D") because of wildcards.
   (interactive (dired-read-dir-and-switches ""))
-  (switch-to-buffer (dired-noselect dirname switches)))
+  (set-window-buffer (selected-window)
+                     (set-buffer (dired-noselect dirname switches))))
 
 ;;;###autoload (define-key ctl-x-4-map "d" 'dired-other-window)
 ;;;###autoload

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Antoine Levitt]

Works for me, feel free to merge!

Best,
Antoine

17 June 2017 15:32 +02, Stephen Berman <stephen.berman@gmx.net>:
> On Sat, 17 Jun 2017 14:16:01 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:
>
>> I just noticed this is not yet completely fixed: starting from emacs -Q,
>> (setq dired-auto-revert-buffer t), open dired, open any file in that
>> directory, C-x d RET to run dired again, the point jumps back to the
>> beginning of the buffer.
>
> I suppose the command `dired' should not use switch-to-buffer.  With the
> following patch, executing the above recipe does not move point.
>
> diff --git a/lisp/dired.el b/lisp/dired.el
> index 8396652d50..aa59f01af9 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -786,7 +786,8 @@ dired
>  If DIRNAME is already in a Dired buffer, that buffer is used without refresh."
>    ;; Cannot use (interactive "D") because of wildcards.
>    (interactive (dired-read-dir-and-switches ""))
> -  (switch-to-buffer (dired-noselect dirname switches)))
> +  (set-window-buffer (selected-window)
> +                     (set-buffer (dired-noselect dirname switches))))
>  
>  ;;;###autoload (define-key ctl-x-4-map "d" 'dired-other-window)
>  ;;;###autoload
>
> Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Stephen Berman]

On Sun, 09 Jul 2017 19:37:53 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:

> Works for me, feel free to merge!
>
> Best,
> Antoine

Thanks for confirming.  I think this is the right fix and would commit
it, but since this exchange has taken place in a closed bug, it may have
fallen under the radar, so I'd like an explicit go-ahead.  Eli, John?

Steve Berman

> 17 June 2017 15:32 +02, Stephen Berman <stephen.berman@gmx.net>:
>> On Sat, 17 Jun 2017 14:16:01 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:
>>
>>> I just noticed this is not yet completely fixed: starting from emacs -Q,
>>> (setq dired-auto-revert-buffer t), open dired, open any file in that
>>> directory, C-x d RET to run dired again, the point jumps back to the
>>> beginning of the buffer.
>>
>> I suppose the command `dired' should not use switch-to-buffer.  With the
>> following patch, executing the above recipe does not move point.
>>
>> diff --git a/lisp/dired.el b/lisp/dired.el
>> index 8396652d50..aa59f01af9 100644
>> --- a/lisp/dired.el
>> +++ b/lisp/dired.el
>> @@ -786,7 +786,8 @@ dired
>>  If DIRNAME is already in a Dired buffer, that buffer is used without refresh."
>>    ;; Cannot use (interactive "D") because of wildcards.
>>    (interactive (dired-read-dir-and-switches ""))
>> -  (switch-to-buffer (dired-noselect dirname switches)))
>> +  (set-window-buffer (selected-window)
>> +                     (set-buffer (dired-noselect dirname switches))))
>>  
>>  ;;;###autoload (define-key ctl-x-4-map "d" 'dired-other-window)
>>  ;;;###autoload
>>
>> Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[John Wiegley]

>>>>> Stephen Berman <stephen.berman@gmx.net> writes:

> Thanks for confirming. I think this is the right fix and would commit it,
> but since this exchange has taken place in a closed bug, it may have fallen
> under the radar, so I'd like an explicit go-ahead. Eli, John?

At first glance it looks OK to me, though I defer to Eli's experience in such
matters.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Cc: Antoine Levitt <antoine.levitt@gmail.com>,  27243@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 14 Jul 2017 22:52:18 -0700
> 
> >>>>> Stephen Berman <stephen.berman@gmx.net> writes:
> 
> > Thanks for confirming. I think this is the right fix and would commit it,
> > but since this exchange has taken place in a closed bug, it may have fallen
> > under the radar, so I'd like an explicit go-ahead. Eli, John?
> 
> At first glance it looks OK to me, though I defer to Eli's experience in such
> matters.

No objections here.

Thanks.

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[martin rudalics]

 > +  (set-window-buffer (selected-window)
 > +                     (set-buffer (dired-noselect dirname switches))))

This really should be

      (pop-to-buffer-same-window (dired-noselect dirname switches))

Even if people disliked it in the past and some still dislike it: Try

C-h f
M-x dired

martin

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Stephen Berman]

On Fri, 14 Jul 2017 22:52:18 -0700 John Wiegley <jwiegley@gmail.com> wrote:

>>>>>> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> Thanks for confirming. I think this is the right fix and would commit it,
>> but since this exchange has taken place in a closed bug, it may have fallen
>> under the radar, so I'd like an explicit go-ahead. Eli, John?
>
> At first glance it looks OK to me, though I defer to Eli's experience in such
> matters.

On Sat, 15 Jul 2017 10:18:06 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

> No objections here.
>
> Thanks.

Thanks for okaying, but...

On Sat, 15 Jul 2017 09:36:41 +0200 martin rudalics <rudalics@gmx.at> wrote:

>> +  (set-window-buffer (selected-window)
>> +                     (set-buffer (dired-noselect dirname switches))))
>
> This really should be
>
>      (pop-to-buffer-same-window (dired-noselect dirname switches))
>
> Even if people disliked it in the past and some still dislike it: Try
>
> C-h f
> M-x dired

Wow, that's disastrous!  And frightening: I use the "(set-window-buffer
(selected-window) (set-buffer ...))" idiom a lot in todo-mode.el; I just
checked two commands using `C-h f' followed by the command invocation:
one worked fine but the other caused the same disaster.  So now I have
to check all uses :(.  Is there a general guideline for when to use
set-window-buffer and when to use pop-to-buffer-same-window?

Anyway, I'll commit the dired.el change you recommend; many thanks.

Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[martin rudalics]

 > Wow, that's disastrous!  And frightening: I use the "(set-window-buffer
 > (selected-window) (set-buffer ...))" idiom a lot in todo-mode.el; I just
 > checked two commands using `C-h f' followed by the command invocation:
 > one worked fine but the other caused the same disaster.  So now I have
 > to check all uses :(.  Is there a general guideline for when to use
 > set-window-buffer and when to use pop-to-buffer-same-window?

Always try ‘pop-to-buffer-same-window’ or ‘display-buffer-same-window’
first.  If they fail, try to find out why.  Maybe that can be fixed.

martin

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Stephen Berman]

On Sun, 09 Jul 2017 19:37:53 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:

> Works for me, feel free to merge!
>
> Best,
> Antoine
>
> 17 June 2017 15:32 +02, Stephen Berman <stephen.berman@gmx.net>:
>> On Sat, 17 Jun 2017 14:16:01 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:
>>
>>> I just noticed this is not yet completely fixed: starting from emacs -Q,
>>> (setq dired-auto-revert-buffer t), open dired, open any file in that
>>> directory, C-x d RET to run dired again, the point jumps back to the
>>> beginning of the buffer.
>>
>> I suppose the command `dired' should not use switch-to-buffer.  With the
>> following patch, executing the above recipe does not move point.
>>
>> diff --git a/lisp/dired.el b/lisp/dired.el
>> index 8396652d50..aa59f01af9 100644
>> --- a/lisp/dired.el
>> +++ b/lisp/dired.el
>> @@ -786,7 +786,8 @@ dired
>>  If DIRNAME is already in a Dired buffer, that buffer is used without refresh."
>>    ;; Cannot use (interactive "D") because of wildcards.
>>    (interactive (dired-read-dir-and-switches ""))
>> -  (switch-to-buffer (dired-noselect dirname switches)))
>> +  (set-window-buffer (selected-window)
>> +                     (set-buffer (dired-noselect dirname switches))))
>>  
>>  ;;;###autoload (define-key ctl-x-4-map "d" 'dired-other-window)
>>  ;;;###autoload
>>
>> Steve Berman

On Sat, 15 Jul 2017 09:36:41 +0200 martin rudalics <rudalics@gmx.at> wrote:

>> +  (set-window-buffer (selected-window)
>> +                     (set-buffer (dired-noselect dirname switches))))
>
> This really should be
>
>      (pop-to-buffer-same-window (dired-noselect dirname switches))
>
> Even if people disliked it in the past and some still dislike it: Try
>
> C-h f
> M-x dired

I installed the fix Martin recommended to master as commit b2150e0
(after confirming that it indeed DTRT).  I also added a test for this
case and the first one.

Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Stephen Berman]

On Sat, 15 Jul 2017 15:59:07 +0200 martin rudalics <rudalics@gmx.at> wrote:

>> Wow, that's disastrous!  And frightening: I use the "(set-window-buffer
>> (selected-window) (set-buffer ...))" idiom a lot in todo-mode.el; I just
>> checked two commands using `C-h f' followed by the command invocation:
>> one worked fine but the other caused the same disaster.  So now I have
>> to check all uses :(.  Is there a general guideline for when to use
>> set-window-buffer and when to use pop-to-buffer-same-window?
>
> Always try ‘pop-to-buffer-same-window’ or ‘display-buffer-same-window’
> first.  If they fail, try to find out why.  Maybe that can be fixed.

Thanks for the advice.

Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Antoine Levitt]

Sorry to be the bearer of bad news, but still not completely fixed ;-)
From emacs -Q, (setq dired-auto-revert-buffer t), C-x C-f RET, C-x b
RET, C-x C-f RET, and point jumps.

17 July 2017 11:22 +02, Stephen Berman <stephen.berman@gmx.net>:
> On Sun, 09 Jul 2017 19:37:53 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:
>
>> Works for me, feel free to merge!
>>
>> Best,
>> Antoine
>>
>> 17 June 2017 15:32 +02, Stephen Berman <stephen.berman@gmx.net>:
>>> On Sat, 17 Jun 2017 14:16:01 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:
>>>
>>>> I just noticed this is not yet completely fixed: starting from emacs -Q,
>>>> (setq dired-auto-revert-buffer t), open dired, open any file in that
>>>> directory, C-x d RET to run dired again, the point jumps back to the
>>>> beginning of the buffer.
>>>
>>> I suppose the command `dired' should not use switch-to-buffer.  With the
>>> following patch, executing the above recipe does not move point.
>>>
>>> diff --git a/lisp/dired.el b/lisp/dired.el
>>> index 8396652d50..aa59f01af9 100644
>>> --- a/lisp/dired.el
>>> +++ b/lisp/dired.el
>>> @@ -786,7 +786,8 @@ dired
>>>  If DIRNAME is already in a Dired buffer, that buffer is used without refresh."
>>>    ;; Cannot use (interactive "D") because of wildcards.
>>>    (interactive (dired-read-dir-and-switches ""))
>>> -  (switch-to-buffer (dired-noselect dirname switches)))
>>> +  (set-window-buffer (selected-window)
>>> +                     (set-buffer (dired-noselect dirname switches))))
>>>  
>>>  ;;;###autoload (define-key ctl-x-4-map "d" 'dired-other-window)
>>>  ;;;###autoload
>>>
>>> Steve Berman
>
> On Sat, 15 Jul 2017 09:36:41 +0200 martin rudalics <rudalics@gmx.at> wrote:
>
>>> +  (set-window-buffer (selected-window)
>>> +                     (set-buffer (dired-noselect dirname switches))))
>>
>> This really should be
>>
>>      (pop-to-buffer-same-window (dired-noselect dirname switches))
>>
>> Even if people disliked it in the past and some still dislike it: Try
>>
>> C-h f
>> M-x dired
>
> I installed the fix Martin recommended to master as commit b2150e0
> (after confirming that it indeed DTRT).  I also added a test for this
> case and the first one.
>
> Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Glenn Morris]

Also, the test dired-test-bug27243 fails intermittently.
Eg http://hydra.nixos.org/build/56755248
http://hydra.nixos.org/build/56731329

It switches between passing and failing for no clear reason.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

[Altered Subject to attract more attention]

On Sat, 22 Jul 2017 17:28:59 +0200 Antoine Levitt <antoine.levitt@gmail.com> wrote:

> Sorry to be the bearer of bad news, but still not completely fixed ;-)
>>From emacs -Q, (setq dired-auto-revert-buffer t), C-x C-f RET, C-x b
> RET, C-x C-f RET, and point jumps.

This is exactly the same probem as the last one with dired and the same
change -- using pop-to-buffer-same-window instead of switch-to-buffer in
find-file -- fixes it.  Since the dired change exposed a problematic
dependency on switch-to-buffer in todo-mode.el (since fixed), if there
are other packages that have a similar dependency, making the same
change in find-file runs the risk of changing behavior in these
packages.  But like with todo-mode.el, presumably it would be better to
expose and eliminate such a dependency than to leave it as a possibly
ticking time-bomb.  So should we go ahead and make this change to
find-file?  (I wonder if there are more such cases waiting in the
wings...)

Steve Berman

bug#27243: closed (Re: bug#27243: dired-auto-revert-buffer jumps point to beginning of buffer)

[Stephen Berman]

On Sat, 22 Jul 2017 12:55:41 -0400 Glenn Morris <rgm@gnu.org> wrote:

> Also, the test dired-test-bug27243 fails intermittently.
> Eg http://hydra.nixos.org/build/56755248
> http://hydra.nixos.org/build/56731329
>
> It switches between passing and failing for no clear reason.

Hm, I didn't get a failure when testing this test.  It's too bad the
hydra log doesn't pinpoint where the failure occurs.  Could it be that
the noninteractive use of switch-to-buffer in the test (going proxy for
`C-x b' in the bug recipe) can sometimes mess with point in the test
environment?

Steve Berman

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Sun, 23 Jul 2017 00:48:18 +0200
> Cc: 27243@debbugs.gnu.org
> 
> This is exactly the same probem as the last one with dired and the same
> change -- using pop-to-buffer-same-window instead of switch-to-buffer in
> find-file -- fixes it.  Since the dired change exposed a problematic
> dependency on switch-to-buffer in todo-mode.el (since fixed), if there
> are other packages that have a similar dependency, making the same
> change in find-file runs the risk of changing behavior in these
> packages.  But like with todo-mode.el, presumably it would be better to
> expose and eliminate such a dependency than to leave it as a possibly
> ticking time-bomb.  So should we go ahead and make this change to
> find-file?  (I wonder if there are more such cases waiting in the
> wings...)

Martin, when might pop-to-buffer-same-window show the buffer in a
window other than the selected one?  If we are going to make this
change, we need first to understand when it might misfire, because
some users of find-file might not like the situation where the buffer
pops up in another window.

E.g., what happens in this particular use case if the Dire4d buffer
pops in another window?

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

 > Martin, when might pop-to-buffer-same-window show the buffer in a
 > window other than the selected one?  If we are going to make this
 > change, we need first to understand when it might misfire, because
 > some users of find-file might not like the situation where the buffer
 > pops up in another window.

Conceptually both, ‘pop-to-buffer-same-window’ and ‘switch-to-buffer’,
behave alike in this regard: They avoid the minibuffer and windows
strongly dedicated to their buffers.  Interactively, ‘switch-to-buffer’
may show another buffer in a strongly dedicated window when the option
‘switch-to-buffer-in-dedicated-window’ is non-nil, but I suppose the
issues mentioned in this thread are not about the interactive use of
‘switch-to-buffer’.

 > E.g., what happens in this particular use case if the Dire4d buffer
 > pops in another window?

If the dired buffer was earlier shown by ‘switch-to-buffer’ and is now
shown by ‘pop-to-buffer-same-window’, the behaviors should be the same.
Quitting the dired window, however, may behave differently.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> Date: Sun, 23 Jul 2017 20:40:53 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: antoine.levitt@gmail.com, 27243@debbugs.gnu.org
> 
> Conceptually both, ‘pop-to-buffer-same-window’ and ‘switch-to-buffer’,
> behave alike in this regard: They avoid the minibuffer and windows
> strongly dedicated to their buffers.  Interactively, ‘switch-to-buffer’
> may show another buffer in a strongly dedicated window when the option
> ‘switch-to-buffer-in-dedicated-window’ is non-nil, but I suppose the
> issues mentioned in this thread are not about the interactive use of
> ‘switch-to-buffer’.
> 
>  > E.g., what happens in this particular use case if the Dire4d buffer
>  > pops in another window?
> 
> If the dired buffer was earlier shown by ‘switch-to-buffer’ and is now
> shown by ‘pop-to-buffer-same-window’, the behaviors should be the same.
> Quitting the dired window, however, may behave differently.

I don't think I'm bothered by quitting.  I'm bothered by some feature
calling find-file that could somehow find the file shown in a window
different from the one where the command was invoked.  Are you saying
this isn't more likely to happen than with switch-to-buffer itself?
If so, we can indeed modify find-file to use
pop-to-buffer-same-window.

Thanks.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

 > I don't think I'm bothered by quitting.

The different quitting behavior was what made the test case fail so I
mentioned it.

 > I'm bothered by some feature
 > calling find-file that could somehow find the file shown in a window
 > different from the one where the command was invoked.  Are you saying
 > this isn't more likely to happen than with switch-to-buffer itself?

Yes.  Anything else would be an inconsistency we would have to fix.

 > If so, we can indeed modify find-file to use
 > pop-to-buffer-same-window.

Modulo the fact that Stefan always wanted and probably still wants to
get rid of functions like ‘find-file-other-window’ and
‘find-file-other-frame’ and move their functionality to ‘find-file’ and
I still don't know how to do that and probably never will.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

On Sun, 23 Jul 2017 23:09:50 +0200 martin rudalics <rudalics@gmx.at> wrote:

>> I don't think I'm bothered by quitting.
>
> The different quitting behavior was what made the test case fail so I
> mentioned it.
>
>> I'm bothered by some feature
>> calling find-file that could somehow find the file shown in a window
>> different from the one where the command was invoked.  Are you saying
>> this isn't more likely to happen than with switch-to-buffer itself?
>
> Yes.  Anything else would be an inconsistency we would have to fix.
>
>> If so, we can indeed modify find-file to use
>> pop-to-buffer-same-window.
>
> Modulo the fact that Stefan always wanted and probably still wants to
> get rid of functions like ‘find-file-other-window’ and
> ‘find-file-other-frame’ and move their functionality to ‘find-file’ and
> I still don't know how to do that and probably never will.

Are we waiting for feedback from Stefan (cc'd just in case)?  Or should
I go ahead and push the fix?

Steve Berman

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>,  antoine.levitt@gmail.com,  27243@debbugs.gnu.org
> Date: Fri, 28 Jul 2017 10:49:44 +0200
> 
> >> I'm bothered by some feature
> >> calling find-file that could somehow find the file shown in a window
> >> different from the one where the command was invoked.  Are you saying
> >> this isn't more likely to happen than with switch-to-buffer itself?
> >
> > Yes.  Anything else would be an inconsistency we would have to fix.
> >
> >> If so, we can indeed modify find-file to use
> >> pop-to-buffer-same-window.
> >
> > Modulo the fact that Stefan always wanted and probably still wants to
> > get rid of functions like ‘find-file-other-window’ and
> > ‘find-file-other-frame’ and move their functionality to ‘find-file’ and
> > I still don't know how to do that and probably never will.
> 
> Are we waiting for feedback from Stefan (cc'd just in case)?  Or should
> I go ahead and push the fix?

Unless Martin objects, I think you should push it.

Thanks.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

> Are we waiting for feedback from Stefan (cc'd just in case)?  Or should
> I go ahead and push the fix?

Go ahead and push it.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stefan Monnier]

>>> I'm bothered by some feature calling find-file that could somehow
>>> find the file shown in a window different from the one where the
>>> command was invoked.  Are you saying this isn't more likely to
>>> happen than with switch-to-buffer itself?
>> Yes.  Anything else would be an inconsistency we would have to fix.

Actually, when I do C-x C-f from a minibuffer-only frame it would be
wrong to display the file in that (mini) window, so there are
circumstances where it makes perfect sense for find-file to display the
file in another window.


        Stefan

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: martin rudalics <rudalics@gmx.at>, Eli Zaretskii <eliz@gnu.org>,
>         antoine.levitt@gmail.com, 27243@debbugs.gnu.org
> Date: Fri, 28 Jul 2017 09:02:58 -0400
> 
> >>> I'm bothered by some feature calling find-file that could somehow
> >>> find the file shown in a window different from the one where the
> >>> command was invoked.  Are you saying this isn't more likely to
> >>> happen than with switch-to-buffer itself?
> >> Yes.  Anything else would be an inconsistency we would have to fix.
> 
> Actually, when I do C-x C-f from a minibuffer-only frame it would be
> wrong to display the file in that (mini) window, so there are
> circumstances where it makes perfect sense for find-file to display the
> file in another window.

Doesn't this already happen, and will continue happening after the
proposed change?

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stefan Monnier]

> Doesn't this already happen, and will continue happening after the
> proposed change?

Yes.

I was just pointing out that it's not quite true that "anything else
would be an inconsistency".


        Stefan

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: stephen.berman@gmx.net, rudalics@gmx.at, antoine.levitt@gmail.com,
>         27243@debbugs.gnu.org
> Date: Fri, 28 Jul 2017 09:16:00 -0400
> 
> > Doesn't this already happen, and will continue happening after the
> > proposed change?
> 
> Yes.
> 
> I was just pointing out that it's not quite true that "anything else
> would be an inconsistency".

AFAIU, Martin was answering my question:

> Are you saying this isn't more likely to happen than with
> switch-to-buffer itself?

IOW, the inconsistency would happen if the current behavior would
change when switch-to-buffer is replaced with
pop-to-buffer-same-window, not if find-file displayed in another
window.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

 >> Doesn't this already happen, and will continue happening after the
 >> proposed change?
 >
 > Yes.

Here it doesn't already happen and will not continue happening
after the proposed change.

 > I was just pointing out that it's not quite true that "anything else
 > would be an inconsistency".

It's quite true here.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Eli Zaretskii]

> Date: Fri, 28 Jul 2017 16:14:50 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: stephen.berman@gmx.net, antoine.levitt@gmail.com, 
>  27243@debbugs.gnu.org
> 
>  >> Doesn't this already happen, and will continue happening after the
>  >> proposed change?
>  >
>  > Yes.
> 
> Here it doesn't already happen and will not continue happening
> after the proposed change.

As long as the current behavior doesn't change, it's fine with me.

Thanks.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

On Fri, 28 Jul 2017 17:45:01 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> Date: Fri, 28 Jul 2017 16:14:50 +0200
>> From: martin rudalics <rudalics@gmx.at>
>> CC: stephen.berman@gmx.net, antoine.levitt@gmail.com, 
>>  27243@debbugs.gnu.org
>> 
>>  >> Doesn't this already happen, and will continue happening after the
>>  >> proposed change?
>>  >
>>  > Yes.
>> 
>> Here it doesn't already happen and will not continue happening
>> after the proposed change.
>
> As long as the current behavior doesn't change, it's fine with me.

I agree with Martin, using a minibuffer-only setup doesn't change
things:

1. emacs -Q --eval "(setq initial-frame-alist '((minibuffer . nil)))"
   ;; Now there's a minibufferless frame and a minibuffer-only frame.
2. M-: (setq dired-auto-revert-buffer t)
3. C-x C-f RET ; Now the default directory is shown with point on the
               ; first entry.
4. C-x b ; Back to *scratch*.
5. C-x C-f RET ; Now the default directory is shown with point at
               ; point-min.  This is the bug.

When switch-to-buffer in find-file is replaced by
pop-to-buffer-same-window and the above recipe is repeated, after step 5
point has not moved to point-min but is still on the first entry as in
step 3.  So using pop-to-buffer-same-window works here.  Or does Stefan
have some other setup?

Steve Berman

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

On Fri, 28 Jul 2017 17:45:01 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> Date: Fri, 28 Jul 2017 16:14:50 +0200
>> From: martin rudalics <rudalics@gmx.at>
>> CC: stephen.berman@gmx.net, antoine.levitt@gmail.com, 
>>  27243@debbugs.gnu.org
>> 
>>  >> Doesn't this already happen, and will continue happening after the
>>  >> proposed change?
>>  >
>>  > Yes.
>> 
>> Here it doesn't already happen and will not continue happening
>> after the proposed change.
>
> As long as the current behavior doesn't change, it's fine with me.

I pushed the fix to master (8e394b082b).  I also added a test for this
case; even though the change is to find-file, I added the test to
dired-tests.el, since it's testing dired-auto-revert-buffer and belongs
with the other two cases of this bug.  (I also pulled apart the test of
the two previous cases into two separate tests.  I did this because the
Hydra build system shows the previous combined test failing
intermittently; it doesn't fail for me with make check, and AFAICS the
test doesn't depend on anything outside of the test environment, so it's
a mystery why it sometimes fails on Hydra, but maybe separately testing
the two cases, though nearly identical, will help locate the point of
failure.)

Steve Berman

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Michael Heerdegen]

Stephen Berman <stephen.berman@gmx.net> writes:

> I pushed the fix to master (8e394b082b).

Unfortunately this brakes a different use case for me.  Since
`pop-to-buffer-same-window' doesn't respect
`switch-to-buffer-preserve-window-point', when I use RET on a directory
in dired and the target buffer had already been displayed in the
selected-window, Emacs fails to restore the corresponding old
window-point now.  Works when I revert that commit.

Sorry if this message is a kind of duplicate for you - pilot error with
unarchiving...

Michael.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Stephen Berman]

On Mon, 25 May 2020 05:51:07 +0200 Michael Heerdegen <michael_heerdegen@web.de> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> > As long as the current behavior doesn't change, it's fine with me.
>>
>> I pushed the fix to master (8e394b082b).
>
> This broke a different use case for me.
>
> I'm using switch-to-buffer-preserve-window-point > t and
> dired-auto-revert-buffer > nil.
>
> Say I have a frame showing the same dired buffer twice, with different
> values of point, in two windows.  When I hit ^ and then RET in
> succession in the first window, I'm back in that buffer but point now
> equals window-point of the second window.  Emacs failed to restore the
> correct value of point.  If I revert this commit which replaced
> `switch-to-buffer' with `pop-to-buffer-same-window' in `find-file', the
> correct value of point is restored as expected.
> `pop-to-buffer-same-window' probably doesn't care about
> `switch-to-buffer-preserve-window-point' at all?

Is this with -Q?  I don't see this with -Q in a build from (not current
but recent) master.  I tried to reproduce what you described as follows:

1. emacs -Q
2. M-: switch-to-buffer-preserve-window-point RET => t
   M-: dired-auto-revert-buffer RET => nil
3. C-x d RET M-: (point) => 211
4. C-x 2 C-x o 10 n M-: (point) => 890
5. C-x o ^ RET M-: (point) => 211
also: 6. C-x o ^ RET M-: (point) => 890

If Emacs behaves as you described, the result in step 5 should have been
890, or have I misunderstood you?

Steve Berman

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Michael Heerdegen]

Stephen Berman <stephen.berman@gmx.net> writes:

> If Emacs behaves as you described, the result in step 5 should have been
> 890, or have I misunderstood you?

Hmm indeed, it doesn't seem to happen with emacs -Q.  Sorry, I didn't
expect that, it was late when I wrote that message.

I spent several hours to debug this issue with my setup.  It behaves
like a Heisenbug.  I cannot reliably reproduce, so I can't just bisect
my setup.  And whenever I thought I have a way to debug...it's gone.
Debugging is hard anyway, since AFAICT redisplay plays a certain role.

I only know that when reverting your patch, the problem is gone as well
(I guess quantum Emacs counts this as a debugging attempt).

I've no idea how to make progress, so feel free to ignore, since the
issue is not hard to avoid for me by rebinding RET in dired.

Michael.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

 > This broke a different use case for me.
 >
 > I'm using switch-to-buffer-preserve-window-point > t and
 > dired-auto-revert-buffer > nil.

Would setting 'switch-to-buffer-preserve-window-point' to
'already-displayed' change anything in the behavior?  Or setting
'switch-to-buffer-obey-display-actions'?  Maybe we missed something when
introducing those.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Michael Heerdegen]

martin rudalics <rudalics@gmx.at> writes:

> Would setting 'switch-to-buffer-preserve-window-point' to
> 'already-displayed' change anything in the behavior?  Or setting
> 'switch-to-buffer-obey-display-actions'?  Maybe we missed something when
> introducing those.

I don't think these are related.  I had already played with
switch-to-buffer-preserve-window-point -> already-displayed, that
doesn't make any difference.  And
`switch-to-buffer-obey-display-actions' should not be referenced at all.

Michael.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Michael Heerdegen]

Stephen Berman <stephen.berman@gmx.net> writes:

> 1. emacs -Q
> 2. M-: switch-to-buffer-preserve-window-point RET => t
>    M-: dired-auto-revert-buffer RET => nil
> 3. C-x d RET M-: (point) => 211
> 4. C-x 2 C-x o 10 n M-: (point) => 890
> 5. C-x o ^ RET M-: (point) => 211
> also: 6. C-x o ^ RET M-: (point) => 890

But if I do this with emacs -Q, and use edebug to step through the
related functions, I can reproduce it there.  It is enough to edebug
just `dired-find-file' and hit SPC, SPC ... for the issue to occur as I
see it.

I was using two different frames for Edebugging and performing the
recipe.

Does this confirm my suspicions that intermediate redisplay may play a
role?

Michael.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Does this confirm my suspicions that intermediate redisplay may play a
> role?

Yes, I guess that's it.

In the recipe, when you are in emacs -Q, it's enough to select the
second window before the last step to make the issue happen.

There is no restoration of the old window's window-point, I think it's
the buffer's current value of point that becomes window-point in the
first window.  I verified here that when running the recipe the function
`window-prev-buffers' is never consulted.

Michael.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

 > In the recipe, when you are in emacs -Q, it's enough to select the
 > second window before the last step to make the issue happen.

Anything that moves point of that buffer elsewhere.  This includes
selecting another window on that buffer whose window-point does not
equal that of the window we switched away from.

 > There is no restoration of the old window's window-point, I think it's
 > the buffer's current value of point that becomes window-point in the
 > first window.  I verified here that when running the recipe the function
 > `window-prev-buffers' is never consulted.

Right.  Maybe we should call 'switch-to-buffer' here so people with
'switch-to-buffer-obey-display-actions' non-nil get the
'pop-to-buffer-same-window' behavior and others are unaffected.  Let's
wait for Juri to comment.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Michael Heerdegen]

martin rudalics <rudalics@gmx.at> writes:

>  Maybe we should call 'switch-to-buffer' here so people with
> 'switch-to-buffer-obey-display-actions' non-nil get the
> 'pop-to-buffer-same-window' behavior and others are unaffected.

The code once used `switch-to-buffer' but had been changed to use
`pop-to-buffer-same-window' when fixing one aspect of this Bug report.

Michael.

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[martin rudalics]

 > The code once used `switch-to-buffer' but had been changed to use
 > `pop-to-buffer-same-window' when fixing one aspect of this Bug report.

Right.  But that change happened some time before Juri added
'switch-to-buffer-obey-display-actions'.

martin

bug#27243: another case: dired-auto-revert-buffer jumps point to beginning of buffer

[Juri Linkov]

>> There is no restoration of the old window's window-point, I think it's
>> the buffer's current value of point that becomes window-point in the
>> first window.  I verified here that when running the recipe the function
>> `window-prev-buffers' is never consulted.
>
> Right.  Maybe we should call 'switch-to-buffer' here so people with
> 'switch-to-buffer-obey-display-actions' non-nil get the
> 'pop-to-buffer-same-window' behavior and others are unaffected.  Let's
> wait for Juri to comment.

Yes, now 'pop-to-buffer-same-window' could be changed back to 'switch-to-buffer'.

But we need to ask Stephen if reverting back to 'switch-to-buffer'
and customizing 'switch-to-buffer-obey-display-actions' to non-nil
really provides the same behavior.