Package: emacs;
Reported by: Eshel Yaron <me <at> eshelyaron.com>
Date: Sun, 24 Nov 2024 07:13:02 UTC
Severity: normal
Found in version 31.0.50
Fixed in version 31.1
Done: Eshel Yaron <me <at> eshelyaron.com>
Bug is archived. No further changes may be made.
Message #14 received at 74504 <at> debbugs.gnu.org (full text, mbox):
From: Eshel Yaron <me <at> eshelyaron.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 74504 <at> debbugs.gnu.org Subject: Re: bug#74504: 31.0.50; Wrong source directory in *Help* Date: Sun, 08 Dec 2024 08:35:34 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: 74504 <at> debbugs.gnu.org
>> Date: Mon, 25 Nov 2024 18:26:12 -0500
>> From: Stefan Monnier via "Bug reports for GNU Emacs,
>> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> > 1. Build Emacs
>> > 2. Run src/emacs -Q
>> > 3. In the *scratch* buffer say M-x cd /tmp
>> > (/tmp here is just an arbitrary directory that is not the Emacs
>> > sources directory)
>> > 4. C-h f make-overlay
>> > Now the first line in the *Help* buffer says:
>> > "make-overlay is a primitive-function in ‘C source code’."
>> > So far so good.
>> > 5. Switch to the *Help* buffer and type s, this finds buffer.c
>> > 6. Switch back to the *Help* buffer and type g
>> > The first line in *Help* now says:
>> > "make-overlay is a primitive-function in ‘/tmp/src/buffer.c’."
>> > ^^^^^ <-- Wrong!
>>
>> Hmm... my code assumed the file was always absolute, but apparently
>> that's not always the case.
>>
>> The patch below seems to give us back the previous behavior, but maybe
>> we should change some other part of the code so
>> `help-fns-short-filename` is not used at all for the C sources (since
>> it's specifically trying to give a name relative to `load-path`, which
>> is nonsensical for our C files).
>>
>>
>> Stefan
>>
>>
>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index c87c86bae84..266cf8eb4a9 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -1052,7 +1052,8 @@ help-fns--radix-trees
>> "Cache of radix-tree representation of `load-path'.")
>>
>> (defun help-fns--filename (file)
>> - (let ((f (abbreviate-file-name (expand-file-name file))))
>> + (let ((f (abbreviate-file-name
>> + (if (file-name-absolute-p file) (expand-file-name file) file))))
>> (if (file-name-case-insensitive-p f) (downcase f) f)))
>>
>> (defun help-fns--radix-tree (dirs)
>
> Ping! Can we make further progress with this bug?
I think that the workaround Stefan suggests makes sense. It doesn't
make the code (and the implicit assumptions about when a file name is
absolute and when it isn't) any clearer though. Another option may be
something like the following, where we explicitly look only for .el
files under load-path:
diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index c87c86bae84..b57201c6100 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1064,22 +1064,14 @@ help-fns--radix-tree
rt)))
(defun help-fns-short-filename (filename)
- (let* ((short (help-fns--filename filename))
- (prefixes (radix-tree-prefixes (help-fns--radix-tree load-path)
- (file-name-directory short))))
- (if (not prefixes)
- ;; The file is not inside the `load-path'.
- ;; FIXME: Here's the old code (too slow, bug#73766),
- ;; which used to try and shorten it with "../" as well.
- ;; (dolist (dir load-path)
- ;; (let ((rel (file-relative-name filename dir)))
- ;; (if (< (length rel) (length short))
- ;; (setq short rel)))
- ;; (let ((rel (file-relative-name abbrev dir)))
- ;; (if (< (length rel) (length short))
- ;; (setq short rel))))
- short
- (file-relative-name short (caar prefixes)))))
+ (cond
+ ((eq filename 'C-source) "C source code")
+ ((equal (file-name-extension filename) "el")
+ (let* ((short (help-fns--filename filename))
+ (prefixes (radix-tree-prefixes (help-fns--radix-tree load-path)
+ (file-name-directory short))))
+ (if prefixes (file-relative-name short (caar prefixes)) short)))
+ (t filename)))
(defun help-fns--analyze-function (function)
;; FIXME: Document/explain the differences between FUNCTION,
@@ -1203,10 +1195,7 @@ help-fns-function-description-header
(setq help-mode--current-data (list :symbol function)))
;; We used to add .el to the file name,
;; but that's completely wrong when the user used load-file.
- (princ (format-message " in `%s'"
- (if (eq file-name 'C-source)
- "C source code"
- (help-fns-short-filename file-name))))
+ (princ (format-message " in `%s'" (help-fns-short-filename file-name)))
;; Make a hyperlink to the library.
(with-current-buffer standard-output
(setq help-mode--current-data (list :symbol function
@@ -1399,9 +1388,7 @@ describe-variable
(progn
(princ (format-message
" is a variable defined in `%s'.\n\n"
- (if (eq file-name 'C-source)
- "C source code"
- (help-fns-short-filename file-name))))
+ (help-fns-short-filename file-name)))
(with-current-buffer standard-output
(setq help-mode--current-data
(list :symbol variable
@@ -2146,9 +2133,7 @@ describe-keymap
(princ ".\n\n"))
(princ (format-message
" defined in `%s'.\n\n"
- (if (eq file-name 'C-source)
- "C source code"
- (help-fns-short-filename file-name))))
+ (help-fns-short-filename file-name)))
(save-excursion
(re-search-backward (substitute-command-keys "`\\([^`']+\\)'"))
(setq help-mode--current-data (list :symbol keymap
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.