bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[Zachary Kanfer]

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

The command list-buffers always puts point at the end of the buffer. This
patch changes the list-buffers command to simply revert the buffer if point
is inside the *Buffer List* buffer. Since point is kept at the same line,
the user can more easily see what changes have occurred.
 -Zachary Kanfer


=== modified file 'lisp/buff-menu.el'
*** lisp/buff-menu.el    2011-09-18 20:43:20 +0000
--- lisp/buff-menu.el    2011-10-15 03:59:12 +0000
*************** Non-null optional arg FILES-ONLY means m
*** 652,658 ****

  For more information, see the function `buffer-menu'."
    (interactive "P")
!   (display-buffer (list-buffers-noselect files-only)))

  (defconst Buffer-menu-short-ellipsis
    ;; This file is preloaded, so we can't use char-displayable-p here
--- 652,661 ----

  For more information, see the function `buffer-menu'."
    (interactive "P")
!   (if (string= (buffer-name)
!                "*Buffer List*")
!       (revert-buffer)
!     (display-buffer (list-buffers-noselect files-only))))

  (defconst Buffer-menu-short-ellipsis
    ;; This file is preloaded, so we can't use char-displayable-p here

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

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[martin rudalics]

 > !   (if (string= (buffer-name)
 > !                "*Buffer List*")
 > !       (revert-buffer)

This doesn't strike me as TRT when "*Buffer List*" is currently not
visible in any window.  So you probably want something like

(if (and (string= (buffer-name) "*Buffer List*")
	 (get-buffer-window (buffer-name) 0))
     (revert-buffer)
   (display-buffer (list-buffers-noselect files-only)))

here (but I don't use `list-buffers').

 > !     (display-buffer (list-buffers-noselect files-only))))

martin

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[Chong Yidong]

Zachary Kanfer <zkanfer@gmail.com> writes:

> The command list-buffers always puts point at the end of the
> buffer. This patch changes the list-buffers command to simply revert
> the buffer if point is inside the *Buffer List* buffer. Since point is
> kept at the same line, the user can more easily see what changes have
> occurred.

This doesn't seem like a good idea.  It is better for `list-buffers' to
behave as unsurprisingly as possible, i.e. regardless of whether it's
called from a buffer list.

After all, the user can always call revert-buffer if that's what's
desired.

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[Zachary Kanfer]

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

I agree -- less surprising is better. To me, that means *not* moving point
when it's called again. It makes it easier for the user to answer the
question "what's changed?", which, at least for me, is a common question
when I already have a buffer list window open. Would you be in favor of this
change if I can use Martin's suggestion so that the behavior is the same no
matter if point is in the Buffer List buffer or not?

You are right, though -- the user can call revert-buffer (which is bound to
'g' in Buffer Menu mode) if desired. But for calling `list-buffers', I think
that for the vast majority of cases, keeping point where it is is either
superior to putting it at the bottom, or just as good. I can't think of too
many cases where moving point to the bottom of the buffer is better.

-Zachary

On Sat, Oct 15, 2011 at 1:14 PM, Chong Yidong <cyd@stupidchicken.com> wrote:

> Zachary Kanfer <zkanfer@gmail.com> writes:
>
> > The command list-buffers always puts point at the end of the
> > buffer. This patch changes the list-buffers command to simply revert
> > the buffer if point is inside the *Buffer List* buffer. Since point is
> > kept at the same line, the user can more easily see what changes have
> > occurred.
>
> This doesn't seem like a good idea.  It is better for `list-buffers' to
> behave as unsurprisingly as possible, i.e. regardless of whether it's
> called from a buffer list.
>
> After all, the user can always call revert-buffer if that's what's
> desired.
>
>

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

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[Stefan Monnier]

> !   (display-buffer (list-buffers-noselect files-only)))
[...]
> !   (if (string= (buffer-name)
> !                "*Buffer List*")
> !       (revert-buffer)
> !     (display-buffer (list-buffers-noselect files-only))))

I don't think calling revert-buffer is right.  Much better would be for
list-buffer-noselect to try to preserve point when reusing a buffer
rather than creating a new one.


        Stefan

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[Zachary Kanfer]

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

