Re: master b2150e0: Preserve point under 'dired-auto-revert-buffer' (second case)

Re: master b2150e0: Preserve point under 'dired-auto-revert-buffer' (second case)

[Glenn Morris]

Stephen Berman wrote:

> branch: master
> commit b2150e0b02fa4a7ad4c1461e0b4ff8fd632c0fb8

This change causes todo-test-todo-quit02 (which uses dired) to fail.

Ref eg http://hydra.nixos.org/build/56529421

Re: master b2150e0: Preserve point under 'dired-auto-revert-buffer' (second case)

[Stephen Berman]

On Mon, 17 Jul 2017 19:58:53 -0400 Glenn Morris <rgm@gnu.org> wrote:

> Stephen Berman wrote:
>
>> branch: master
>> commit b2150e0b02fa4a7ad4c1461e0b4ff8fd632c0fb8
>
> This change causes todo-test-todo-quit02 (which uses dired) to fail.
>
> Ref eg http://hydra.nixos.org/build/56529421

Sorry about that; I swear to Saint IGNUcius I'll always run `make check'
before pushing any future test changes.

I think I see what the problem is, but I don't know what to do about it.

The test fails now because, after the change to dired (replacing
switch-to-buffer by pop-to-buffer-same-window), calling quit-window in
the directory results in buffer-list returning a different order of the
buffers than the order returned before the change, and the test assumes
the previous order (as does todo-quit, which is what is being tested
here).  If I debugged this correctly, it seems that the crucial
difference is that, on calling quit-window after
pop-to-buffer-same-window, the window displaying the directory has a
non-nil quit-restore window parameter (in fact, its value contains the
buffer visiting the todo-file), while on calling quit-window after
switch-to-buffer, the quit-restore window parameter is nil.  Is this
correct (cc'ing Martin)?

Here is a recipe that shows the same effect without using todo-mode or
dired:

Start Emacs with -Q and evaluate the following sexp:

(progn
  (require 'cl-lib)
  (defun bl (l)
    (cl-remove-if (lambda (x) (string-match " \\*" (buffer-name x))) l))
  (message "First: %S" (bl (buffer-list)))
  (set-window-buffer (selected-window)
                     (set-buffer (find-file-noselect "bla")))
  (message "After s-w-b 1: %S" (bl (buffer-list)))
  (pop-to-buffer-same-window default-directory)
  (message "After pop: %S" (bl (buffer-list)))
  ;; (switch-to-buffer default-directory)
  ;; (message "After switch: %S" (bl (buffer-list)))
  (set-window-buffer (selected-window)
                     (set-buffer (find-file-noselect "bla")))
  (message "After s-w-b 2: %S" (bl (buffer-list)))
  (bury-buffer)
  (message "After bury-buffer: %S" (bl (buffer-list)))
  (quit-window)
  (message "After quit-window: %S" (bl (buffer-list))))

This is what *Messages* shows:

First: (#<buffer *scratch*> #<buffer *Messages*>)
(New file)
After s-w-b 1: (#<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After pop: (#<buffer /home/steve/> #<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After s-w-b 2: (#<buffer /home/steve/> #<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After bury-buffer: (#<buffer /home/steve/> #<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After quit-window: (#<buffer bla> #<buffer *scratch*> #<buffer *Messages*> #<buffer /home/steve/>)

Now start Emacs with -Q again, yank in the above sexp, uncomment the two
commented out lines and comment out the two lines preceding these (so
switch-to-buffer and the appropriate message are used), then evaluate
the sexp; now *Messages* shows this:

First: (#<buffer *scratch*> #<buffer *Messages*>)
(New file)
After s-w-b 1: (#<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After switch: (#<buffer /home/steve/> #<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After s-w-b 2: (#<buffer /home/steve/> #<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After bury-buffer: (#<buffer /home/steve/> #<buffer *scratch*> #<buffer *Messages*> #<buffer bla>)
After quit-window: (#<buffer *scratch*> #<buffer *Messages*> #<buffer bla> #<buffer /home/steve/>)

The only difference in the buffer lists of the two outputs is after
quit-window (the last line in each output).  Is this difference due to
the different quit-restore window parameter values?  If so, I'm not sure
what the best course of action is.  This is what's supposed to happen
(what the test tests for): (1) the todo-mode buffer is displayed in a
window; (2) calling dired makes the window display a directory; (3)
calling todo-show makes the window display the todo-mode buffer again;
(4) calling todo-quit buries the todo-mode buffer, so the window
displays the directory again; (5) now immediately calling quit-window in
the directory should not make the window display the todo-mode buffer
again, if there is an alternative.  This worked when dired used
switch-to-buffer, but no longer works now that dired uses
pop-to-buffer-same-window.  I'd be grateful for any advice.

Steve Berman

Re: master b2150e0: Preserve point under 'dired-auto-revert-buffer' (second case)

[martin rudalics]

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

 > If I debugged this correctly, it seems that the crucial
 > difference is that, on calling quit-window after
 > pop-to-buffer-same-window, the window displaying the directory has a
 > non-nil quit-restore window parameter (in fact, its value contains the
 > buffer visiting the todo-file), while on calling quit-window after
 > switch-to-buffer, the quit-restore window parameter is nil.  Is this
 > correct (cc'ing Martin)?

Yes.  ‘switch-to-buffer’ misses a number of features which make it
behave differently from ‘display-buffer’ based functions.  In
particular, it cannot be used for displaying buffers temporarily.

 > The only difference in the buffer lists of the two outputs is after
 > quit-window (the last line in each output).  Is this difference due to
 > the different quit-restore window parameter values?

‘quit-window’ works on a per window basis.  Earlier the situation was as
follows: Consider two windows showing two different portions of the same
buffer.  If in one of these windows you temporarily worked on another
buffer, then quitting that window would not restore the buffer
previously shown in it because there is already another window showing
that buffer.  Even if it did restore the buffer, it would not have
restored that window's position of point.  ‘quit-window’ simply tries to
restore the buffer as previously shown in that window regardless of what
the buffer list says.

 > If so, I'm not sure
 > what the best course of action is.  This is what's supposed to happen
 > (what the test tests for): (1) the todo-mode buffer is displayed in a
 > window; (2) calling dired makes the window display a directory; (3)
 > calling todo-show makes the window display the todo-mode buffer again;
 > (4) calling todo-quit buries the todo-mode buffer, so the window
 > displays the directory again; (5) now immediately calling quit-window in
 > the directory should not make the window display the todo-mode buffer
 > again, if there is an alternative.  This worked when dired used
 > switch-to-buffer, but no longer works now that dired uses
 > pop-to-buffer-same-window.  I'd be grateful for any advice.

The problematic step is (4).  ‘quit-window’ has no idea of the semantics
of ‘bury-buffer’ (neither do I).  But if no one objects we could install
the attached patch.

martin

[-- Attachment #2: window.el.diff --]
[-- Type: text/plain, Size: 787 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 2b979f4..20864c6 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4627,6 +4627,15 @@ bury-buffer
      ((or buffer-or-name (not (eq buffer (window-buffer)))))
      ((window--delete nil t))
      (t
+      (let ((quit-restore (window-parameter nil 'quit-restore))
+	    quad)
+	(when (and (eq (car quit-restore) 'other)
+		   (consp (setq quad (nth 1 quit-restore)))
+		   (eq (car quad) buffer))
+	  ;; Reset selected window's 'quit-restore' parameter to avoid
+	  ;; that the buffer is restored in that window by a subsequent
+	  ;; ‘quit-window'.
+	  (set-window-parameter nil 'quit-restore nil)))
       ;; Switch to another buffer in window.
       (set-window-dedicated-p nil nil)
       (switch-to-prev-buffer nil 'bury)))

Re: master b2150e0: Preserve point under 'dired-auto-revert-buffer' (second case)

[Stephen Berman]

On Wed, 19 Jul 2017 10:18:32 +0200 martin rudalics <rudalics@gmx.at> wrote:

>> This is what's supposed to happen
>> (what the test tests for): (1) the todo-mode buffer is displayed in a
>> window; (2) calling dired makes the window display a directory; (3)
>> calling todo-show makes the window display the todo-mode buffer again;
>> (4) calling todo-quit buries the todo-mode buffer, so the window
>> displays the directory again; (5) now immediately calling quit-window in
>> the directory should not make the window display the todo-mode buffer
>> again, if there is an alternative.  This worked when dired used
>> switch-to-buffer, but no longer works now that dired uses
>> pop-to-buffer-same-window.  I'd be grateful for any advice.
>
> The problematic step is (4).  ‘quit-window’ has no idea of the semantics
> of ‘bury-buffer’ (neither do I).  But if no one objects we could install
> the attached patch.

Thanks for the patch (and the clarifications).  But I feel uneasy about
changing a general function to cater to the specific needs of todo-mode,
unless it is certain that the change would not cause problems for other
existing uses.  This unease prompted me to look at todo-quit again, and
it turns out that replacing bury-buffer there by quit-window DTRT as
described above.  So I will make that change and there's no need to
change bury-buffer.  This should also mean the existing test will work
again without change (I'll confirm with `make check' before pushing).
(I had taken over the invocation of bury-buffer in todo-quit from the
old first version of todo-mode.el, which I believe predates the
introduction quit-window, and till now had not second-guessed its use.
Better late than never.)

Thanks for the feedback.

Steve Berman