Subject: bug#73792: 30; Confusion in loaddefs-generate--*-prefixes

From: Jonas Bernoulli <[email-masked]>
Date: Sun, 13 Oct 2024 17:12:23 +0200

Hello,

This comment confused me

,----
| (defun loaddefs-generate--make-prefixes (defs file)
|   ;; Remove the defs that obey the rule that file foo.el (or
|   ;; foo-mode.el) uses "foo-" as prefix.  Then compute a small set of
|   ;; prefixes that cover all the remaining definitions.
|   (let* ((tree (let ((tree radix-tree-empty))
|                  (dolist (def defs)
|                    (setq tree (radix-tree-insert tree def t)))
|                  tree))
|          (prefixes nil))
`----

as I could not see anything being removed here.

Digging through history I learned this comment was lifted from older
code shown below and removed in 1d4e90341782030cc7d8c29c639450b079587908,
where it was followed by commented code that would actually have done
that.

,----
| (defun autoload--make-defs-autoload (defs file)
| 
|   ;; Remove the defs that obey the rule that file foo.el (or
|   ;; foo-mode.el) uses "foo-" as prefix.
|   ;; FIXME: help--symbol-completion-table still doesn't know how to use
|   ;; the rule that file foo.el (or foo-mode.el) uses "foo-" as prefix.
|   ;;(let ((prefix
|   ;;       (concat (substring file 0 (string-match "-mode\\'" file)) "-")))
|   ;;  (dolist (def (prog1 defs (setq defs nil)))
|   ;;    (unless (string-prefix-p prefix def)
|   ;;      (push def defs))))
| 
|   ;; Then compute a small set of prefixes that cover all the
|   ;; remaining definitions.
`----

Thus, the first sentence (and the following "Then") should be removed
from the new comment.

----
And now for something completely different... (As in "here you might
want to *add* a comment" ;P )

loaddefs-generate--compute-prefixes ignores definitions with an
autoload cookie:

,----
|             (when (save-excursion
|                     (goto-char (match-beginning 0))
|                     (or (bobp)
|                         (progn
|                           (forward-line -1)
|                           (not (looking-at ";;;###autoload")))))
|               (push name prefs))))))
`----

Why?!

I might overlook something, but it seems that the old implementation
in pre-1d4e90341782030cc7d8c29c639450b079587908 autoloads.el didn't
do that.

     Cheers,
     Jonas