GNU bug report logs -
#59575
29.0.50; add-log-current-defun-header-regexp matches Windows drive letter
Previous Next
Reported by: Juanma Barranquero <lekktu <at> gmail.com>
Date: Fri, 25 Nov 2022 15:55:01 UTC
Severity: normal
Found in version 29.0.50
Done: Juanma Barranquero <lekktu <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59575 in the body.
You can then email your comments to 59575 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Fri, 25 Nov 2022 15:55:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Juanma Barranquero <lekktu <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Fri, 25 Nov 2022 15:55:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
If you have an *xref* buffer with absolute Windows filenames, like
~/.emacs.d/init.el
93: server-name (replace-regexp-in-string "\\\\+" "\\"
serv t t)
1102: (let ((s (when server-name
1104: (string-match (rx (+ (not (any ?\\))) line-end)
server-name)
1105: (upcase (match-string 0 server-name))))))
d:/Devel/emacs/repo/trunk/lisp/erc/erc-backend.el
1820: (pcase-let ((`(,server-name ,server-version)
1823: (setq erc-server-announced-name server-name)
1827: 's004 ?s server-name ?v server-version
and put the cursor in an absolute filename line (like the one
d:/Devel/[etc] above), calling `add-log-current-defun' returns the drive
letter "d", because it matches a-l-c-d-header-regexp.
The effect is visible when you have which-function-mode enabled, because
the function returns non-nil, so which-function does not resort to imenu,
and you end with "d" in the mode-line.
I suppose this should be fixed in xref.el, which apparently assumes that
file name lines will be either relative or Unix-style (/path/file works
correctly, it's just d:/path/file that fails) and the add-log heuristics
will always fail.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sat, 26 Nov 2022 13:04:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 59575 <at> debbugs.gnu.org (full text, mbox):
> From: Juanma Barranquero <lekktu <at> gmail.com>
> Date: Fri, 25 Nov 2022 16:53:51 +0100
>
> If you have an *xref* buffer with absolute Windows filenames, like
>
> ~/.emacs.d/init.el
> 93: server-name (replace-regexp-in-string "\\\\+" "\\" serv t t)
> 1102: (let ((s (when server-name
> 1104: (string-match (rx (+ (not (any ?\\))) line-end) server-name)
> 1105: (upcase (match-string 0 server-name))))))
> d:/Devel/emacs/repo/trunk/lisp/erc/erc-backend.el
> 1820: (pcase-let ((`(,server-name ,server-version)
> 1823: (setq erc-server-announced-name server-name)
> 1827: 's004 ?s server-name ?v server-version
>
> and put the cursor in an absolute filename line (like the one d:/Devel/[etc] above), calling
> `add-log-current-defun' returns the drive letter "d", because it matches a-l-c-d-header-regexp.
>
> The effect is visible when you have which-function-mode enabled, because the function returns non-nil, so
> which-function does not resort to imenu, and you end with "d" in the mode-line.
>
> I suppose this should be fixed in xref.el, which apparently assumes that file name lines will be either relative
> or Unix-style (/path/file works correctly, it's just d:/path/file that fails) and the add-log heuristics will always
> fail.
xref.el doesn't know anything about add-log, and AFAICT doesn't customize it
in any way, shape, or form. So I think this should be fixed in add-log.el.
Its regexp is too naïve, and should be beefed-up not to fail in this way.
For example, is it really reasonable to accept "defuns" whose name is a
single letter? Or if it's impossible to do that in the regexp, then we
should reject such "matches" in add-log-current-defun instead.
It is also possible to have xref.el define its customized
add-log-current-defun-function, but that sounds like overkill to me.
Dmitry, WDYT?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sat, 26 Nov 2022 13:19:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sat, Nov 26, 2022 at 2:03 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> It is also possible to have xref.el define its customized
> add-log-current-defun-function, but that sounds like overkill to me.
That's what I meant, because it seemed to me less fragile than fiddling
with the regexp. But I don't have a strong preference, as long as it is
fixed.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 01:55:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 59575 <at> debbugs.gnu.org (full text, mbox):
On 26/11/22 15:03, Eli Zaretskii wrote:
> It is also possible to have xref.el define its customized
> add-log-current-defun-function, but that sounds like overkill to me.
> Dmitry, WDYT?
It seems like
(setq add-log-current-defun-function nil)
should suffice.
Is that the intended behavior, to report the absence of the current defun?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 06:50:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 59575 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 27 Nov 2022 03:54:27 +0200
> Cc: 59575 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> On 26/11/22 15:03, Eli Zaretskii wrote:
> > It is also possible to have xref.el define its customized
> > add-log-current-defun-function, but that sounds like overkill to me.
> > Dmitry, WDYT?
>
> It seems like
>
> (setq add-log-current-defun-function nil)
>
> should suffice.
The value is already nil in the *xref* buffer, so I'm unsure how will the
above help.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 08:31:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, Nov 27, 2022 at 2:54 AM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> It seems like
>
> (setq add-log-current-defun-function nil)
>
> should suffice.
That does not stop add-log-current-defun for working, in absence of an
a-l-c-d-function it just uses the regexp, which is what is returning the
spurious name.
Either adding a custom add-log-current-defun-function, or setting
add-log-current-defun-header-regexp to a buffer-local value so it
recognizes filenames would work. (Buffer-locally, because the variable is
in fact customizable so we can't be sure what its global value will be.)
This works, for example:
(defun xref--add-log-current-defun ()
(if-let (item (xref--item-at-point))
(xref-file-location-file (xref-match-item-location item))
(xref--imenu-extract-index-name)))
(setq-local add-log-current-defun-function #'xref--add-log-current-defun)
> Is that the intended behavior, to report the absence of the current defun?
The final intended behavior, IIUC, is that the filenames in the
xref--xref-buffer-mode get passed to which-function as "defun" names.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 11:12:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, Nov 27, 2022 at 9:29 AM Juanma Barranquero <lekktu <at> gmail.com> wrote:
>
> This works, for example:
>
> (defun xref--add-log-current-defun ()
> (if-let (item (xref--item-at-point))
> (xref-file-location-file (xref-match-item-location item))
> (xref--imenu-extract-index-name)))
>
> (setq-local add-log-current-defun-function #'xref--add-log-current-defun)
In fact, to respect the value of `xref-file-name-display' (which seems a
good idea) a bit more complexity is required:
(defun xref--add-log-current-defun ()
"Return the string used to group a set of locations.
This function is used as a value for `add-log-current-defun-function'."
(xref--group-name-for-display
(if-let (item (xref--item-at-point))
(xref-location-group (xref-match-item-location item))
(xref--imenu-extract-index-name))
(xref--project-root (project-current))))
but that uncovers a different bug in xref--group-name-for-display:
(cl-ecase xref-file-name-display
(abs group)
(nondirectory
(if (string-match-p "\\`~?/" group)
(file-name-nondirectory group)
group))
that is, for the 'nondirectory case it tries to match against ~/filename or
/filename, but (again) ignores absolute Windows paths with a drive letter.
That should be changed to
(cl-ecase xref-file-name-display
(abs group)
(nondirectory
- (if (string-match-p "\\`~?/" group)
+ (if (string-match-p "\\`\\(~\\|[A-Za-z]:\\)?/" group)
(file-name-nondirectory group)
group))
(project-relative
or something similar.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 11:23:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, Nov 27, 2022 at 12:11 PM Juanma Barranquero <lekktu <at> gmail.com>
wrote:
> That should be changed to
>
> (cl-ecase xref-file-name-display
> (abs group)
> (nondirectory
> - (if (string-match-p "\\`~?/" group)
> + (if (string-match-p "\\`\\(~\\|[A-Za-z]:\\)?/" group)
> (file-name-nondirectory group)
> group))
> (project-relative
>
> or something similar.
>
Silly me, that should just use `file-name-absolute-p'.
OK to fix this, regardless of what's decided for the original bug?
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 12:35:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 59575 <at> debbugs.gnu.org (full text, mbox):
On 27/11/22 08:49, Eli Zaretskii wrote:
>> Date: Sun, 27 Nov 2022 03:54:27 +0200
>> Cc:59575 <at> debbugs.gnu.org
>> From: Dmitry Gutov<dgutov <at> yandex.ru>
>>
>> On 26/11/22 15:03, Eli Zaretskii wrote:
>>> It is also possible to have xref.el define its customized
>>> add-log-current-defun-function, but that sounds like overkill to me.
>>> Dmitry, WDYT?
>> It seems like
>>
>> (setq add-log-current-defun-function nil)
>>
>> should suffice.
> The value is already nil in the*xref* buffer, so I'm unsure how will the
> above help.
Sorry, brain fart. I meant
(setq add-log-current-defun-function #'ignore)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 12:41:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, Nov 27, 2022 at 1:33 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Sorry, brain fart. I meant
>
> (setq add-log-current-defun-function #'ignore)
That "works", in the sense that now which-func gets "n/a" in the xref
results buffer instead of a bogus name. But which-func can be useful in
xref result buffers.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 13:05:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 59575 <at> debbugs.gnu.org (full text, mbox):
On 27/11/22 13:22, Juanma Barranquero wrote:
> On Sun, Nov 27, 2022 at 12:11 PM Juanma Barranquero <lekktu <at> gmail.com
> <mailto:lekktu <at> gmail.com>> wrote:
>
> That should be changed to
>
> (cl-ecase xref-file-name-display
> (abs group)
> (nondirectory
> - (if (string-match-p "\\`~?/" group)
> + (if (string-match-p "\\`\\(~\\|[A-Za-z]:\\)?/" group)
> (file-name-nondirectory group)
> group))
> (project-relative
>
> or something similar.
>
>
> Silly me, that should just use `file-name-absolute-p'.
>
> OK to fix this, regardless of what's decided for the original bug?
Yes, please. The original idea was to avoid costly operations (in case
the list of matches is long), but looks like file-name-absolute-p
doesn't hit the disk.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 13:06:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 59575 <at> debbugs.gnu.org (full text, mbox):
On 27/11/22 10:29, Juanma Barranquero wrote:
>> Is that the intended behavior, to report the absence of the current defun?
>
> The final intended behavior, IIUC, is that the filenames in the
> xref--xref-buffer-mode get passed to which-function as "defun" names.
I don't mind this approach either, but given that the "definitions" in
Xref buffers are often only 1 line long, or maybe a few, wouldn't
showing the file name again somewhere nearby (e.g. the header bar) just
be redundant?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 13:20:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, Nov 27, 2022 at 2:05 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> I don't mind this approach either, but given that the "definitions" in
> Xref buffers are often only 1 line long, or maybe a few, wouldn't
> showing the file name again somewhere nearby (e.g. the header bar) just
> be redundant?
Sometimes you've got a lot of matches. Also, I tend to display the output
of xref (and, generally speaking, of all kinds of "locating"/"matching"
functions, like occur, etc) in small windows, usually less than ten lines
high. And anyway, this won't bother anyone who's not already using
which-func.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 13:23:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 59575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, Nov 27, 2022 at 2:04 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Yes, please. The original idea was to avoid costly operations (in case
> the list of matches is long), but looks like file-name-absolute-p
> doesn't hit the disk.
Done in commit 41d2365d58 of 2022-11-27:
Fix xref to correctly display Windows absolute filenames
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59575; Package
emacs.
(Sun, 27 Nov 2022 13:42:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 59575 <at> debbugs.gnu.org (full text, mbox):
On 27/11/22 15:18, Juanma Barranquero wrote:
> On Sun, Nov 27, 2022 at 2:05 PM Dmitry Gutov <dgutov <at> yandex.ru
> <mailto:dgutov <at> yandex.ru>> wrote:
>
>> I don't mind this approach either, but given that the "definitions" in
>> Xref buffers are often only 1 line long, or maybe a few, wouldn't
>> showing the file name again somewhere nearby (e.g. the header bar) just
>> be redundant?
>
> Sometimes you've got a lot of matches. Also, I tend to display the
> output of xref (and, generally speaking, of all kinds of
> "locating"/"matching" functions, like occur, etc) in small windows,
> usually less than ten lines high. And anyway, this won't bother anyone
> who's not already using which-func.
Yeah, okay. Sounds good.
Reply sent
to
Juanma Barranquero <lekktu <at> gmail.com>:
You have taken responsibility.
(Sun, 27 Nov 2022 14:04:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Juanma Barranquero <lekktu <at> gmail.com>:
bug acknowledged by developer.
(Sun, 27 Nov 2022 14:04:02 GMT)
Full text and
rfc822 format available.
Message #52 received at 59575-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Fixed in commit 31cfd6d311 of 2022-11-27
Fix xref interaction with which-func (bug#59575)
[Message part 2 (text/html, inline)]
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Mon, 26 Dec 2022 12:24:10 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 233 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.