* bug#65631: Xref updates stack in case of error @ 2023-08-30 16:44 Juri Linkov 2023-08-30 17:01 ` Eli Zaretskii 2023-09-01 13:25 ` Mattias Engdegård 0 siblings, 2 replies; 8+ messages in thread From: Juri Linkov @ 2023-08-30 16:44 UTC (permalink / raw) To: 65631; +Cc: dmitry gutov X-Debbugs-CC: Dmitry Gutov <dmitry@gutov.dev> 0. emacs -Q 1. move point to any word 2. type 'M-.' (xref-find-definitions) 3. an error is displayed correctly "No definitions found for: This" But the problem is that an unrecognized word is added to the xref stack. So the context menu shows "Go Back", and 'M-,' (xref-go-back) goes back to that word. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-08-30 16:44 bug#65631: Xref updates stack in case of error Juri Linkov @ 2023-08-30 17:01 ` Eli Zaretskii 2023-08-30 17:16 ` Juri Linkov 2023-09-01 13:25 ` Mattias Engdegård 1 sibling, 1 reply; 8+ messages in thread From: Eli Zaretskii @ 2023-08-30 17:01 UTC (permalink / raw) To: Juri Linkov; +Cc: 65631, dmitry > Cc: dmitry gutov <dmitry@gutov.dev> > From: Juri Linkov <juri@linkov.net> > Date: Wed, 30 Aug 2023 19:44:13 +0300 > > X-Debbugs-CC: Dmitry Gutov <dmitry@gutov.dev> > > 0. emacs -Q > 1. move point to any word > 2. type 'M-.' (xref-find-definitions) > 3. an error is displayed correctly "No definitions found for: This" > > But the problem is that an unrecognized word is added to the xref stack. > So the context menu shows "Go Back", and 'M-,' (xref-go-back) goes back > to that word. FWIW, I'm not sure this is necessarily a bug. It looks like a bug in your case, because you deliberately tricked Xref into using a word that is definitely not a program symbol. But that is not the case in a more reasonable situation, where point is on a symbol, but for some reason the symbol's definition is not found, e.g., because the TAGS table needs to be regenerated. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-08-30 17:01 ` Eli Zaretskii @ 2023-08-30 17:16 ` Juri Linkov 2023-08-31 0:50 ` Dmitry Gutov 0 siblings, 1 reply; 8+ messages in thread From: Juri Linkov @ 2023-08-30 17:16 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65631, dmitry >> 1. move point to any word >> 2. type 'M-.' (xref-find-definitions) >> 3. an error is displayed correctly "No definitions found for: This" >> >> But the problem is that an unrecognized word is added to the xref stack. >> So the context menu shows "Go Back", and 'M-,' (xref-go-back) goes back >> to that word. > > FWIW, I'm not sure this is necessarily a bug. It looks like a bug in > your case, because you deliberately tricked Xref into using a word > that is definitely not a program symbol. But that is not the case in > a more reasonable situation, where point is on a symbol, but for some > reason the symbol's definition is not found, e.g., because the TAGS > table needs to be regenerated. In case of error, point doesn't move. So there is no need to go back. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-08-30 17:16 ` Juri Linkov @ 2023-08-31 0:50 ` Dmitry Gutov 2023-08-31 16:33 ` Juri Linkov 0 siblings, 1 reply; 8+ messages in thread From: Dmitry Gutov @ 2023-08-31 0:50 UTC (permalink / raw) To: Juri Linkov, Eli Zaretskii; +Cc: 65631 [-- Attachment #1: Type: text/plain, Size: 1073 bytes --] Hi! On 30/08/2023 20:16, Juri Linkov wrote: >>> 1. move point to any word >>> 2. type 'M-.' (xref-find-definitions) >>> 3. an error is displayed correctly "No definitions found for: This" >>> >>> But the problem is that an unrecognized word is added to the xref stack. >>> So the context menu shows "Go Back", and 'M-,' (xref-go-back) goes back >>> to that word. >> FWIW, I'm not sure this is necessarily a bug. It looks like a bug in >> your case, because you deliberately tricked Xref into using a word >> that is definitely not a program symbol. But that is not the case in >> a more reasonable situation, where point is on a symbol, but for some >> reason the symbol's definition is not found, e.g., because the TAGS >> table needs to be regenerated. > In case of error, point doesn't move. So there is no need to go back. At this point, I was kind of feeling that this is a known-but-tolerated behavior (that some might already be relying on, also known as "spacebar heater" effect), but if it's still annoying, let's see about a fix. How's the attached patch? [-- Attachment #2: xref--push-markers.diff --] [-- Type: text/x-patch, Size: 2078 bytes --] diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el index dbafa00c3ad..9b64ef4de10 100644 --- a/lisp/progmodes/xref.el +++ b/lisp/progmodes/xref.el @@ -1468,7 +1468,6 @@ xref-show-xrefs (xref--show-xrefs fetcher display-action)) (defun xref--show-xrefs (fetcher display-action &optional _always-show-list) - (xref--push-markers) (unless (functionp fetcher) ;; Old convention. (let ((xrefs fetcher)) @@ -1479,21 +1478,30 @@ xref--show-xrefs (prog1 xrefs (setq xrefs 'called-already))))))) - (funcall xref-show-xrefs-function fetcher - `((window . ,(selected-window)) - (display-action . ,display-action) - (auto-jump . ,xref-auto-jump-to-first-xref)))) + (let ((cb (current-buffer)) + (pt (point))) + (funcall xref-show-xrefs-function fetcher + `((window . ,(selected-window)) + (display-action . ,display-action) + (auto-jump . ,xref-auto-jump-to-first-xref))) + (xref--push-markers cb pt))) (defun xref--show-defs (xrefs display-action) - (xref--push-markers) - (funcall xref-show-definitions-function xrefs - `((window . ,(selected-window)) - (display-action . ,display-action) - (auto-jump . ,xref-auto-jump-to-first-definition)))) - -(defun xref--push-markers () - (unless (region-active-p) (push-mark nil t)) - (xref-push-marker-stack)) + (let ((cb (current-buffer)) + (pt (point))) + (funcall xref-show-definitions-function xrefs + `((window . ,(selected-window)) + (display-action . ,display-action) + (auto-jump . ,xref-auto-jump-to-first-definition))) + (xref--push-markers cb pt))) + +(defun xref--push-markers (buf pt) + (when (buffer-live-p buf) + (save-excursion + (with-no-warnings (set-buffer buf)) + (goto-char pt) + (unless (region-active-p) (push-mark nil t)) + (xref-push-marker-stack)))) (defun xref--prompt-p (command) (or (eq xref-prompt-for-identifier t) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-08-31 0:50 ` Dmitry Gutov @ 2023-08-31 16:33 ` Juri Linkov 2023-09-01 1:39 ` Dmitry Gutov 0 siblings, 1 reply; 8+ messages in thread From: Juri Linkov @ 2023-08-31 16:33 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 65631, Eli Zaretskii >>>> 1. move point to any word >>>> 2. type 'M-.' (xref-find-definitions) >>>> 3. an error is displayed correctly "No definitions found for: This" >>>> >>>> But the problem is that an unrecognized word is added to the xref stack. >>>> So the context menu shows "Go Back", and 'M-,' (xref-go-back) goes back >>>> to that word. >>> FWIW, I'm not sure this is necessarily a bug. It looks like a bug in >>> your case, because you deliberately tricked Xref into using a word >>> that is definitely not a program symbol. But that is not the case in >>> a more reasonable situation, where point is on a symbol, but for some >>> reason the symbol's definition is not found, e.g., because the TAGS >>> table needs to be regenerated. >> In case of error, point doesn't move. So there is no need to go back. > > At this point, I was kind of feeling that this is a known-but-tolerated > behavior (that some might already be relying on, also known as "spacebar > heater" effect), but if it's still annoying, let's see about a fix. It's not annoying, just surprises with the requirement to type extra ‘M-,’. > How's the attached patch? Thanks. I expected a smaller change, but if this is still reliable, then why not. At least, it works in cases that I tested. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-08-31 16:33 ` Juri Linkov @ 2023-09-01 1:39 ` Dmitry Gutov 0 siblings, 0 replies; 8+ messages in thread From: Dmitry Gutov @ 2023-09-01 1:39 UTC (permalink / raw) To: Juri Linkov; +Cc: 65631-done, Eli Zaretskii Version: 30.1 On 31/08/2023 19:33, Juri Linkov wrote: >>>>> 1. move point to any word >>>>> 2. type 'M-.' (xref-find-definitions) >>>>> 3. an error is displayed correctly "No definitions found for: This" >>>>> >>>>> But the problem is that an unrecognized word is added to the xref stack. >>>>> So the context menu shows "Go Back", and 'M-,' (xref-go-back) goes back >>>>> to that word. >>>> FWIW, I'm not sure this is necessarily a bug. It looks like a bug in >>>> your case, because you deliberately tricked Xref into using a word >>>> that is definitely not a program symbol. But that is not the case in >>>> a more reasonable situation, where point is on a symbol, but for some >>>> reason the symbol's definition is not found, e.g., because the TAGS >>>> table needs to be regenerated. >>> In case of error, point doesn't move. So there is no need to go back. >> >> At this point, I was kind of feeling that this is a known-but-tolerated >> behavior (that some might already be relying on, also known as "spacebar >> heater" effect), but if it's still annoying, let's see about a fix. > > It's not annoying, just surprises with the requirement to type extra ‘M-,’. > >> How's the attached patch? > > Thanks. I expected a smaller change, but if this is still reliable, > then why not. At least, it works in cases that I tested. I was looking for a smaller change, too. Thanks for testing. Pushed to master in 17188e07ab9, closing. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-08-30 16:44 bug#65631: Xref updates stack in case of error Juri Linkov 2023-08-30 17:01 ` Eli Zaretskii @ 2023-09-01 13:25 ` Mattias Engdegård 2023-09-02 2:00 ` Dmitry Gutov 1 sibling, 1 reply; 8+ messages in thread From: Mattias Engdegård @ 2023-09-01 13:25 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 65631, Eli Zaretskii, Juri Linkov 17188e07ab9 appears to break etags-tests on master. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#65631: Xref updates stack in case of error 2023-09-01 13:25 ` Mattias Engdegård @ 2023-09-02 2:00 ` Dmitry Gutov 0 siblings, 0 replies; 8+ messages in thread From: Dmitry Gutov @ 2023-09-02 2:00 UTC (permalink / raw) To: Mattias Engdegård; +Cc: 65631, Eli Zaretskii, Juri Linkov On 01/09/2023 16:25, Mattias Engdegård wrote: > 17188e07ab9 appears to break etags-tests on master. Thanks for reporting, should be fixed in f735eb9628. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-02 2:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-30 16:44 bug#65631: Xref updates stack in case of error Juri Linkov 2023-08-30 17:01 ` Eli Zaretskii 2023-08-30 17:16 ` Juri Linkov 2023-08-31 0:50 ` Dmitry Gutov 2023-08-31 16:33 ` Juri Linkov 2023-09-01 1:39 ` Dmitry Gutov 2023-09-01 13:25 ` Mattias Engdegård 2023-09-02 2:00 ` Dmitry Gutov
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).