bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

I noticed a certain issue with the new Buffer-menu-group-by option: when
grouping is enabled, reverting the Buffer List buffer may move point
back to the beginning on the buffer.  This is especially problematic
when auto-revert-mode is involved.  Consider:

1. emacs -Q
2. (setq global-auto-revert-non-file-buffers t)
3. M-x global-auto-revert-mode RET
4. (setq Buffer-menu-group-by '(Buffer-menu-group-by-mode))
5. M-x list-buffers
6. Navigate to some heading in the Buffer List buffer, any heading
   besides the first.
7. Wait (or think) for 5 seconds.
8. Auto-revert kicks in and point moves back to the beginning
   of the buffer.

Without grouping, reverting the Buffer List (manually or via
global-auto-revert-mode) keeps point in place.


Best,

Eshel

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

> I noticed a certain issue with the new Buffer-menu-group-by option: when
> grouping is enabled, reverting the Buffer List buffer may move point
> back to the beginning on the buffer.  This is especially problematic
> when auto-revert-mode is involved.  Consider:
>
> 1. emacs -Q
> 2. (setq global-auto-revert-non-file-buffers t)
> 3. M-x global-auto-revert-mode RET
> 4. (setq Buffer-menu-group-by '(Buffer-menu-group-by-mode))
> 5. M-x list-buffers
> 6. Navigate to some heading in the Buffer List buffer, any heading
>    besides the first.
> 7. Wait (or think) for 5 seconds.
> 8. Auto-revert kicks in and point moves back to the beginning
>    of the buffer.

Thanks for the request, this definitely should be improved.

> Without grouping, reverting the Buffer List (manually or via
> global-auto-revert-mode) keeps point in place.

When point is on an entry, then 'tabulated-list-print'
moves point to the entry with the same ID.

However, what ID to prefer for outline heading lines
is not quite clear.  Possible variants:

1. The simplest way would be to remember the position of point
   or the line number.  But this is not quite reliable
   when new entries are inserted before.

2. Remembering the outline heading line as a string and searching for it
   afterwards would be ambiguous when there are more headings with the
   same string.  For example, when at the top level there are project names,
   and at the second level mode names repeated for every project.

3. To remember a complete path like outline-hidden-headings-paths does,
   e.g. '("Project1 name" "Mode2 name").  But this will not handle modes
   that don't use tabulated-list.  For example, reverting an xref buffer
   with outlines now restores visibility of outlines, but doesn't restore point.
   OTOH, maybe it's not responsibility of outline-minor-mode to restore point
   when it's not on a heading line.

4. To remember some context before and after point, then after reverting
   search context with 'rear-context-string' and 'front-context-string'.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

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

> When point is on an entry, then 'tabulated-list-print'
> moves point to the entry with the same ID.
>
> However, what ID to prefer for outline heading lines
> is not quite clear.  Possible variants:
>
> 1. The simplest way would be to remember the position of point
>    or the line number.  But this is not quite reliable
>    when new entries are inserted before.
>
> 2. Remembering the outline heading line as a string and searching for it
>    afterwards would be ambiguous when there are more headings with the
>    same string.  For example, when at the top level there are project names,
>    and at the second level mode names repeated for every project.
>
> 3. To remember a complete path like outline-hidden-headings-paths does,
>    e.g. '("Project1 name" "Mode2 name").

The third variant is implemented now.

> But this will not handle modes that don't use tabulated-list.
> For example, reverting an xref buffer with outlines now restores
> visibility of outlines, but doesn't restore point.  OTOH, maybe it's
> not responsibility of outline-minor-mode to restore point when it's
> not on a heading line.

For xref I propose a separate patch that keeps point on the same line
after reverting the xref buffer:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-revert-buffer-restore-point.patch --]
[-- Type: text/x-diff, Size: 2304 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index fb6c9dad73b..1ff02d3712a 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1017,7 +1017,9 @@ xref--xref-buffer-mode
               (lambda (&optional bound move backward looking-at)
                 (outline-search-text-property
                  'xref-group nil bound move backward looking-at)))
-  (setq-local outline-level (lambda () 1)))
+  (setq-local outline-level (lambda () 1))
+  (add-hook 'revert-buffer-restore-functions
+            #'xref-revert-buffer-restore-point nil t))
 
 (defvar xref--transient-buffer-mode-map
   (let ((map (make-sparse-keymap)))
@@ -1282,19 +1284,19 @@ xref-revert-buffer
     (when (boundp 'revert-buffer-restore-functions)
       (run-hook-wrapped 'revert-buffer-restore-functions
                         (lambda (f) (push (funcall f) restore-functions) nil)))
-    (save-excursion
-      (condition-case err
-          (let ((alist (xref--analyze (funcall xref--fetcher)))
-                (inhibit-modification-hooks t))
-            (erase-buffer)
-            (prog1 (xref--insert-xrefs alist)
-              (mapc #'funcall (delq nil restore-functions))))
-        (user-error
-         (erase-buffer)
-         (insert
-          (propertize
-           (error-message-string err)
-           'face 'error)))))))
+    (condition-case err
+        (let ((alist (xref--analyze (funcall xref--fetcher)))
+              (inhibit-modification-hooks t))
+          (erase-buffer)
+          (prog1 (xref--insert-xrefs alist)
+            (goto-char (point-min))
+            (mapc #'funcall (delq nil restore-functions))))
+      (user-error
+       (erase-buffer)
+       (insert
+        (propertize
+         (error-message-string err)
+         'face 'error))))))
 
 (defun xref--auto-jump-first (buf value)
   (when value
@@ -1309,6 +1311,13 @@ xref--auto-jump-first
    ((eq value 'move)
     (forward-line 1))))
 
+(defun xref-revert-buffer-restore-point ()
+  (let ((current-line (buffer-substring-no-properties (pos-bol) (pos-eol))))
+    (lambda ()
+      (goto-char (point-min))
+      (when (search-forward current-line)
+        (goto-char (pos-bol))))))
+
 (defun xref-show-definitions-buffer (fetcher alist)
   "Show the definitions list in a regular window.
 

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Juri,

Juri Linkov <juri@linkov.net> writes:

>> When point is on an entry, then 'tabulated-list-print'
>> moves point to the entry with the same ID.
>>
>> However, what ID to prefer for outline heading lines
>> is not quite clear.  Possible variants:
>>
>> 1. The simplest way would be to remember the position of point
>>    or the line number.  But this is not quite reliable
>>    when new entries are inserted before.
>>
>> 2. Remembering the outline heading line as a string and searching for it
>>    afterwards would be ambiguous when there are more headings with the
>>    same string.  For example, when at the top level there are project names,
>>    and at the second level mode names repeated for every project.
>>
>> 3. To remember a complete path like outline-hidden-headings-paths does,
>>    e.g. '("Project1 name" "Mode2 name").
>
> The third variant is implemented now.

Works like a charm here, thank you!

>> But this will not handle modes that don't use tabulated-list.
>> For example, reverting an xref buffer with outlines now restores
>> visibility of outlines, but doesn't restore point.  OTOH, maybe it's
>> not responsibility of outline-minor-mode to restore point when it's
>> not on a heading line.
>
> For xref I propose a separate patch that keeps point on the same line
> after reverting the xref buffer:

LGTM, but FWIW the situation with xref-revert-buffer is not ideal IMO:
it might be cleaner to leave 'g' bound to the usual revert-buffer and
set revert-buffer-function to (a slightly modified) xref-revert-buffer.
That way xref-revert-buffer wouldn't need to duplicate generic parts of
revert-buffer, such as running revert-buffer-restore-functions.  WDYT?


Eshel

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 17/06/2024 10:40, Eshel Yaron wrote:
>>> But this will not handle modes that don't use tabulated-list.
>>> For example, reverting an xref buffer with outlines now restores
>>> visibility of outlines, but doesn't restore point.  OTOH, maybe it's
>>> not responsibility of outline-minor-mode to restore point when it's
>>> not on a heading line.
>> For xref I propose a separate patch that keeps point on the same line
>> after reverting the xref buffer:
> LGTM, but FWIW the situation with xref-revert-buffer is not ideal IMO:
> it might be cleaner to leave 'g' bound to the usual revert-buffer and
> set revert-buffer-function to (a slightly modified) xref-revert-buffer.
> That way xref-revert-buffer wouldn't need to duplicate generic parts of
> revert-buffer, such as running revert-buffer-restore-functions.  WDYT?

I'm okay with that.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 17/06/2024 09:35, Juri Linkov wrote:
> +(defun xref-revert-buffer-restore-point ()
> +  (let ((current-line (buffer-substring-no-properties (pos-bol) (pos-eol))))
> +    (lambda ()
> +      (goto-char (point-min))
> +      (when (search-forward current-line)
> +        (goto-char (pos-bol))))))

Perhaps it would make sense to save the current group as well, for 
additional precision.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 17/06/2024 10:40, Eshel Yaron wrote:
>>>> But this will not handle modes that don't use tabulated-list.
>>>> For example, reverting an xref buffer with outlines now restores
>>>> visibility of outlines, but doesn't restore point.  OTOH, maybe it's
>>>> not responsibility of outline-minor-mode to restore point when it's
>>>> not on a heading line.
>>> For xref I propose a separate patch that keeps point on the same line
>>> after reverting the xref buffer:
>> LGTM, but FWIW the situation with xref-revert-buffer is not ideal IMO:
>> it might be cleaner to leave 'g' bound to the usual revert-buffer and
>> set revert-buffer-function to (a slightly modified) xref-revert-buffer.
>> That way xref-revert-buffer wouldn't need to duplicate generic parts of
>> revert-buffer, such as running revert-buffer-restore-functions.  WDYT?
>
> I'm okay with that.

Here's a concrete proposal:

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index fb6c9dad73b..9878806c0de 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -993,7 +993,6 @@ xref--xref-buffer-mode-map
     ;; suggested by Johan Claesson "to further reduce finger movement":
     (define-key map (kbd ".") #'xref-next-line)
     (define-key map (kbd ",") #'xref-prev-line)
-    (define-key map (kbd "g") #'xref-revert-buffer)
     (define-key map (kbd "M-,") #'xref-quit-and-pop-marker-stack)
     map))
 
@@ -1011,6 +1010,7 @@ xref--xref-buffer-mode
         #'xref--imenu-extract-index-name)
   (setq-local add-log-current-defun-function
               #'xref--add-log-current-defun)
+  (setq-local revert-buffer-function #'xref-revert-buffer)
   (setq-local outline-minor-mode-cycle t)
   (setq-local outline-minor-mode-use-buttons 'insert)
   (setq-local outline-search-function
@@ -1273,22 +1273,17 @@ xref--show-common-initialize
           xref--original-window-intent (assoc-default 'display-action alist))
     (setq xref--fetcher fetcher)))
 
-(defun xref-revert-buffer ()
+(defun xref-revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
   "Refresh the search results in the current buffer."
   (interactive)
   (let ((inhibit-read-only t)
-        (buffer-undo-list t)
-        restore-functions)
-    (when (boundp 'revert-buffer-restore-functions)
-      (run-hook-wrapped 'revert-buffer-restore-functions
-                        (lambda (f) (push (funcall f) restore-functions) nil)))
+        (buffer-undo-list t))
     (save-excursion
       (condition-case err
           (let ((alist (xref--analyze (funcall xref--fetcher)))
                 (inhibit-modification-hooks t))
             (erase-buffer)
-            (prog1 (xref--insert-xrefs alist)
-              (mapc #'funcall (delq nil restore-functions))))
+            (xref--insert-xrefs alist))
         (user-error
          (erase-buffer)
          (insert

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

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

>> +(defun xref-revert-buffer-restore-point ()
>> +  (let ((current-line (buffer-substring-no-properties (pos-bol) (pos-eol))))
>> +    (lambda ()
>> +      (goto-char (point-min))
>> +      (when (search-forward current-line)
>> +        (goto-char (pos-bol))))))
>
> Perhaps it would make sense to save the current group as well, for
> additional precision.

Ok, so this is implemented here:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-revert-buffer-restore-point_2.patch --]
[-- Type: text/x-diff, Size: 1645 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index fb6c9dad73b..987571b92c5 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1017,7 +1017,9 @@ xref--xref-buffer-mode
               (lambda (&optional bound move backward looking-at)
                 (outline-search-text-property
                  'xref-group nil bound move backward looking-at)))
-  (setq-local outline-level (lambda () 1)))
+  (setq-local outline-level (lambda () 1))
+  (add-hook 'revert-buffer-restore-functions
+            #'xref-revert-buffer-restore-point nil t))
 
 (defvar xref--transient-buffer-mode-map
   (let ((map (make-sparse-keymap)))
@@ -1309,6 +1311,24 @@ xref--auto-jump-first
    ((eq value 'move)
     (forward-line 1))))
 
+(defun xref-revert-buffer-restore-point ()
+  (let* ((item
+          (when (xref--item-at-point)
+            (buffer-substring-no-properties (pos-bol) (pos-eol))))
+         (group
+          (save-excursion
+            (when (or (get-text-property (point) 'xref-group)
+                      (and item (xref--search-property 'xref-group t)
+                           (get-text-property (point) 'xref-group)))
+              (buffer-substring-no-properties (pos-bol) (pos-eol))))))
+    (when (or item group)
+      (lambda ()
+        (goto-char (point-min))
+        (when (and group (search-forward (concat "\n" group "\n") nil t))
+          (goto-char (pos-bol 0)))
+        (when (and item (search-forward (concat "\n" item "\n") nil t))
+          (goto-char (pos-bol 0)))))))
+
 (defun xref-show-definitions-buffer (fetcher alist)
   "Show the definitions list in a regular window.
 

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 17/06/2024 19:31, Juri Linkov wrote:
> Ok, so this is implemented here:

LGTM, thanks!

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 17/06/2024 18:43, Eshel Yaron wrote:
> Dmitry Gutov <dmitry@gutov.dev> writes:
> 
>> On 17/06/2024 10:40, Eshel Yaron wrote:
>>>>> But this will not handle modes that don't use tabulated-list.
>>>>> For example, reverting an xref buffer with outlines now restores
>>>>> visibility of outlines, but doesn't restore point.  OTOH, maybe it's
>>>>> not responsibility of outline-minor-mode to restore point when it's
>>>>> not on a heading line.
>>>> For xref I propose a separate patch that keeps point on the same line
>>>> after reverting the xref buffer:
>>> LGTM, but FWIW the situation with xref-revert-buffer is not ideal IMO:
>>> it might be cleaner to leave 'g' bound to the usual revert-buffer and
>>> set revert-buffer-function to (a slightly modified) xref-revert-buffer.
>>> That way xref-revert-buffer wouldn't need to duplicate generic parts of
>>> revert-buffer, such as running revert-buffer-restore-functions.  WDYT?
>>
>> I'm okay with that.
> 
> Here's a concrete proposal:

Thanks, works for me, with some caveats.

> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index fb6c9dad73b..9878806c0de 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -993,7 +993,6 @@ xref--xref-buffer-mode-map
>       ;; suggested by Johan Claesson "to further reduce finger movement":
>       (define-key map (kbd ".") #'xref-next-line)
>       (define-key map (kbd ",") #'xref-prev-line)
> -    (define-key map (kbd "g") #'xref-revert-buffer)
>       (define-key map (kbd "M-,") #'xref-quit-and-pop-marker-stack)
>       map))
>   
> @@ -1011,6 +1010,7 @@ xref--xref-buffer-mode
>           #'xref--imenu-extract-index-name)
>     (setq-local add-log-current-defun-function
>                 #'xref--add-log-current-defun)
> +  (setq-local revert-buffer-function #'xref-revert-buffer)
>     (setq-local outline-minor-mode-cycle t)
>     (setq-local outline-minor-mode-use-buttons 'insert)
>     (setq-local outline-search-function
> @@ -1273,22 +1273,17 @@ xref--show-common-initialize
>             xref--original-window-intent (assoc-default 'display-action alist))
>       (setq xref--fetcher fetcher)))
>   
> -(defun xref-revert-buffer ()
> +(defun xref-revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
>     "Refresh the search results in the current buffer."
>     (interactive)

It seems like the interactive use of 'xref-revert-buffer' would suffer 
with this change (it wouldn't call the restore functions). And it might 
be used where people rebind it to a different key in their configs.

Perhaps instead xref-revert-buffer should become an obsolete alias to 
'revert-buffer'. And the current definition would be renamed to 
xref--revert-buffer (which we'll set revert-buffer-function to).

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 17/06/2024 18:43, Eshel Yaron wrote:
>> Dmitry Gutov <dmitry@gutov.dev> writes:
>> 
>>> On 17/06/2024 10:40, Eshel Yaron wrote:
>>>>>> But this will not handle modes that don't use tabulated-list.
>>>>>> For example, reverting an xref buffer with outlines now restores
>>>>>> visibility of outlines, but doesn't restore point.  OTOH, maybe it's
>>>>>> not responsibility of outline-minor-mode to restore point when it's
>>>>>> not on a heading line.
>>>>> For xref I propose a separate patch that keeps point on the same line
>>>>> after reverting the xref buffer:
>>>> LGTM, but FWIW the situation with xref-revert-buffer is not ideal IMO:
>>>> it might be cleaner to leave 'g' bound to the usual revert-buffer and
>>>> set revert-buffer-function to (a slightly modified) xref-revert-buffer.
>>>> That way xref-revert-buffer wouldn't need to duplicate generic parts of
>>>> revert-buffer, such as running revert-buffer-restore-functions.  WDYT?
>>>
>>> I'm okay with that.
>> Here's a concrete proposal:
>
> Thanks, works for me, with some caveats.
>
>> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
>> [...]
>
> It seems like the interactive use of 'xref-revert-buffer' would suffer
> with this change (it wouldn't call the restore functions). And it
> might be used where people rebind it to a different key in their
> configs.

Right, good point.

> Perhaps instead xref-revert-buffer should become an obsolete alias to
> 'revert-buffer'. And the current definition would be renamed to
> xref--revert-buffer (which we'll set revert-buffer-function to).

SGTM, here's an updated diff, also with a NEWS entry and doc updates:

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 3a9bef9884a..3e5f3831260 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -2467,9 +2467,8 @@ Xref Commands
 all the relevant files.  @xref{Identifier Search}.
 
 @item g
-@findex xref-revert-buffer
-Refresh the contents of the @file{*xref*} buffer
-(@code{xref-revert-buffer}).
+Refresh the contents of the @file{*xref*} buffer (@code{revert-buffer}).
+@xref{Reverting}.
 
 @item M-,
 @findex xref-quit-and-pop-marker-stack
diff --git a/etc/NEWS b/etc/NEWS
index b2fdbc4a88f..b5dcd87e005 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1895,6 +1895,13 @@ options of GNU 'ls'.
 If non-nil, moving point forward or backward between widgets by typing
 'TAB' or 'S-TAB' skips over inactive widgets.  The default value is nil.
 
+** Xref
+
+*** 'xref-revert-buffer' is obsolete, prefer 'revert-buffer' instead.
+The former is now an alias of the latter.  The Xref results buffer sets
+up 'revert-buffer-function' such that 'revert-buffer' behaves like
+'xref-revert-buffer' did in previous Emacs versions.
+
 ** Ruby mode
 
 *** New user option 'ruby-rubocop-use-bundler'.
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index fb6c9dad73b..37f5220cec3 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -993,7 +993,6 @@ xref--xref-buffer-mode-map
     ;; suggested by Johan Claesson "to further reduce finger movement":
     (define-key map (kbd ".") #'xref-next-line)
     (define-key map (kbd ",") #'xref-prev-line)
-    (define-key map (kbd "g") #'xref-revert-buffer)
     (define-key map (kbd "M-,") #'xref-quit-and-pop-marker-stack)
     map))
 
@@ -1011,6 +1010,7 @@ xref--xref-buffer-mode
         #'xref--imenu-extract-index-name)
   (setq-local add-log-current-defun-function
               #'xref--add-log-current-defun)
+  (setq-local revert-buffer-function #'xref--revert-buffer)
   (setq-local outline-minor-mode-cycle t)
   (setq-local outline-minor-mode-use-buttons 'insert)
   (setq-local outline-search-function
@@ -1273,22 +1273,16 @@ xref--show-common-initialize
           xref--original-window-intent (assoc-default 'display-action alist))
     (setq xref--fetcher fetcher)))
 
-(defun xref-revert-buffer ()
+(defun xref--revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
   "Refresh the search results in the current buffer."
-  (interactive)
   (let ((inhibit-read-only t)
-        (buffer-undo-list t)
-        restore-functions)
-    (when (boundp 'revert-buffer-restore-functions)
-      (run-hook-wrapped 'revert-buffer-restore-functions
-                        (lambda (f) (push (funcall f) restore-functions) nil)))
+        (buffer-undo-list t))
     (save-excursion
       (condition-case err
           (let ((alist (xref--analyze (funcall xref--fetcher)))
                 (inhibit-modification-hooks t))
             (erase-buffer)
-            (prog1 (xref--insert-xrefs alist)
-              (mapc #'funcall (delq nil restore-functions))))
+            (xref--insert-xrefs alist))
         (user-error
          (erase-buffer)
          (insert
@@ -1296,6 +1290,8 @@ xref-revert-buffer
            (error-message-string err)
            'face 'error)))))))
 
+(define-obsolete-function-alias 'xref-revert-buffer #'revert-buffer "30.1")
+
 (defun xref--auto-jump-first (buf value)
   (when value
     (select-window (get-buffer-window buf))

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> Cc: 71466@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
> Date: Tue, 18 Jun 2024 09:00:38 +0200
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
>  @item g
> -@findex xref-revert-buffer
> -Refresh the contents of the @file{*xref*} buffer
> -(@code{xref-revert-buffer}).
> +Refresh the contents of the @file{*xref*} buffer (@code{revert-buffer}).
> +@xref{Reverting}.

Why remove the index entry?  It needs to be rewritten, not removed.

> +** Xref
> +
> +*** 'xref-revert-buffer' is obsolete, prefer 'revert-buffer' instead.
> +The former is now an alias of the latter.  The Xref results buffer sets

Please use "The Xref buffer".  "The Xref results buffer" reads
awkwardly, and there actually is no such thing as "Xref results".

> @@ -1011,6 +1010,7 @@ xref--xref-buffer-mode
>          #'xref--imenu-extract-index-name)
>    (setq-local add-log-current-defun-function
>                #'xref--add-log-current-defun)
> +  (setq-local revert-buffer-function #'xref--revert-buffer)
>    (setq-local outline-minor-mode-cycle t)
>    (setq-local outline-minor-mode-use-buttons 'insert)
>    (setq-local outline-search-function
> @@ -1273,22 +1273,16 @@ xref--show-common-initialize
>            xref--original-window-intent (assoc-default 'display-action alist))
>      (setq xref--fetcher fetcher)))
>  
> -(defun xref-revert-buffer ()
> +(defun xref--revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
>    "Refresh the search results in the current buffer."

And I wonder why you preferred a backward-incompatible change to a
backward-compatible one: leave the function's name alone, and just set
up revert-buffer-function to invoke it.  Was this not possible for
some technical reason that evades me?

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 71466@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
>> Date: Tue, 18 Jun 2024 09:00:38 +0200
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>>  @item g
>> -@findex xref-revert-buffer
>> -Refresh the contents of the @file{*xref*} buffer
>> -(@code{xref-revert-buffer}).
>> +Refresh the contents of the @file{*xref*} buffer (@code{revert-buffer}).
>> +@xref{Reverting}.
>
> Why remove the index entry?  It needs to be rewritten, not removed.

The index entry is for xref-revert-buffer, which we're making obsolete
here in favor of revert-buffer, which has its own index entry elsewhere.
How do you suggest rewriting it instead?

>> +** Xref
>> +
>> +*** 'xref-revert-buffer' is obsolete, prefer 'revert-buffer' instead.
>> +The former is now an alias of the latter.  The Xref results buffer sets
>
> Please use "The Xref buffer".  "The Xref results buffer" reads
> awkwardly, and there actually is no such thing as "Xref results".

All right, will do.

>> @@ -1011,6 +1010,7 @@ xref--xref-buffer-mode
>>          #'xref--imenu-extract-index-name)
>>    (setq-local add-log-current-defun-function
>>                #'xref--add-log-current-defun)
>> +  (setq-local revert-buffer-function #'xref--revert-buffer)
>>    (setq-local outline-minor-mode-cycle t)
>>    (setq-local outline-minor-mode-use-buttons 'insert)
>>    (setq-local outline-search-function
>> @@ -1273,22 +1273,16 @@ xref--show-common-initialize
>>            xref--original-window-intent (assoc-default 'display-action alist))
>>      (setq xref--fetcher fetcher)))
>>  
>> -(defun xref-revert-buffer ()
>> +(defun xref--revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
>>    "Refresh the search results in the current buffer."
>
> And I wonder why you preferred a backward-incompatible change to a
> backward-compatible one:

This is intended to be (basically) fully backward-compatible:
xref-revert-buffer becomes an alias of revert-buffer, which does exactly
what xref-revert-buffer would do.

> leave the function's name alone, and just set up
> revert-buffer-function to invoke it.  Was this not possible for some
> technical reason that evades me?

It's possible, and it's more or less what I suggested upthread, but
Dmitry correctly noted that this approach (using xref--revert-buffer)
improves backward-compatibility in the following sense: users that
currently invoke xref-revert-buffer not by pressing 'g', but in some
other way, can continue to do so and get the same behavior that now
revert-buffer provides when you press 'g'.  Since revert-buffer does
more than just calling revert-buffer-function (namely, it also runs
revert-buffer-restore-functions), making xref-revert-buffer an alias of
revert-buffer ensures invoking and xref-revert-buffer and pressing 'g'
continues to behave the same.


Eshel

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: dmitry@gutov.dev,  71466@debbugs.gnu.org,  juri@linkov.net
> Date: Tue, 18 Jun 2024 16:01:40 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >>  @item g
> >> -@findex xref-revert-buffer
> >> -Refresh the contents of the @file{*xref*} buffer
> >> -(@code{xref-revert-buffer}).
> >> +Refresh the contents of the @file{*xref*} buffer (@code{revert-buffer}).
> >> +@xref{Reverting}.
> >
> > Why remove the index entry?  It needs to be rewritten, not removed.
> 
> The index entry is for xref-revert-buffer, which we're making obsolete
> here in favor of revert-buffer, which has its own index entry elsewhere.
> How do you suggest rewriting it instead?

Like this:

  @cindex revert-buffer, in @file{*xref*} buffers

And please move it before "@item g", so that following the index entry
with 'i' in Info lands on the line showing `g', not the line after it.

> >> -(defun xref-revert-buffer ()
> >> +(defun xref--revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
> >>    "Refresh the search results in the current buffer."
> >
> > And I wonder why you preferred a backward-incompatible change to a
> > backward-compatible one:
> 
> This is intended to be (basically) fully backward-compatible:
> xref-revert-buffer becomes an alias of revert-buffer, which does exactly
> what xref-revert-buffer would do.

Yes, but why not leave xref-revert-buffer alone, under its original
name?

> > leave the function's name alone, and just set up
> > revert-buffer-function to invoke it.  Was this not possible for some
> > technical reason that evades me?
> 
> It's possible, and it's more or less what I suggested upthread, but
> Dmitry correctly noted that this approach (using xref--revert-buffer)
> improves backward-compatibility in the following sense: users that
> currently invoke xref-revert-buffer not by pressing 'g', but in some
> other way, can continue to do so and get the same behavior that now
> revert-buffer provides when you press 'g'.  Since revert-buffer does
> more than just calling revert-buffer-function (namely, it also runs
> revert-buffer-restore-functions), making xref-revert-buffer an alias of
> revert-buffer ensures invoking and xref-revert-buffer and pressing 'g'
> continues to behave the same.

But the original xref-revert-buffer didn't do all those other things,
did it?  So invoking it directly will be more similar to what that did
before.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: dmitry@gutov.dev,  71466@debbugs.gnu.org,  juri@linkov.net
>> Date: Tue, 18 Jun 2024 16:01:40 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >>  @item g
>> >> -@findex xref-revert-buffer
>> >> -Refresh the contents of the @file{*xref*} buffer
>> >> -(@code{xref-revert-buffer}).
>> >> +Refresh the contents of the @file{*xref*} buffer (@code{revert-buffer}).
>> >> +@xref{Reverting}.
>> >
>> > Why remove the index entry?  It needs to be rewritten, not removed.
>> 
>> The index entry is for xref-revert-buffer, which we're making obsolete
>> here in favor of revert-buffer, which has its own index entry elsewhere.
>> How do you suggest rewriting it instead?
>
> Like this:
>
>   @cindex revert-buffer, in @file{*xref*} buffers
>
> And please move it before "@item g", so that following the index entry
> with 'i' in Info lands on the line showing `g', not the line after it.

OK, thanks.

>> >> -(defun xref-revert-buffer ()
>> >> +(defun xref--revert-buffer (&rest _)     ; Ignore `revert-buffer' args.
>> >>    "Refresh the search results in the current buffer."
>> >
>> > And I wonder why you preferred a backward-incompatible change to a
>> > backward-compatible one:
>> 
>> This is intended to be (basically) fully backward-compatible:
>> xref-revert-buffer becomes an alias of revert-buffer, which does exactly
>> what xref-revert-buffer would do.
>
> Yes, but why not leave xref-revert-buffer alone, under its original
> name?

See below.

>> > leave the function's name alone, and just set up
>> > revert-buffer-function to invoke it.  Was this not possible for some
>> > technical reason that evades me?
>> 
>> It's possible, and it's more or less what I suggested upthread, but
>> Dmitry correctly noted that this approach (using xref--revert-buffer)
>> improves backward-compatibility in the following sense: users that
>> currently invoke xref-revert-buffer not by pressing 'g', but in some
>> other way, can continue to do so and get the same behavior that now
>> revert-buffer provides when you press 'g'.  Since revert-buffer does
>> more than just calling revert-buffer-function (namely, it also runs
>> revert-buffer-restore-functions), making xref-revert-buffer an alias of
>> revert-buffer ensures invoking and xref-revert-buffer and pressing 'g'
>> continues to behave the same.
>
> But the original xref-revert-buffer didn't do all those other things,
> did it?  So invoking it directly will be more similar to what that did
> before.

That's right, but revert-buffer-restore-functions makes revert-buffer
more correct because it also restores outline-minor-mode state.  So
making xref-revert-buffer an alias of revert-buffer brings this benefit
to users that invoke xref-revert-buffer directly, without duplicating
any revert-buffer code in xref-revert-buffer.  Anyway, I trust you and
Dmitry to decide on the preferred solution, let me know I'll and post a
final patch accordingly.


Eshel

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 18/06/2024 17:46, Eli Zaretskii wrote:
> But the original xref-revert-buffer didn't do all those other things,
> did it?  So invoking it directly will be more similar to what that did
> before.

The current xref-revert-buffer already does those things - namely invoke 
revert-buffer-restore-functions. Even though it's a relatively recent 
phenomenon.

And it makes sense to provide this feature to the users of 
xref-revert-buffer, so I don't see why not.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> Date: Tue, 18 Jun 2024 20:05:14 +0300
> Cc: 71466@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 18/06/2024 17:46, Eli Zaretskii wrote:
> > But the original xref-revert-buffer didn't do all those other things,
> > did it?  So invoking it directly will be more similar to what that did
> > before.
> 
> The current xref-revert-buffer already does those things - namely invoke 
> revert-buffer-restore-functions. Even though it's a relatively recent 
> phenomenon.
> 
> And it makes sense to provide this feature to the users of 
> xref-revert-buffer, so I don't see why not.

All things being equal, leaving the old names of the functions and
commands should be preferred.  That's all I'm saying.  It's a minor
issue, but the cost of staying more compatible is also minor.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 18/06/2024 20:36, Eli Zaretskii wrote:
>> And it makes sense to provide this feature to the users of
>> xref-revert-buffer, so I don't see why not.
> All things being equal, leaving the old names of the functions and
> commands should be preferred.  That's all I'm saying.  It's a minor
> issue, but the cost of staying more compatible is also minor.

It might be important for the case when somebody has rebound 'g' in Xref 
buffers to some other letter - then they reference xref-revert-buffer by 
name in their init script.

Having them end up with a subpar version of it (and perhaps not notice 
it either) feels like an unfortunate outcome.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

>>> And it makes sense to provide this feature to the users of
>>> xref-revert-buffer, so I don't see why not.
>> All things being equal, leaving the old names of the functions and
>> commands should be preferred.  That's all I'm saying.  It's a minor
>> issue, but the cost of staying more compatible is also minor.
>
> It might be important for the case when somebody has rebound 'g' in Xref
> buffers to some other letter - then they reference xref-revert-buffer by
> name in their init script.
>
> Having them end up with a subpar version of it (and perhaps not notice it
> either) feels like an unfortunate outcome.

So what is the decision?  Maybe better to keep xref-revert-buffer
as a wrapper around the new function xref--revert-buffer?

(defun xref-revert-buffer ()
  (let (restore-functions)
    (when (boundp 'revert-buffer-restore-functions)
      (run-hook-wrapped 'revert-buffer-restore-functions
                        (lambda (f) (push (funcall f) restore-functions) nil)))
    (prog1 (xref--revert-buffer)
      (mapc #'funcall (delq nil restore-functions)))))

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 20/06/2024 19:38, Juri Linkov wrote:
> So what is the decision?  Maybe better to keep xref-revert-buffer
> as a wrapper around the new function xref--revert-buffer?

That should work, though at the expense of duplicating some code.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

>> So what is the decision?  Maybe better to keep xref-revert-buffer
>> as a wrapper around the new function xref--revert-buffer?
>
> That should work, though at the expense of duplicating some code.

I still don't understand how duplicating revert-buffer

+(defun xref-revert-buffer ()
+  "Refresh the search results in the current buffer."
+  (declare (obsolete revert-buffer "30.1"))
+  (interactive)
+  (let (restore-functions)
+    (when (boundp 'revert-buffer-restore-functions)
+      (run-hook-wrapped 'revert-buffer-restore-functions
+                        (lambda (f) (push (funcall f) restore-functions) nil)))
+    (prog1 (xref--revert-buffer)
+      (mapc #'funcall (delq nil restore-functions)))))

can be better than what Eshel proposed with an alias:

+ (define-obsolete-function-alias 'xref-revert-buffer #'revert-buffer "30.1")

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 24/06/2024 09:27, Juri Linkov wrote:
> I still don't understand how duplicating revert-buffer
> 
> +(defun xref-revert-buffer ()
> +  "Refresh the search results in the current buffer."
> +  (declare (obsolete revert-buffer "30.1"))
> +  (interactive)
> +  (let (restore-functions)
> +    (when (boundp 'revert-buffer-restore-functions)
> +      (run-hook-wrapped 'revert-buffer-restore-functions
> +                        (lambda (f) (push (funcall f) restore-functions) nil)))
> +    (prog1 (xref--revert-buffer)
> +      (mapc #'funcall (delq nil restore-functions)))))
> 
> can be better than what Eshel proposed with an alias:
> 
> + (define-obsolete-function-alias 'xref-revert-buffer #'revert-buffer "30.1")

That is my opinion as well: better obsolete it this way.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

>> I still don't understand how duplicating revert-buffer
>> +(defun xref-revert-buffer ()
>> +  "Refresh the search results in the current buffer."
>> +  (declare (obsolete revert-buffer "30.1"))
>> +  (interactive)
>> +  (let (restore-functions)
>> +    (when (boundp 'revert-buffer-restore-functions)
>> +      (run-hook-wrapped 'revert-buffer-restore-functions
>> +                        (lambda (f) (push (funcall f) restore-functions) nil)))
>> +    (prog1 (xref--revert-buffer)
>> +      (mapc #'funcall (delq nil restore-functions)))))
>> can be better than what Eshel proposed with an alias:
>> + (define-obsolete-function-alias 'xref-revert-buffer #'revert-buffer
>> "30.1")
>
> That is my opinion as well: better obsolete it this way.

Nice, then Eshel could push the latest patch (with discussed amendments).

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> Date: Tue, 25 Jun 2024 01:42:46 +0300
> Cc: Eli Zaretskii <eliz@gnu.org>, me@eshelyaron.com, 71466@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 24/06/2024 09:27, Juri Linkov wrote:
> > I still don't understand how duplicating revert-buffer
> > 
> > +(defun xref-revert-buffer ()
> > +  "Refresh the search results in the current buffer."
> > +  (declare (obsolete revert-buffer "30.1"))
> > +  (interactive)
> > +  (let (restore-functions)
> > +    (when (boundp 'revert-buffer-restore-functions)
> > +      (run-hook-wrapped 'revert-buffer-restore-functions
> > +                        (lambda (f) (push (funcall f) restore-functions) nil)))
> > +    (prog1 (xref--revert-buffer)
> > +      (mapc #'funcall (delq nil restore-functions)))))
> > 
> > can be better than what Eshel proposed with an alias:
> > 
> > + (define-obsolete-function-alias 'xref-revert-buffer #'revert-buffer "30.1")
> 
> That is my opinion as well: better obsolete it this way.

Why obsolete it at all?  If we use an alias without obsoleting, I
think everyone wins.  No?

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 25/06/2024 15:54, Eli Zaretskii wrote:
>> Date: Tue, 25 Jun 2024 01:42:46 +0300
>> Cc: Eli Zaretskii<eliz@gnu.org>,me@eshelyaron.com,71466@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> On 24/06/2024 09:27, Juri Linkov wrote:
>>> I still don't understand how duplicating revert-buffer
>>>
>>> +(defun xref-revert-buffer ()
>>> +  "Refresh the search results in the current buffer."
>>> +  (declare (obsolete revert-buffer "30.1"))
>>> +  (interactive)
>>> +  (let (restore-functions)
>>> +    (when (boundp 'revert-buffer-restore-functions)
>>> +      (run-hook-wrapped 'revert-buffer-restore-functions
>>> +                        (lambda (f) (push (funcall f) restore-functions) nil)))
>>> +    (prog1 (xref--revert-buffer)
>>> +      (mapc #'funcall (delq nil restore-functions)))))
>>>
>>> can be better than what Eshel proposed with an alias:
>>>
>>> + (define-obsolete-function-alias 'xref-revert-buffer #'revert-buffer "30.1")
>> That is my opinion as well: better obsolete it this way.
> Why obsolete it at all?  If we use an alias without obsoleting, I
> think everyone wins.  No?

Well, we normally obsolete functions that aren't in use anymore, nor 
recommended for third parties. Right?

We can stop from obsoleting it now (just make an alias), but add a 
comment to do that in the next Emacs release. How about that?

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> Date: Wed, 26 Jun 2024 02:14:45 +0300
> Cc: juri@linkov.net, me@eshelyaron.com, 71466@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> >> That is my opinion as well: better obsolete it this way.
> > Why obsolete it at all?  If we use an alias without obsoleting, I
> > think everyone wins.  No?
> 
> Well, we normally obsolete functions that aren't in use anymore, nor 
> recommended for third parties. Right?
> 
> We can stop from obsoleting it now (just make an alias), but add a 
> comment to do that in the next Emacs release. How about that?

Better.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Wed, 26 Jun 2024 02:14:45 +0300
>> Cc: juri@linkov.net, me@eshelyaron.com, 71466@debbugs.gnu.org
>> From: Dmitry Gutov <dmitry@gutov.dev>
>> 
>> >> That is my opinion as well: better obsolete it this way.
>> > Why obsolete it at all?  If we use an alias without obsoleting, I
>> > think everyone wins.  No?
>> 
>> Well, we normally obsolete functions that aren't in use anymore, nor 
>> recommended for third parties. Right?
>> 
>> We can stop from obsoleting it now (just make an alias), but add a 
>> comment to do that in the next Emacs release. How about that?
>
> Better.

All right, so here's an updated patch.  This is based on emacs-30, let
me know if this should go to master instead.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-revert-function-in-xref-buffer.patch --]
[-- Type: text/x-patch, Size: 4331 bytes --]

From 0b6756fd31b071a2c88f4731d4dc3a74fcfac5f8 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Wed, 26 Jun 2024 18:51:32 +0200
Subject: [PATCH] Use 'revert-function' in *xref* buffer

* lisp/progmodes/xref.el (xref--xref-buffer-mode-map): Cease
binding 'g' to 'xref-revert-buffer'.
(xref--xref-buffer-mode): Set 'revert-buffer-function' to...
(xref--revert-buffer): ...this.  New function, renamed from...
(xref-revert-buffer): ...this.  Make it an alias of
'revert-buffer'.
* etc/NEWS: Announce it.
* doc/emacs/maintaining.texi (Xref Commands): Update docs.
---
 doc/emacs/maintaining.texi |  6 +++---
 etc/NEWS                   |  7 +++++++
 lisp/progmodes/xref.el     | 17 +++++++----------
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 3a9bef9884a..64d77fb88a5 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -2466,10 +2466,10 @@ Xref Commands
 @file{*xref*} buffers that show all the matches for an identifier in
 all the relevant files.  @xref{Identifier Search}.
 
+@cindex revert-buffer, in @file{*xref*} buffers
 @item g
-@findex xref-revert-buffer
-Refresh the contents of the @file{*xref*} buffer
-(@code{xref-revert-buffer}).
+Refresh the contents of the @file{*xref*} buffer (@code{revert-buffer}).
+@xref{Reverting}.
 
 @item M-,
 @findex xref-quit-and-pop-marker-stack
diff --git a/etc/NEWS b/etc/NEWS
index 716eb612720..8d360063f7a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1916,6 +1916,13 @@ the 'widget-inactive' face).
 If non-nil, moving point forward or backward between widgets by typing
 'TAB' or 'S-TAB' skips over inactive widgets.  The default value is nil.
 
+** Xref
+
+*** 'xref-revert-buffer' is now an alias of 'revert-buffer'.
+The Xref buffer now sets up 'revert-buffer-function' such that
+'revert-buffer' behaves like 'xref-revert-buffer' did in previous Emacs
+versions, and the latter is now an alias of the former.
+
 ** Ruby mode
 
 *** New user option 'ruby-rubocop-use-bundler'.
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index fb6c9dad73b..0b069941438 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -993,7 +993,6 @@ xref--xref-buffer-mode-map
     ;; suggested by Johan Claesson "to further reduce finger movement":
     (define-key map (kbd ".") #'xref-next-line)
     (define-key map (kbd ",") #'xref-prev-line)
-    (define-key map (kbd "g") #'xref-revert-buffer)
     (define-key map (kbd "M-,") #'xref-quit-and-pop-marker-stack)
     map))
 
@@ -1011,6 +1010,7 @@ xref--xref-buffer-mode
         #'xref--imenu-extract-index-name)
   (setq-local add-log-current-defun-function
               #'xref--add-log-current-defun)
+  (setq-local revert-buffer-function #'xref--revert-buffer)
   (setq-local outline-minor-mode-cycle t)
   (setq-local outline-minor-mode-use-buttons 'insert)
   (setq-local outline-search-function
@@ -1273,22 +1273,16 @@ xref--show-common-initialize
           xref--original-window-intent (assoc-default 'display-action alist))
     (setq xref--fetcher fetcher)))
 
-(defun xref-revert-buffer ()
+(defun xref--revert-buffer (&rest _)    ; Ignore `revert-buffer' args.
   "Refresh the search results in the current buffer."
-  (interactive)
   (let ((inhibit-read-only t)
-        (buffer-undo-list t)
-        restore-functions)
-    (when (boundp 'revert-buffer-restore-functions)
-      (run-hook-wrapped 'revert-buffer-restore-functions
-                        (lambda (f) (push (funcall f) restore-functions) nil)))
+        (buffer-undo-list t))
     (save-excursion
       (condition-case err
           (let ((alist (xref--analyze (funcall xref--fetcher)))
                 (inhibit-modification-hooks t))
             (erase-buffer)
-            (prog1 (xref--insert-xrefs alist)
-              (mapc #'funcall (delq nil restore-functions))))
+            (xref--insert-xrefs alist))
         (user-error
          (erase-buffer)
          (insert
@@ -1296,6 +1290,9 @@ xref-revert-buffer
            (error-message-string err)
            'face 'error)))))))
 
+;;; FIXME: Make this alias obsolete in future release.
+(defalias 'xref-revert-buffer #'revert-buffer)
+
 (defun xref--auto-jump-first (buf value)
   (when value
     (select-window (get-buffer-window buf))
-- 
2.45.0

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: Dmitry Gutov <dmitry@gutov.dev>,  juri@linkov.net,  71466@debbugs.gnu.org
> Date: Wed, 26 Jun 2024 18:56:53 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Wed, 26 Jun 2024 02:14:45 +0300
> >> Cc: juri@linkov.net, me@eshelyaron.com, 71466@debbugs.gnu.org
> >> From: Dmitry Gutov <dmitry@gutov.dev>
> >> 
> >> >> That is my opinion as well: better obsolete it this way.
> >> > Why obsolete it at all?  If we use an alias without obsoleting, I
> >> > think everyone wins.  No?
> >> 
> >> Well, we normally obsolete functions that aren't in use anymore, nor 
> >> recommended for third parties. Right?
> >> 
> >> We can stop from obsoleting it now (just make an alias), but add a 
> >> comment to do that in the next Emacs release. How about that?
> >
> > Better.
> 
> All right, so here's an updated patch.  This is based on emacs-30, let
> me know if this should go to master instead.

I think emacs-30 is fine for this, thanks.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

close 71466 30.1
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> 
>> All right, so here's an updated patch.  This is based on emacs-30, let
>> me know if this should go to master instead.
>
> I think emacs-30 is fine for this, thanks.

Dmitry Gutov <dmitry@gutov.dev> writes:

> LGTM, please install. Thanks!

Done in commit 82125b1a661, and closing the bug.

Thank you all,

Eshel

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Dmitry Gutov]

On 26/06/2024 19:56, Eshel Yaron wrote:
> All right, so here's an updated patch.  This is based on emacs-30, let
> me know if this should go to master instead.

LGTM, please install. Thanks!

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

>>> All right, so here's an updated patch.  This is based on emacs-30, let
>>> me know if this should go to master instead.
>>
>> I think emacs-30 is fine for this, thanks.
>
>> LGTM, please install. Thanks!
>
> Done in commit 82125b1a661, and closing the bug.
>
> Thank you all,

This "FIXME: Make this alias obsolete in future release"
means that after syncing the commit from emacs-30 to master,
it could be declared obsolete in master immediately?

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Juri Linkov]

>> Ok, so this is implemented here:
>
> LGTM, thanks!

So now this is pushed as well.

bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  dmitry@gutov.dev,  71466@debbugs.gnu.org
> Date: Thu, 27 Jun 2024 09:39:36 +0300
> 
> >>> All right, so here's an updated patch.  This is based on emacs-30, let
> >>> me know if this should go to master instead.
> >>
> >> I think emacs-30 is fine for this, thanks.
> >
> >> LGTM, please install. Thanks!
> >
> > Done in commit 82125b1a661, and closing the bug.
> >
> > Thank you all,
> 
> This "FIXME: Make this alias obsolete in future release"
> means that after syncing the commit from emacs-30 to master,
> it could be declared obsolete in master immediately?

No, we need to collect user experience first.