* bug#44294: No widen by xref-find-definitions
@ 2020-10-28 20:01 Juri Linkov
2020-10-28 21:37 ` Dmitry Gutov
2020-10-29 11:31 ` Dmitry Gutov
0 siblings, 2 replies; 13+ messages in thread
From: Juri Linkov @ 2020-10-28 20:01 UTC (permalink / raw)
To: 44294; +Cc: dmitry gutov
[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]
X-Debbugs-CC: Dmitry Gutov <dgutov@yandex.ru>
Tags: patch
>>> diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
>>> @@ -897,8 +897,10 @@ xref-location-marker
>>> - (goto-char (or (cdr buffer-point) (point-min)))
>>> - (point-marker))))))
>>> + (save-restriction
>>> + (widen)
>>> + (goto-char (or (cdr buffer-point) (point-min)))
>>> + (point-marker)))))))
>
> If this works, sure, please install. This piece by itself makes sense.
>
>> Hm... Does Dmitry have any comments here? (Added to Cc.)
>
> I'm subscribed to the bug tracker, but I skip over a number of discussions
> automatically (and an extra email in a thread is relatively easy to miss,
> too).
>
> In case I don't respond in similar circumstances in the future, may I ask
> for a personal email? One without 'debbugs.gnu.org' in the From/To/Cc.
Oh, sorry, I noticed this problem is just the tip of the iceberg,
so creating a separate bug report from bug#5042 and bug#9917.
The patch above fixes the problem of using xref-find-definitions
in elisp-mode, but using xref-find-definitions in narrowed
C files still fails with the error "Rerun etags: `%s' not found in %s".
Here is an additional patch that fixes this as well:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: etags-goto-tag-location-widen.patch --]
[-- Type: text/x-diff, Size: 845 bytes --]
diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index f6af1f2ea8..1b7f3c72a2 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1406,8 +1406,12 @@ etags-goto-tag-location
offset (* 3 offset))) ; expand search window
(or found
(re-search-forward pat nil t)
- (user-error "Rerun etags: `%s' not found in %s"
- pat buffer-file-name)))
+ (if (or (= (point-min) 1) (not widen-automatically))
+ (user-error "Rerun etags: `%s' not found in %s"
+ pat buffer-file-name)
+ ;; Rerun after removing narrowing
+ (widen)
+ (etags-goto-tag-location tag-info))))
;; Position point at the right place
;; if the search string matched an extra Ctrl-m at the beginning.
(and (eq selective-display t)
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-28 20:01 bug#44294: No widen by xref-find-definitions Juri Linkov
@ 2020-10-28 21:37 ` Dmitry Gutov
2020-10-29 8:30 ` Philipp Stephani
2020-10-29 9:09 ` Juri Linkov
2020-10-29 11:31 ` Dmitry Gutov
1 sibling, 2 replies; 13+ messages in thread
From: Dmitry Gutov @ 2020-10-28 21:37 UTC (permalink / raw)
To: Juri Linkov, 44294
On 28.10.2020 22:01, Juri Linkov wrote:
> Oh, sorry, I noticed this problem is just the tip of the iceberg,
> so creating a separate bug report from bug#5042 and bug#9917.
>
> The patch above fixes the problem of using xref-find-definitions
> in elisp-mode, but using xref-find-definitions in narrowed
> C files still fails with the error "Rerun etags: `%s' not found in %s".
>
> Here is an additional patch that fixes this as well:
Thank you. I'm fine with both patches, please feel free to install.
> - (user-error "Rerun etags: `%s' not found in %s"
> - pat buffer-file-name)))
> + (if (or (= (point-min) 1) (not widen-automatically))
Couldn't a buffer be narrowed, but point-min still be equal to 1?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-28 21:37 ` Dmitry Gutov
@ 2020-10-29 8:30 ` Philipp Stephani
2020-10-29 9:11 ` Juri Linkov
2020-10-29 9:09 ` Juri Linkov
1 sibling, 1 reply; 13+ messages in thread
From: Philipp Stephani @ 2020-10-29 8:30 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 44294, Juri Linkov
Am Mi., 28. Okt. 2020 um 22:39 Uhr schrieb Dmitry Gutov <dgutov@yandex.ru>:
>
> > - (user-error "Rerun etags: `%s' not found in %s"
> > - pat buffer-file-name)))
> > + (if (or (= (point-min) 1) (not widen-automatically))
>
> Couldn't a buffer be narrowed, but point-min still be equal to 1?
>
Yes, this should be (eql (buffer-size) (- (point-max) (point-min)).
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-29 8:30 ` Philipp Stephani
@ 2020-10-29 9:11 ` Juri Linkov
0 siblings, 0 replies; 13+ messages in thread
From: Juri Linkov @ 2020-10-29 9:11 UTC (permalink / raw)
To: Philipp Stephani; +Cc: 44294, Dmitry Gutov
>> > - (user-error "Rerun etags: `%s' not found in %s"
>> > - pat buffer-file-name)))
>> > + (if (or (= (point-min) 1) (not widen-automatically))
>>
>> Couldn't a buffer be narrowed, but point-min still be equal to 1?
>
> Yes, this should be (eql (buffer-size) (- (point-max) (point-min)).
It's nice that you arrived to the same code that is used in the
existing function buffer-narrowed-p (I missed it before).
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-28 21:37 ` Dmitry Gutov
2020-10-29 8:30 ` Philipp Stephani
@ 2020-10-29 9:09 ` Juri Linkov
2020-10-29 21:20 ` Juri Linkov
1 sibling, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2020-10-29 9:09 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 44294
tags 44294 fixed
close 44294 28.0.50
thanks
>> - (user-error "Rerun etags: `%s' not found in %s"
>> - pat buffer-file-name)))
>> + (if (or (= (point-min) 1) (not widen-automatically))
>
> Couldn't a buffer be narrowed, but point-min still be equal to 1?
Thanks for the good question. Now I installed the patch that
uses the special function 'buffer-narrowed-p'.
This bug could be reopened when more such cases will be discovered.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-29 9:09 ` Juri Linkov
@ 2020-10-29 21:20 ` Juri Linkov
0 siblings, 0 replies; 13+ messages in thread
From: Juri Linkov @ 2020-10-29 21:20 UTC (permalink / raw)
To: 44294
BTW, I noticed that when isearch fails it doesn't inform the user
that there are possibly more matches outside the narrowed region:
diff --git a/lisp/isearch.el b/lisp/isearch.el
index c3d5ff2d31..dae7618556 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3276,6 +3276,8 @@ isearch-message-prefix
(multi-isearch-file-list "multi-file ")
(multi-isearch-buffer-list "multi-buffer ")
(t ""))
+ (if (and (not isearch-success) (buffer-narrowed-p) widen-automatically)
+ "narrowed-buffer " "")
(or isearch-message-prefix-add "")
(if nonincremental "search" "I-search")
(if isearch-forward "" " backward")
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-28 20:01 bug#44294: No widen by xref-find-definitions Juri Linkov
2020-10-28 21:37 ` Dmitry Gutov
@ 2020-10-29 11:31 ` Dmitry Gutov
2020-10-29 21:18 ` Juri Linkov
1 sibling, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2020-10-29 11:31 UTC (permalink / raw)
To: Juri Linkov, 44294
On 28.10.2020 22:01, Juri Linkov wrote:
> - (user-error "Rerun etags: `%s' not found in %s"
> - pat buffer-file-name)))
> + (if (or (= (point-min) 1) (not widen-automatically))
> + (user-error "Rerun etags: `%s' not found in %s"
> + pat buffer-file-name)
> + ;; Rerun after removing narrowing
> + (widen)
> + (etags-goto-tag-location tag-info))))
By the way... have you tried to use the same method here as in
elisp-mode? Meaning, widen unconditionally inside 'save-restriction'.
There should be no reason for backends to do it differently. And this
way, you don't have to always search twice for a missing tag when inside
a narrowing.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-29 11:31 ` Dmitry Gutov
@ 2020-10-29 21:18 ` Juri Linkov
2020-10-29 21:33 ` Dmitry Gutov
0 siblings, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2020-10-29 21:18 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 44294
[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]
reopen 44294
stop
>> - (user-error "Rerun etags: `%s' not found in %s"
>> - pat buffer-file-name)))
>> + (if (or (= (point-min) 1) (not widen-automatically))
>> + (user-error "Rerun etags: `%s' not found in %s"
>> + pat buffer-file-name)
>> + ;; Rerun after removing narrowing
>> + (widen)
>> + (etags-goto-tag-location tag-info))))
>
> By the way... have you tried to use the same method here as in elisp-mode?
> Meaning, widen unconditionally inside 'save-restriction'.
>
> There should be no reason for backends to do it differently. And this way,
> you don't have to always search twice for a missing tag when inside
> a narrowing.
It should not widen unnecessarily when the found position is within the
narrowed region. In this regard, xref--goto-char does the right thing:
(defun xref--goto-char (pos)
(cond
((and (<= (point-min) pos) (<= pos (point-max))))
(widen-automatically (widen))
(t (user-error "Position is outside accessible part of buffer")))
(goto-char pos))
It widens only when position is outside accessible part of buffer
(and widen-automatically is non-nil).
Now I fixed goto-line to use the same condition as in xref--goto-char
(and (<= (point-min) p) (<= p (point-max)))
to widen only when the found position is outside accessible part of buffer:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: goto-line-widen-only-accessible.patch --]
[-- Type: text/x-diff, Size: 1335 bytes --]
diff --git a/lisp/simple.el b/lisp/simple.el
index 2e40e3261c..0cd1739c4d 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1340,18 +1340,19 @@ goto-line
;; Leave mark at previous position
(or (region-active-p) (push-mark))
;; Move to the specified line number in that buffer.
- (if (and (not relative) (not widen-automatically))
- (save-restriction
- (widen)
- (goto-char (point-min))
- (if (eq selective-display t)
- (re-search-forward "[\n\C-m]" nil 'end (1- line))
- (forward-line (1- line))))
- (unless relative (widen))
- (goto-char (point-min))
- (if (eq selective-display t)
- (re-search-forward "[\n\C-m]" nil 'end (1- line))
- (forward-line (1- line)))))
+ (let ((p (save-restriction
+ (unless relative (widen))
+ (goto-char (point-min))
+ (if (eq selective-display t)
+ (re-search-forward "[\n\C-m]" nil 'end (1- line))
+ (forward-line (1- line)))
+ (point))))
+ (when (and (not relative)
+ (buffer-narrowed-p)
+ widen-automatically
+ (not (and (<= (point-min) p) (<= p (point-max)))))
+ (widen))
+ (goto-char p)))
(defun goto-line-relative (line &optional buffer)
"Go to LINE, counting from line at (point-min).
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-29 21:18 ` Juri Linkov
@ 2020-10-29 21:33 ` Dmitry Gutov
2020-10-30 7:38 ` Juri Linkov
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2020-10-29 21:33 UTC (permalink / raw)
To: Juri Linkov; +Cc: 44294
On 29.10.2020 23:18, Juri Linkov wrote:
> reopen 44294
> stop
>
>>> - (user-error "Rerun etags: `%s' not found in %s"
>>> - pat buffer-file-name)))
>>> + (if (or (= (point-min) 1) (not widen-automatically))
>>> + (user-error "Rerun etags: `%s' not found in %s"
>>> + pat buffer-file-name)
>>> + ;; Rerun after removing narrowing
>>> + (widen)
>>> + (etags-goto-tag-location tag-info))))
>>
>> By the way... have you tried to use the same method here as in elisp-mode?
>> Meaning, widen unconditionally inside 'save-restriction'.
>>
>> There should be no reason for backends to do it differently. And this way,
>> you don't have to always search twice for a missing tag when inside
>> a narrowing.
>
> It should not widen unnecessarily when the found position is within the
> narrowed region.
Hence the use of save-restriction in elisp-mode which you added.
etags can use the exact same approach.
> In this regard, xref--goto-char does the right thing:
Yes.
> (defun xref--goto-char (pos)
> (cond
> ((and (<= (point-min) pos) (<= pos (point-max))))
> (widen-automatically (widen))
> (t (user-error "Position is outside accessible part of buffer")))
> (goto-char pos))
>
> It widens only when position is outside accessible part of buffer
> (and widen-automatically is non-nil).
And this code runs whether the backend is elisp, etags, or whatever else.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-29 21:33 ` Dmitry Gutov
@ 2020-10-30 7:38 ` Juri Linkov
2020-10-30 16:54 ` Dmitry Gutov
0 siblings, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2020-10-30 7:38 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 44294
>>> By the way... have you tried to use the same method here as in elisp-mode?
>>> Meaning, widen unconditionally inside 'save-restriction'.
>>>
>>> There should be no reason for backends to do it differently. And this way,
>>> you don't have to always search twice for a missing tag when inside
>>> a narrowing.
>> It should not widen unnecessarily when the found position is within the
>> narrowed region.
>
> Hence the use of save-restriction in elisp-mode which you added.
>
> etags can use the exact same approach.
I see now what you mean. Then maybe better to revert the change in
etags-goto-tag-location to restore its original behavior, and
fix only its usage by xref, i.e. exactly the same change
for xref-location-marker ((l xref-etags-location)) as was made
for xref-location-marker ((l xref-elisp-location)):
diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 41ed317766..8879726ad5 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -2140,8 +2135,10 @@ xref-location-marker
(let ((buffer (find-file-noselect file)))
(with-current-buffer buffer
(save-excursion
- (etags-goto-tag-location tag-info)
- (point-marker))))))
+ (save-restriction
+ (widen)
+ (etags-goto-tag-location tag-info)
+ (point-marker)))))))
(cl-defmethod xref-location-line ((l xref-etags-location))
(with-slots (tag-info) l
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-30 7:38 ` Juri Linkov
@ 2020-10-30 16:54 ` Dmitry Gutov
2020-10-31 19:41 ` Juri Linkov
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2020-10-30 16:54 UTC (permalink / raw)
To: Juri Linkov; +Cc: 44294
On 30.10.2020 09:38, Juri Linkov wrote:
> I see now what you mean. Then maybe better to revert the change in
> etags-goto-tag-location to restore its original behavior, and
> fix only its usage by xref, i.e. exactly the same change
> for xref-location-marker ((l xref-etags-location)) as was made
> for xref-location-marker ((l xref-elisp-location)):
Sounds good. 'find-tag-in-order' calls 'widen' already, so 'find-tag' is
probably fine.
I would personally combine the revert and the new change in one commit,
but either option will work, obviously.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#44294: No widen by xref-find-definitions
2020-10-30 16:54 ` Dmitry Gutov
@ 2020-10-31 19:41 ` Juri Linkov
2020-10-31 23:14 ` Dmitry Gutov
0 siblings, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2020-10-31 19:41 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 44294
tags 44294 fixed
close 44294 28.0.50
thanks
>> I see now what you mean. Then maybe better to revert the change in
>> etags-goto-tag-location to restore its original behavior, and
>> fix only its usage by xref, i.e. exactly the same change
>> for xref-location-marker ((l xref-etags-location)) as was made
>> for xref-location-marker ((l xref-elisp-location)):
>
> Sounds good. 'find-tag-in-order' calls 'widen' already, so 'find-tag' is
> probably fine.
>
> I would personally combine the revert and the new change in one commit, but
> either option will work, obviously.
Thanks for the suggestion. Now installed and closed.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-10-31 23:14 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-28 20:01 bug#44294: No widen by xref-find-definitions Juri Linkov
2020-10-28 21:37 ` Dmitry Gutov
2020-10-29 8:30 ` Philipp Stephani
2020-10-29 9:11 ` Juri Linkov
2020-10-29 9:09 ` Juri Linkov
2020-10-29 21:20 ` Juri Linkov
2020-10-29 11:31 ` Dmitry Gutov
2020-10-29 21:18 ` Juri Linkov
2020-10-29 21:33 ` Dmitry Gutov
2020-10-30 7:38 ` Juri Linkov
2020-10-30 16:54 ` Dmitry Gutov
2020-10-31 19:41 ` Juri Linkov
2020-10-31 23:14 ` 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).