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

Previous Next

Package: emacs;

Reported by: Eshel Yaron <me <at> eshelyaron.com>

Date: Mon, 10 Jun 2024 10:26:02 UTC

Severity: normal

Found in version 30.0.50

Fixed in version 30.1

Done: Eshel Yaron <me <at> eshelyaron.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71466 in the body.
You can then email your comments to 71466 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to juri <at> linkov.net, bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 10 Jun 2024 10:26:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Eshel Yaron <me <at> eshelyaron.com>:
New bug report received and forwarded. Copy sent to juri <at> linkov.net, bug-gnu-emacs <at> gnu.org. (Mon, 10 Jun 2024 10:26:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List
 is reverted
Date: Mon, 10 Jun 2024 10:49:18 +0200
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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 11 Jun 2024 17:10:02 GMT) Full text and rfc822 format available.

Message #8 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 71466 <at> debbugs.gnu.org
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 11 Jun 2024 20:05:51 +0300
> 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'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 06:37:02 GMT) Full text and rfc822 format available.

Message #11 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 71466 <at> debbugs.gnu.org
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 17 Jun 2024 09:35:11 +0300
[Message part 1 (text/plain, inline)]
> 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:

[xref-revert-buffer-restore-point.patch (text/x-diff, inline)]
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.
 

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 07:41:01 GMT) Full text and rfc822 format available.

Message #14 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 71466 <at> debbugs.gnu.org
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 17 Jun 2024 09:40:30 +0200
Hi Juri,

Juri Linkov <juri <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 12:29:01 GMT) Full text and rfc822 format available.

Message #17 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>, Juri Linkov <juri <at> linkov.net>
Cc: 71466 <at> debbugs.gnu.org
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 17 Jun 2024 15:27:45 +0300
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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 12:33:01 GMT) Full text and rfc822 format available.

Message #20 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>, Eshel Yaron <me <at> eshelyaron.com>
Cc: 71466 <at> debbugs.gnu.org
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 17 Jun 2024 15:32:14 +0300
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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 15:44:01 GMT) Full text and rfc822 format available.

Message #23 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 17 Jun 2024 17:43:48 +0200
Dmitry Gutov <dmitry <at> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 16:35:02 GMT) Full text and rfc822 format available.

Message #26 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 17 Jun 2024 19:31:01 +0300
[Message part 1 (text/plain, inline)]
>> +(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:

[xref-revert-buffer-restore-point_2.patch (text/x-diff, inline)]
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.
 

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 22:17:02 GMT) Full text and rfc822 format available.

Message #29 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71466 <at> debbugs.gnu.org, Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 01:16:31 +0300
On 17/06/2024 19:31, Juri Linkov wrote:
> Ok, so this is implemented here:

LGTM, thanks!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 17 Jun 2024 22:25:02 GMT) Full text and rfc822 format available.

Message #32 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 71466 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 01:24:26 +0300
On 17/06/2024 18:43, Eshel Yaron wrote:
> Dmitry Gutov <dmitry <at> 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).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 07:01:02 GMT) Full text and rfc822 format available.

Message #35 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 09:00:38 +0200
Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 17/06/2024 18:43, Eshel Yaron wrote:
>> Dmitry Gutov <dmitry <at> 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))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 12:59:01 GMT) Full text and rfc822 format available.

Message #38 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50;
 Buffer-menu-group-by non-nil resets point when Buffer List is reverted
Date: Tue, 18 Jun 2024 15:58:45 +0300
> Cc: 71466 <at> debbugs.gnu.org, Juri Linkov <juri <at> 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 <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 14:02:02 GMT) Full text and rfc822 format available.

Message #41 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 16:01:40 +0200
Hi Eli,

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: 71466 <at> debbugs.gnu.org, Juri Linkov <juri <at> 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 <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 14:47:01 GMT) Full text and rfc822 format available.

Message #44 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 17:46:10 +0300
> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: dmitry <at> gutov.dev,  71466 <at> debbugs.gnu.org,  juri <at> linkov.net
> Date: Tue, 18 Jun 2024 16:01:40 +0200
> 
> Eli Zaretskii <eliz <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 16:56:02 GMT) Full text and rfc822 format available.

Message #47 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 18:55:46 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Eshel Yaron <me <at> eshelyaron.com>
>> Cc: dmitry <at> gutov.dev,  71466 <at> debbugs.gnu.org,  juri <at> linkov.net
>> Date: Tue, 18 Jun 2024 16:01:40 +0200
>> 
>> Eli Zaretskii <eliz <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 17:06:01 GMT) Full text and rfc822 format available.

Message #50 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Eshel Yaron <me <at> eshelyaron.com>
Cc: 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 20:05:14 +0300
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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 17:40:02 GMT) Full text and rfc822 format available.

Message #53 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 20:36:39 +0300
> Date: Tue, 18 Jun 2024 20:05:14 +0300
> Cc: 71466 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 18 Jun 2024 17:49:01 GMT) Full text and rfc822 format available.

