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

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

[Jonas Bernoulli via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

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

[Eli Zaretskii]

> Date: Sun, 13 Oct 2024 17:12:23 +0200
> From:  Jonas Bernoulli via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 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.

I've added people who might know the answers.