* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted @ 2024-06-10 8:49 Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-11 17:05 ` Juri Linkov 0 siblings, 1 reply; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-10 8:49 UTC (permalink / raw) To: 71466; +Cc: Juri Linkov 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-10 8:49 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 @ 2024-06-11 17:05 ` Juri Linkov 2024-06-17 6:35 ` Juri Linkov 0 siblings, 1 reply; 33+ messages in thread From: Juri Linkov @ 2024-06-11 17:05 UTC (permalink / raw) To: Eshel Yaron; +Cc: 71466 > 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'. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-11 17:05 ` Juri Linkov @ 2024-06-17 6:35 ` Juri Linkov 2024-06-17 7:40 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-17 12:32 ` Dmitry Gutov 0 siblings, 2 replies; 33+ messages in thread From: Juri Linkov @ 2024-06-17 6:35 UTC (permalink / raw) To: Eshel Yaron; +Cc: Dmitry Gutov, 71466 [-- 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. ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 6:35 ` Juri Linkov @ 2024-06-17 7:40 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-17 12:27 ` Dmitry Gutov 2024-06-17 12:32 ` Dmitry Gutov 1 sibling, 1 reply; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-17 7:40 UTC (permalink / raw) To: Juri Linkov; +Cc: Dmitry Gutov, 71466 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 7:40 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-17 12:27 ` Dmitry Gutov 2024-06-17 15:43 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-17 12:27 UTC (permalink / raw) To: Eshel Yaron, Juri Linkov; +Cc: 71466 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 12:27 ` Dmitry Gutov @ 2024-06-17 15:43 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-17 22:24 ` Dmitry Gutov 0 siblings, 1 reply; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-17 15:43 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, Juri Linkov 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 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 15:43 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-17 22:24 ` Dmitry Gutov 2024-06-18 7:00 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-17 22:24 UTC (permalink / raw) To: Eshel Yaron; +Cc: 71466, Juri Linkov 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). ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 22:24 ` Dmitry Gutov @ 2024-06-18 7:00 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 12:58 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-18 7:00 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, Juri Linkov 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)) ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 7:00 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-18 12:58 ` Eli Zaretskii 2024-06-18 14:01 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2024-06-18 12:58 UTC (permalink / raw) To: Eshel Yaron; +Cc: dmitry, 71466, juri > 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? ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 12:58 ` Eli Zaretskii @ 2024-06-18 14:01 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 14:46 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-18 14:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dmitry, 71466, juri 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 14:01 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-18 14:46 ` Eli Zaretskii 2024-06-18 16:55 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 17:05 ` Dmitry Gutov 0 siblings, 2 replies; 33+ messages in thread From: Eli Zaretskii @ 2024-06-18 14:46 UTC (permalink / raw) To: Eshel Yaron; +Cc: dmitry, 71466, juri > 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 14:46 ` Eli Zaretskii @ 2024-06-18 16:55 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 17:05 ` Dmitry Gutov 1 sibling, 0 replies; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-18 16:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dmitry, 71466, juri 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 14:46 ` Eli Zaretskii 2024-06-18 16:55 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-18 17:05 ` Dmitry Gutov 2024-06-18 17:36 ` Eli Zaretskii 1 sibling, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-18 17:05 UTC (permalink / raw) To: Eli Zaretskii, Eshel Yaron; +Cc: 71466, juri 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 17:05 ` Dmitry Gutov @ 2024-06-18 17:36 ` Eli Zaretskii 2024-06-18 17:47 ` Dmitry Gutov 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2024-06-18 17:36 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, me, juri > 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 17:36 ` Eli Zaretskii @ 2024-06-18 17:47 ` Dmitry Gutov 2024-06-20 16:38 ` Juri Linkov 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-18 17:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 71466, me, juri 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-18 17:47 ` Dmitry Gutov @ 2024-06-20 16:38 ` Juri Linkov 2024-06-20 17:35 ` Dmitry Gutov 0 siblings, 1 reply; 33+ messages in thread From: Juri Linkov @ 2024-06-20 16:38 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, 71466, me >>> 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))))) ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-20 16:38 ` Juri Linkov @ 2024-06-20 17:35 ` Dmitry Gutov 2024-06-24 6:27 ` Juri Linkov 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-20 17:35 UTC (permalink / raw) To: Juri Linkov; +Cc: Eli Zaretskii, 71466, me 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-20 17:35 ` Dmitry Gutov @ 2024-06-24 6:27 ` Juri Linkov 2024-06-24 22:42 ` Dmitry Gutov 0 siblings, 1 reply; 33+ messages in thread From: Juri Linkov @ 2024-06-24 6:27 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, 71466, me >> 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") ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-24 6:27 ` Juri Linkov @ 2024-06-24 22:42 ` Dmitry Gutov 2024-06-25 6:54 ` Juri Linkov 2024-06-25 12:54 ` Eli Zaretskii 0 siblings, 2 replies; 33+ messages in thread From: Dmitry Gutov @ 2024-06-24 22:42 UTC (permalink / raw) To: Juri Linkov; +Cc: Eli Zaretskii, 71466, me 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-24 22:42 ` Dmitry Gutov @ 2024-06-25 6:54 ` Juri Linkov 2024-06-25 12:54 ` Eli Zaretskii 1 sibling, 0 replies; 33+ messages in thread From: Juri Linkov @ 2024-06-25 6:54 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, 71466, me >> 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). ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-24 22:42 ` Dmitry Gutov 2024-06-25 6:54 ` Juri Linkov @ 2024-06-25 12:54 ` Eli Zaretskii 2024-06-25 23:14 ` Dmitry Gutov 1 sibling, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2024-06-25 12:54 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, me, juri > 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? ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-25 12:54 ` Eli Zaretskii @ 2024-06-25 23:14 ` Dmitry Gutov 2024-06-26 11:25 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-25 23:14 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 71466, me, juri 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? ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-25 23:14 ` Dmitry Gutov @ 2024-06-26 11:25 ` Eli Zaretskii 2024-06-26 16:56 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2024-06-26 11:25 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, me, juri > 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-26 11:25 ` Eli Zaretskii @ 2024-06-26 16:56 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-26 18:33 ` Eli Zaretskii 2024-06-27 0:11 ` Dmitry Gutov 0 siblings, 2 replies; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-26 16:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Dmitry Gutov, 71466, juri [-- 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 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-26 16:56 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-26 18:33 ` Eli Zaretskii 2024-06-26 21:05 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-27 0:11 ` Dmitry Gutov 1 sibling, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2024-06-26 18:33 UTC (permalink / raw) To: Eshel Yaron; +Cc: dmitry, 71466, juri > 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-26 18:33 ` Eli Zaretskii @ 2024-06-26 21:05 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-27 6:39 ` Juri Linkov 0 siblings, 1 reply; 33+ messages in thread From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-26 21:05 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dmitry, 71466, juri 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-26 21:05 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-27 6:39 ` Juri Linkov 2024-06-27 7:19 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Juri Linkov @ 2024-06-27 6:39 UTC (permalink / raw) To: Eshel Yaron; +Cc: dmitry, Eli Zaretskii, 71466 >>> 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? ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-27 6:39 ` Juri Linkov @ 2024-06-27 7:19 ` Eli Zaretskii 0 siblings, 0 replies; 33+ messages in thread From: Eli Zaretskii @ 2024-06-27 7:19 UTC (permalink / raw) To: Juri Linkov; +Cc: dmitry, 71466, me > 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-26 16:56 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-26 18:33 ` Eli Zaretskii @ 2024-06-27 0:11 ` Dmitry Gutov 1 sibling, 0 replies; 33+ messages in thread From: Dmitry Gutov @ 2024-06-27 0:11 UTC (permalink / raw) To: Eshel Yaron, Eli Zaretskii; +Cc: 71466, juri 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! ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 6:35 ` Juri Linkov 2024-06-17 7:40 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-17 12:32 ` Dmitry Gutov 2024-06-17 16:31 ` Juri Linkov 1 sibling, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-17 12:32 UTC (permalink / raw) To: Juri Linkov, Eshel Yaron; +Cc: 71466 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 12:32 ` Dmitry Gutov @ 2024-06-17 16:31 ` Juri Linkov 2024-06-17 22:16 ` Dmitry Gutov 0 siblings, 1 reply; 33+ messages in thread From: Juri Linkov @ 2024-06-17 16:31 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, Eshel Yaron [-- 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. ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 16:31 ` Juri Linkov @ 2024-06-17 22:16 ` Dmitry Gutov 2024-06-27 6:43 ` Juri Linkov 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Gutov @ 2024-06-17 22:16 UTC (permalink / raw) To: Juri Linkov; +Cc: 71466, Eshel Yaron On 17/06/2024 19:31, Juri Linkov wrote: > Ok, so this is implemented here: LGTM, thanks! ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#71466: 30.0.50; Buffer-menu-group-by non-nil resets point when Buffer List is reverted 2024-06-17 22:16 ` Dmitry Gutov @ 2024-06-27 6:43 ` Juri Linkov 0 siblings, 0 replies; 33+ messages in thread From: Juri Linkov @ 2024-06-27 6:43 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 71466, Eshel Yaron >> Ok, so this is implemented here: > > LGTM, thanks! So now this is pushed as well. ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2024-06-27 7:19 UTC | newest] Thread overview: 33+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-10 8:49 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 2024-06-11 17:05 ` Juri Linkov 2024-06-17 6:35 ` Juri Linkov 2024-06-17 7:40 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-17 12:27 ` Dmitry Gutov 2024-06-17 15:43 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-17 22:24 ` Dmitry Gutov 2024-06-18 7:00 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 12:58 ` Eli Zaretskii 2024-06-18 14:01 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 14:46 ` Eli Zaretskii 2024-06-18 16:55 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-18 17:05 ` Dmitry Gutov 2024-06-18 17:36 ` Eli Zaretskii 2024-06-18 17:47 ` Dmitry Gutov 2024-06-20 16:38 ` Juri Linkov 2024-06-20 17:35 ` Dmitry Gutov 2024-06-24 6:27 ` Juri Linkov 2024-06-24 22:42 ` Dmitry Gutov 2024-06-25 6:54 ` Juri Linkov 2024-06-25 12:54 ` Eli Zaretskii 2024-06-25 23:14 ` Dmitry Gutov 2024-06-26 11:25 ` Eli Zaretskii 2024-06-26 16:56 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-26 18:33 ` Eli Zaretskii 2024-06-26 21:05 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-27 6:39 ` Juri Linkov 2024-06-27 7:19 ` Eli Zaretskii 2024-06-27 0:11 ` Dmitry Gutov 2024-06-17 12:32 ` Dmitry Gutov 2024-06-17 16:31 ` Juri Linkov 2024-06-17 22:16 ` Dmitry Gutov 2024-06-27 6:43 ` Juri Linkov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.