I have included two diffs in this email. The first is the diff without
whitespace, generated by bzr diff lisp/buff-menu.el --no-aliases
--diff-options="-cp -w" . The second is the full diff, as generated by bzr
diff lisp/buff-menu.el --no-aliases --diff-options=-cp . I introduced a
`let` statement which wraps a large majority of the function, so a large
part of the full diff is merely whitespace changes. The no-whitespace diff
shows more clearly the code changes I've made. I could have used setq to
avoid introducing a new level of nesting, but didn't want to leak new
variables outside the function. I will change it, if advised that it's
better practice.

The majority of my changes happen near the end of the method. I get the
*Buffer List* window, and the current window. If the *Buffer List* window
exists, I select it. I then move point to the correct position, and restore
the selected window if necessary.

Diff #1 (diff ignoring whitespace):

=== modified file 'lisp/buff-menu.el'
*** lisp/buff-menu.el    2011-09-18 20:43:20 +0000
--- lisp/buff-menu.el    2011-12-09 06:18:25 +0000
*************** For more information, see the function `
*** 807,812 ****
--- 807,815 ----
        (setq header (concat (propertize " " 'display '(space :align-to 0))
                 header)))
      (with-current-buffer (get-buffer-create "*Buffer List*")
+       (let ((line-number (line-number-at-pos))
+             (column-number (current-column)))
+         (message (concat "line: " (number-to-string line-number) "\tcol:
" (number-to-string column-number)))
          (setq buffer-read-only nil)
          (erase-buffer)
          (setq standard-output (current-buffer))
*************** For more information, see the function `
*** 857,863 ****
                 ?\s)))
          (unless file
            ;; No visited file.  Check local value of
!           ;; list-buffers-directory and, for Info buffers,
            ;; Info-current-file.
            (cond ((and (boundp 'list-buffers-directory)
                    list-buffers-directory)
--- 860,866 ----
                               ?\s)))
                    (unless file
                      ;; No visited file.  Check local value of
!                     ;; list-buffers-diectory and, for Info buffers,
                      ;; Info-current-file.
                      (cond ((and (boundp 'list-buffers-directory)
                                  list-buffers-directory)
*************** For more information, see the function `
*** 896,903 ****
                   (string< (nth Buffer-menu-sort-column a)
                        (nth Buffer-menu-sort-column b)))))
           list))
-     (if (eq (car buffer) old-buffer)
-         (setq desired-point (point)))
      (insert (cadr buffer)
          ;; Put the buffer name into a text property
          ;; so we don't have to extract it from the text.
--- 899,904 ----
*************** For more information, see the function `
*** 929,941 ****
        (Buffer-menu-mode)
        (when Buffer-menu-use-header-line
      (setq header-line-format header))
!       ;; DESIRED-POINT doesn't have to be set; it is not when the
!       ;; current buffer is not displayed for some reason.
!       (and desired-point
!        (goto-char desired-point))
        (setq Buffer-menu-files-only files-only)
        (setq Buffer-menu--buffers buffer-list)
        (set-buffer-modified-p nil)
!       (current-buffer))))

  ;;; buff-menu.el ends here
--- 930,950 ----
          (Buffer-menu-mode)
          (when Buffer-menu-use-header-line
            (setq header-line-format header))
!         (let ((buffer-list-window (get-window-with-predicate
!                                    (lambda (window)
!                                          (string= "*Buffer List*"
!                                                   (buffer-name
(window-buffer window))))))
!               (current-window (selected-window)))
!           (when buffer-list-window
!             (select-window buffer-list-window))
!           (goto-char (point-min))
!           (forward-line (1- line-number))
!           (forward-char column-number)
!           (when buffer-list-window
!             (select-window current-window)))
          (setq Buffer-menu-files-only files-only)
          (setq Buffer-menu--buffers buffer-list)
          (set-buffer-modified-p nil)
!         (current-buffer)))))

  ;;; buff-menu.el ends here



Diff #2 (full diff):

