On Wed, May 24, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> ...
> Could you please describe how this makes sure Info-directory-list will
> be set to a correct value, in the various scenarios that are relevant?
>
> In particular, Info-directory-list is not the right variable to tweak
> here, as it is computed by info.el.  I think we need a separate
> variable.
In the original function `info-initialize', it will rely on the value
`Info-directory-list' to indicate function initialization and parse
the "INFOPATH" or get system info dirs as the initialized value.

The bug comes for: package.el will add several paths into
`Info-directory-list', that maybe lead function `info-initialize'
return without parsing the "INFOPATH" or getting the system info dires
as initial value.

So package.el required the entire `info.el` and called function
`info-initialize' to initialize the value first (by parsing the
"INFOPATH" or get system info dir), then the package.el modify the
`Info-directory-list'.

The patch  will allow users to set the `Info-directory-list' before
calling function `info-initialize'; when info.el use the
`Info-directory-list' variable to read plain *.info file, will call
function `info-initialize' to add system info dirs.

Two functions `Info-insert-dir' and `Info-find-file', will use
(get/read) the value of `Info-directory-list' to read the plain *.info
files.
In the patch file, both functions will call the function
`info-initialize' at their entry to ensure the `Info-directory-list'
is initialized with the "INFOPATH" env string.
Users will just insert/remove dirs from/to `Info-directory-list', but
won't use the list to read/get plain *.info files.
So users can define the `Info-directory-list' and modify its value on
their elisp files.

> > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > +        (pp `(setq Info-directory-list
> > +                   (delete-dups
> > +                    (append ',info-dirs Info-directory-list)))
> > +            (current-buffer)))
>
> Using 'append' here could cause duplicate directories in
> Info-directory-list.
The `delete-dups' will avoid that.

> > +(defvar Info--initialized nil
> > +  "Non-nil if `info-initialize' has been run.")
> There's no need to capitalize the first letter of the name of an
> internal variable.  We capitalize 'I' in "Info" so that it would be
> easier to type info.el commands with completion (other commands that
> begin with 'i' use lower-case 'i').  This is not a factor for internal
> variables.
Very helpful to understand the naming rules. And rename it to
`info--initialized', new patch attached, please review again.

Best Regards
Lin