Message #56 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 18 Jun 2024 20:47:52 +0300
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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 20 Jun 2024 16:43:02 GMT) Full text and rfc822 format available.

Message #59 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Thu, 20 Jun 2024 19:38:58 +0300
>>> 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)))))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 20 Jun 2024 17:37:02 GMT) Full text and rfc822 format available.

Message #62 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Thu, 20 Jun 2024 20:35:47 +0300
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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 24 Jun 2024 06:50:02 GMT) Full text and rfc822 format available.

Message #65 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Mon, 24 Jun 2024 09:27:52 +0300
>> 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")




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Mon, 24 Jun 2024 22:43:02 GMT) Full text and rfc822 format available.

Message #68 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 25 Jun 2024 01:42:46 +0300
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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 25 Jun 2024 07:01:02 GMT) Full text and rfc822 format available.

Message #71 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 25 Jun 2024 09:54:32 +0300
>> 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).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 25 Jun 2024 12:55:02 GMT) Full text and rfc822 format available.

Message #74 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Tue, 25 Jun 2024 15:54:40 +0300
> Date: Tue, 25 Jun 2024 01:42:46 +0300
> Cc: Eli Zaretskii <eliz <at> gnu.org>, me <at> eshelyaron.com, 71466 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Tue, 25 Jun 2024 23:16:02 GMT) Full text and rfc822 format available.

Message #77 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Wed, 26 Jun 2024 02:14:45 +0300
On 25/06/2024 15:54, Eli Zaretskii wrote:
>> Date: Tue, 25 Jun 2024 01:42:46 +0300
>> Cc: Eli Zaretskii<eliz <at> gnu.org>,me <at> eshelyaron.com,71466 <at> debbugs.gnu.org
>> From: Dmitry Gutov<dmitry <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Wed, 26 Jun 2024 11:26:01 GMT) Full text and rfc822 format available.

Message #80 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Wed, 26 Jun 2024 14:25:01 +0300
> Date: Wed, 26 Jun 2024 02:14:45 +0300
> Cc: juri <at> linkov.net, me <at> eshelyaron.com, 71466 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Wed, 26 Jun 2024 16:58:02 GMT) Full text and rfc822 format available.

Message #83 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Wed, 26 Jun 2024 18:56:53 +0200
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Wed, 26 Jun 2024 02:14:45 +0300
>> Cc: juri <at> linkov.net, me <at> eshelyaron.com, 71466 <at> debbugs.gnu.org
>> From: Dmitry Gutov <dmitry <at> 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.


[0001-Use-revert-function-in-xref-buffer.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Wed, 26 Jun 2024 18:34:02 GMT) Full text and rfc822 format available.

Message #86 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Wed, 26 Jun 2024 21:33:01 +0300
> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: Dmitry Gutov <dmitry <at> gutov.dev>,  juri <at> linkov.net,  71466 <at> debbugs.gnu.org
> Date: Wed, 26 Jun 2024 18:56:53 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> Date: Wed, 26 Jun 2024 02:14:45 +0300
> >> Cc: juri <at> linkov.net, me <at> eshelyaron.com, 71466 <at> debbugs.gnu.org
> >> From: Dmitry Gutov <dmitry <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 27 Jun 2024 00:13:01 GMT) Full text and rfc822 format available.

Message #89 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Thu, 27 Jun 2024 03:11:50 +0300
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!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 27 Jun 2024 05:47:02 GMT) Full text and rfc822 format available.

Message #92 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Wed, 26 Jun 2024 23:05:20 +0200
close 71466 30.1
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Eshel Yaron <me <at> 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 <at> gutov.dev> writes:

> LGTM, please install. Thanks!

Done in commit 82125b1a661, and closing the bug.

Thank you all,

Eshel




bug marked as fixed in version 30.1, send any further explanations to 71466 <at> debbugs.gnu.org and Eshel Yaron <me <at> eshelyaron.com> Request was from Eshel Yaron <me <at> eshelyaron.com> to control <at> debbugs.gnu.org. (Thu, 27 Jun 2024 05:47:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 27 Jun 2024 06:56:02 GMT) Full text and rfc822 format available.

Message #97 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71466 <at> debbugs.gnu.org, Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Thu, 27 Jun 2024 09:43:25 +0300
>> Ok, so this is implemented here:
>
> LGTM, thanks!

So now this is pushed as well.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 27 Jun 2024 06:56:02 GMT) Full text and rfc822 format available.

Message #100 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>, 71466 <at> debbugs.gnu.org
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71466; Package emacs. (Thu, 27 Jun 2024 07:21:02 GMT) Full text and rfc822 format available.

Message #103 received at 71466 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: dmitry <at> gutov.dev, 71466 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point
 when Buffer List is reverted
Date: Thu, 27 Jun 2024 10:19:54 +0300
> From: Juri Linkov <juri <at> linkov.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  dmitry <at> gutov.dev,  71466 <at> 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.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 25 Jul 2024 11:24:11 GMT) Full text and rfc822 format available.

This bug report was last modified 329 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.