Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs

[Vincent Belaïche]

Hello Tassilo,

To Stefan also concerning add-to-list -> pushnew change...

Feedback inserted below...

----------------------------------------
> From: address@hidden
> To: address@hidden
> Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in 
> @..ref commands, and solve a few other pbs
> Date: Mon, 26 Oct 2015 14:16:20 +0100
>
> Vincent Belaïche <address@hidden> writes:
>
> Hi Vincent,
>
>> Ooops... sorry, I hadn't noticed that you also had switched to git.
>
> Hey, that has been in April 2013! :-)

Ok, I understand what has happened. I am using some script to do all the
refreshing of the work area, and installation, when I had modified this
script from SVN to GIT quite some time ago I was not so familiar with
git at that time, and I wrote `git fetch' instead of `git pull' for the
case of already existing work area...

>
>> I will do the checks ASAP and let you know.
>
> Great, thank you. I think the only hunk that didn't apply is the one
> adapting `Texinfo-insert-node' so have a special look on that.
>

I had a look at function Texinfo-make-node-list, which is the one of
interest. Some time ago Stefan Monnier changed add-to-list by pushnew, I
don't know what the motivation was, probably somebody had complained
that add-to-list was taking too much time.

However these are not functionally equivalent, pushnew will skip the
replacement only if the node name is already there on top of the list,
while add-to-list will skip it if it is already there at any place.

So, I assume that the motivation was to avoid any duplicate in the
resulting list. Anyways node names are supposed to be unique, so whether
it be pushnew or add-to-list that should make a difference only in the
case of boguous Texinfo source code

Another problem is that the user is not warned if there are duplicate
node names, why doing this check if then things go by silently.

So I propose to revise the code as follows:
(defun Texinfo-make-node-list (&optional nodes)
  ;; Build list of nodes in current buffer.
  ;; (What about using `imenu--index-alist'?)
  ;; FIXME: Support multi-file documents.
  (let ((htable (make-hash-table :test 'equal)) hentry pt key msg)
    (dolist (n nodes)
      (puthash (car n) t htable))
    (save-excursion
      (goto-char (point-min))
      (while (re-search-forward "address@hidden" nil t)
    (skip-chars-forward "[:blank:]")
    (setq key  (Texinfo-nodename-de-escape
              (buffer-substring-no-properties
               (setq pt (point)) (progn (skip-chars-forward "^\r\n,")
                      (skip-chars-backward "[:blank:]")
                      (point))))
          hentry (gethash key htable))
    (cond
     ((integerp hentry)
      (push (format "AUCTeX/Texinfo:%d: Duplicate node name `%s' at line %d" 
(line-number-at-pos hentry) key (line-number-at-pos pt)) msg))
     ((eq hentry t)
      (puthash key pt htable))
     (t
      (puthash key pt htable)
      (push (list key) nodes)))))
      (if msg (warn "AUCTeX/Texinfo...\n%s" (mapconcat 'identity (nreverse msg) 
"\n"))))
  nodes)

The changes are as follows:

- use hash table, to keep both speed advantage of pushnew and thorough
  duplicate check of add-to-list

- warn the user if a duplicate is found

> Bye,
> Tassilo

Bye,
   Vincent