=== modified file 'lisp/buff-menu.el'
*** lisp/buff-menu.el    2011-09-18 20:43:20 +0000
--- lisp/buff-menu.el    2011-12-09 06:18:25 +0000
*************** For more information, see the function `
*** 807,941 ****
        (setq header (concat (propertize " " 'display '(space :align-to 0))
                 header)))
      (with-current-buffer (get-buffer-create "*Buffer List*")
!       (setq buffer-read-only nil)
!       (erase-buffer)
!       (setq standard-output (current-buffer))
!       ;; Force L2R direction, to avoid messing the display if the
!       ;; first buffer in the list happens to begin with a strong R2L
!       ;; character.
!       (setq bidi-paragraph-direction 'left-to-right)
!       (unless Buffer-menu-use-header-line
!         ;; Use U+2014 (EM DASH) to underline if possible, else use ASCII
!         ;; (i.e. U+002D, HYPHEN-MINUS).
!     (let ((underline (if (char-displayable-p ?\u2014) ?\u2014 ?-)))
!       (insert header
!           (apply 'string
!              (mapcar (lambda (c)
!                    (if (memq c '(?\n ?\s)) c underline))
!                  header)))))
!       ;; Collect info for every buffer we're interested in.
!       (dolist (buffer (or buffer-list
!               (buffer-list
!                (when Buffer-menu-use-frame-buffer-list
!                  (selected-frame)))))
!     (with-current-buffer buffer
!       (let ((name (buffer-name))
!         (file buffer-file-name))
!         (unless (and (not buffer-list)
!              (or
!               ;; Don't mention internal buffers.
!               (and (string= (substring name 0 1) " ") (null file))
!               ;; Maybe don't mention buffers without files.
!               (and files-only (not file))
!               (string= name "*Buffer List*")))
!           ;; Otherwise output info.
!           (let ((mode (concat (format-mode-line mode-name nil nil buffer)
!                   (if mode-line-process
!                       (format-mode-line mode-line-process
!                             nil nil buffer))))
!             (bits (string
!                (if (eq buffer old-buffer) ?. ?\s)
!                ;; Handle readonly status.  The output buffer
!                ;; is special cased to appear readonly; it is
!                ;; actually made so at a later date.
!                (if (or (eq buffer standard-output)
!                    buffer-read-only)
!                    ?% ?\s)
!                ;; Identify modified buffers.
!                (if (buffer-modified-p) ?* ?\s)
!                ;; Space separator.
!                ?\s)))
!         (unless file
!           ;; No visited file.  Check local value of
!           ;; list-buffers-directory and, for Info buffers,
!           ;; Info-current-file.
!           (cond ((and (boundp 'list-buffers-directory)
!                   list-buffers-directory)
!              (setq file list-buffers-directory))
!             ((eq major-mode 'Info-mode)
!              (setq file Info-current-file)
!              (cond
!               ((equal file "dir")
!                (setq file "*Info Directory*"))
!               ((eq file 'apropos)
!                (setq file "*Info Apropos*"))
!               ((eq file 'history)
!                (setq file "*Info History*"))
!               ((eq file 'toc)
!                (setq file "*Info TOC*"))
!               ((not (stringp file))  ;; avoid errors
!                (setq file nil))
!               (t
!                (setq file (concat "("
!                           (file-name-nondirectory file)
!                           ") "
!                           Info-current-node)))))))
!         (push (list buffer bits name (buffer-size) mode file)
!               list))))))
!       ;; Preserve the original buffer-list ordering, just in case.
!       (setq list (nreverse list))
!       ;; Place the buffers's info in the output buffer, sorted if
necessary.
!       (dolist (buffer
!            (if Buffer-menu-sort-column
!            (sort list
!              (if (eq Buffer-menu-sort-column 3)
!                  (lambda (a b)
!                    (< (nth Buffer-menu-sort-column a)
!                   (nth Buffer-menu-sort-column b)))
!                (lambda (a b)
!                  (string< (nth Buffer-menu-sort-column a)
!                       (nth Buffer-menu-sort-column b)))))
!          list))
!     (if (eq (car buffer) old-buffer)
!         (setq desired-point (point)))
!     (insert (cadr buffer)
!         ;; Put the buffer name into a text property
!         ;; so we don't have to extract it from the text.
!         ;; This way we avoid problems with unusual buffer names.
!         (let ((name (nth 2 buffer))
!               (size (int-to-string (nth 3 buffer))))
!           (Buffer-menu-buffer+size name size
!                  `(buffer-name ,name
!                        buffer ,(car buffer)
!                        font-lock-face buffer-menu-buffer
!                        mouse-face highlight
!                        help-echo
!                        ,(if (>= (length name)
!                         (- Buffer-menu-buffer+size-width
!                            (max (length size) 3)
!                            2))
!                         name
!                       "mouse-2: select this buffer"))))
!         "  "
!         (if (> (string-width (nth 4 buffer)) Buffer-menu-mode-width)
!             (truncate-string-to-width (nth 4 buffer)
!                           Buffer-menu-mode-width)
!           (nth 4 buffer)))
!     (when (nth 5 buffer)
!       (indent-to (+ Buffer-menu-buffer-column
Buffer-menu-buffer+size-width
!             Buffer-menu-mode-width 4) 1)
!       (princ (abbreviate-file-name (nth 5 buffer))))
!     (princ "\n"))
!       (Buffer-menu-mode)
!       (when Buffer-menu-use-header-line
!     (setq header-line-format header))
!       ;; DESIRED-POINT doesn't have to be set; it is not when the
!       ;; current buffer is not displayed for some reason.
!       (and desired-point
!        (goto-char desired-point))
!       (setq Buffer-menu-files-only files-only)
!       (setq Buffer-menu--buffers buffer-list)
!       (set-buffer-modified-p nil)
!       (current-buffer))))

  ;;; buff-menu.el ends here
--- 807,950 ----
        (setq header (concat (propertize " " 'display '(space :align-to 0))
                 header)))
      (with-current-buffer (get-buffer-create "*Buffer List*")
!       (let ((line-number (line-number-at-pos))
!             (column-number (current-column)))
!         (message (concat "line: " (number-to-string line-number) "\tcol:
" (number-to-string column-number)))
!         (setq buffer-read-only nil)
!         (erase-buffer)
!         (setq standard-output (current-buffer))
!         ;; Force L2R direction, to avoid messing the display if the
!         ;; first buffer in the list happens to begin with a strong R2L
!         ;; character.
!         (setq bidi-paragraph-direction 'left-to-right)
!         (unless Buffer-menu-use-header-line
!           ;; Use U+2014 (EM DASH) to underline if possible, else use ASCII
!           ;; (i.e. U+002D, HYPHEN-MINUS).
!           (let ((underline (if (char-displayable-p ?\u2014) ?\u2014 ?-)))
!             (insert header
!                     (apply 'string
!                            (mapcar (lambda (c)
!                                      (if (memq c '(?\n ?\s)) c underline))
!                                    header)))))
!         ;; Collect info for every buffer we're interested in.
!         (dolist (buffer (or buffer-list
!                             (buffer-list
!                              (when Buffer-menu-use-frame-buffer-list
!                                (selected-frame)))))
!           (with-current-buffer buffer
!             (let ((name (buffer-name))
!                   (file buffer-file-name))
!               (unless (and (not buffer-list)
!                            (or
!                             ;; Don't mention internal buffers.
!                             (and (string= (substring name 0 1) " ") (null
file))
!                             ;; Maybe don't mention buffers without files.
!                             (and files-only (not file))
!                             (string= name "*Buffer List*")))
!                 ;; Otherwise output info.
!                 (let ((mode (concat (format-mode-line mode-name nil nil
buffer)
!                                     (if mode-line-process
!                                         (format-mode-line
mode-line-process
!                                                           nil nil
buffer))))
!                       (bits (string
!                              (if (eq buffer old-buffer) ?. ?\s)
!                              ;; Handle readonly status.  The output buffer
!                              ;; is special cased to appear readonly; it is
!                              ;; actually made so at a later date.
!                              (if (or (eq buffer standard-output)
!                                      buffer-read-only)
!                                  ?% ?\s)
!                              ;; Identify modified buffers.
!                              (if (buffer-modified-p) ?* ?\s)
!                              ;; Space separator.
!                              ?\s)))
!                   (unless file
!                     ;; No visited file.  Check local value of
!                     ;; list-buffers-diectory and, for Info buffers,
!                     ;; Info-current-file.
!                     (cond ((and (boundp 'list-buffers-directory)
!                                 list-buffers-directory)
!                            (setq file list-buffers-directory))
!                           ((eq major-mode 'Info-mode)
!                            (setq file Info-current-file)
!                            (cond
!                             ((equal file "dir")
!                              (setq file "*Info Directory*"))
!                             ((eq file 'apropos)
!                              (setq file "*Info Apropos*"))
!                             ((eq file 'history)
!                              (setq file "*Info History*"))
!                             ((eq file 'toc)
!                              (setq file "*Info TOC*"))
!                             ((not (stringp file))  ;; avoid errors
!                              (setq file nil))
!                             (t
!                              (setq file (concat "("
!                                                 (file-name-nondirectory
file)
!                                                 ") "
!                                                 Info-current-node)))))))
!                   (push (list buffer bits name (buffer-size) mode file)
!                         list))))))
!         ;; Preserve the original buffer-list ordering, just in case.
!         (setq list (nreverse list))
!         ;; Place the buffers's info in the output buffer, sorted if
necessary.
!         (dolist (buffer
!                  (if Buffer-menu-sort-column
!                      (sort list
!                            (if (eq Buffer-menu-sort-column 3)
!                                (lambda (a b)
!                                  (< (nth Buffer-menu-sort-column a)
!                                     (nth Buffer-menu-sort-column b)))
!                              (lambda (a b)
!                                (string< (nth Buffer-menu-sort-column a)
!                                         (nth Buffer-menu-sort-column
b)))))
!                    list))
!           (insert (cadr buffer)
!                   ;; Put the buffer name into a text property
!                   ;; so we don't have to extract it from the text.
!                   ;; This way we avoid problems with unusual buffer names.
!                   (let ((name (nth 2 buffer))
!                         (size (int-to-string (nth 3 buffer))))
!                     (Buffer-menu-buffer+size name size
!                                              `(buffer-name ,name
!                                                            buffer ,(car
buffer)
!                                                            font-lock-face
buffer-menu-buffer
!                                                            mouse-face
highlight
!                                                            help-echo
!                                                            ,(if (>=
(length name)
!                                                                     (-
Buffer-menu-buffer+size-width
!
(max (length size) 3)
!                                                                        2))
!                                                                 name
!                                                               "mouse-2:
select this buffer"))))
!                   "  "
!                   (if (> (string-width (nth 4 buffer))
Buffer-menu-mode-width)
!                       (truncate-string-to-width (nth 4 buffer)
!                                                 Buffer-menu-mode-width)
!                     (nth 4 buffer)))
!           (when (nth 5 buffer)
!             (indent-to (+ Buffer-menu-buffer-column
Buffer-menu-buffer+size-width
!                           Buffer-menu-mode-width 4) 1)
!             (princ (abbreviate-file-name (nth 5 buffer))))
!           (princ "\n"))
!         (Buffer-menu-mode)
!         (when Buffer-menu-use-header-line
!           (setq header-line-format header))
!         (let ((buffer-list-window (get-window-with-predicate
!                                    (lambda (window)
!                                          (string= "*Buffer List*"
!                                                   (buffer-name
(window-buffer window))))))
!               (current-window (selected-window)))
!           (when buffer-list-window
!             (select-window buffer-list-window))
!           (goto-char (point-min))
!           (forward-line (1- line-number))
!           (forward-char column-number)
!           (when buffer-list-window
!             (select-window current-window)))
!         (setq Buffer-menu-files-only files-only)
!         (setq Buffer-menu--buffers buffer-list)
!         (set-buffer-modified-p nil)
!         (current-buffer)))))

  ;;; buff-menu.el ends here

-Zachary Kanfer


On Sun, Oct 16, 2011 at 11:50 AM, Stefan Monnier
<monnier@iro.umontreal.ca>wrote:

> > !   (display-buffer (list-buffers-noselect files-only)))
> [...]
> > !   (if (string= (buffer-name)
> > !                "*Buffer List*")
> > !       (revert-buffer)
> > !     (display-buffer (list-buffers-noselect files-only))))
>
> I don't think calling revert-buffer is right.  Much better would be for
> list-buffer-noselect to try to preserve point when reusing a buffer
> rather than creating a new one.
>
>
>        Stefan
>

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

bug#9761: proposed list-buffers change -- revert buffer if point is in *Buffer List* buffer

[Lars Ingebrigtsen]

Zachary Kanfer <zkanfer@gmail.com> writes:

> The command list-buffers always puts point at the end of the
> buffer. This patch changes the list-buffers command to simply revert
> the buffer if point is inside the *Buffer List* buffer. Since point is
> kept at the same line, the user can more easily see what changes have
> occurred.

I think it makes sense to have `M-x list-buffers' always place the point
in the same place.  If the user wants to update the list, then the `g'
command is available and handy, and doesn't move the point around.  So
I'm closing this bug report.

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