bug#13438: /srv/bzr/emacs/emacs-24 r111196: * imenu.el (imenu-default-create-index-function): Remove useless

bug#13438: /srv/bzr/emacs/emacs-24 r111196: * imenu.el (imenu-default-create-index-function): Remove useless

[Glenn Morris]

>   * imenu.el (imenu-default-create-index-function): Remove useless
>   infinite loop check.
[...]
> --- a/lisp/imenu.el	2013-01-01 09:11:05 +0000
> +++ b/lisp/imenu.el	2013-01-23 21:55:46 +0000
> @@ -683,8 +683,6 @@
>  	   (goto-char (point-max))
>  	   ;; Search for the function
>  	   (while (funcall imenu-prev-index-position-function)
> -             (when (= pos (point))
> -               (error "Infinite loop at %s:%d: imenu-prev-index-position-function does not move point" (buffer-name) pos))
>               (setq pos (point))
>  	     (save-excursion
>  	       (setq name (funcall imenu-extract-index-name-function)))


I'm not keen on removing an infinite loop check from emacs-24 at this
stage. Why is this issue (apparently) only seen in Python mode, and why
isn't there a Python-specific solution (which seems like it would be
just making imenu-prev-index-position-function return nil when
appropriate)?

Related discussion:
http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00536.html

bug#13438: /srv/bzr/emacs/emacs-24 r111196: * imenu.el (imenu-default-create-index-function): Remove useless

[Stefan Monnier]

> I'm not keen on removing an infinite loop check from emacs-24 at this
> stage.

The patch is "safe" in the sense that it will only introduce
an inf-loop in those cases where the user used to get an error.
[ Unless of course, the calling context catches those errors.  ]

> Why is this issue (apparently) only seen in Python mode, and why
> isn't there a Python-specific solution (which seems like it would be
> just making imenu-prev-index-position-function return nil when
> appropriate)?

If we can change python.el to work around the problem, we could install
that workaround in emacs-24 and keep the "remove the inf-loop check" for
the trunk, indeed.


        Stefan

bug#13438: /srv/bzr/emacs/emacs-24 r111196: * imenu.el (imenu-default-create-index-function): Remove useless

[Glenn Morris]

>> Why is this issue (apparently) only seen in Python mode

Answering my own question, it happens in any mode that sets
imenu-prev-index-position-function and
imenu-extract-index-name-function, but there are very few of those.

Eg prolog mode. With buffer contents:

------
fac(0,1).
fac(N,F) :- N > 0, M is N - 1,
       fac(M,Fm), F is N * Fm.
------

and point at point-min, switching to prolog-mode causes the same issue.

How about taking the more cautious approach with:

(when (and (= pos (point))
           (not (bobp)))
   (error "Infinite loop... ))

(IIUC, this was actually the suggestion in
http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00510.html ?)

bug#13438: /srv/bzr/emacs/emacs-24 r111196: * imenu.el (imenu-default-create-index-function): Remove useless

[Fabián Ezequiel Gallina]

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]

On 01/25/2013 05:34 AM, Glenn Morris wrote:
>>> Why is this issue (apparently) only seen in Python mode
> Answering my own question, it happens in any mode that sets
> imenu-prev-index-position-function and
> imenu-extract-index-name-function, but there are very few of those.
>
> Eg prolog mode. With buffer contents:
>
> ------
> fac(0,1).
> fac(N,F) :- N > 0, M is N - 1,
>         fac(M,Fm), F is N * Fm.
> ------
>
> and point at point-min, switching to prolog-mode causes the same issue.
>
> How about taking the more cautious approach with:
>
> (when (and (= pos (point))
>             (not (bobp)))
>     (error "Infinite loop... ))
>
> (IIUC, this was actually the suggestion in
> http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00510.html ?)
I ran into the same thing, and checking bobp is not enough, try adding a 
newline at the beginning of file to the recipe and call imenu with point 
at beginning of defun and you'll see the infinite error check will trigger.

I proposed the following patch that works:

=== modified file 'lisp/imenu.el'
--- lisp/imenu.el       2013-01-01 09:11:05 +0000
+++ lisp/imenu.el       2013-01-22 18:24:45 +0000
@@ -683,7 +683,14 @@
            (goto-char (point-max))
            ;; Search for the function
            (while (funcall imenu-prev-index-position-function)
-             (when (= pos (point))
+             (when (and (= pos (point))
+                        (= pos
+                           (save-excursion
+                             ;; The infinite loop is true if there's
+                             ;; another index position but point keeps
+                             ;; itself in the same place. bug#13438
+                             (if (funcall imenu-prev-index-position-
function)
+                                 (point) 0))))

But finally after some mails with Stefan we decided the best thing will 
be to remove this check altogether.


[-- Attachment #2: Type: text/html, Size: 3440 bytes --]

bug#13438: /srv/bzr/emacs/emacs-24 r111196: * imenu.el (imenu-default-create-index-function): Remove useless

[Glenn Morris]

Oh, so the actual issue is: if you call
imenu-default-create-index-function with point at the start of the last
defun in the buffer. It goes to point-max, then back to the start of the
defun, and mistakenly thinks it is inflooping.

How about the following, which only signals an error if
imenu-prev-index-position-function returns non-nil twice in a row
without moving point.

BTW, neither python-nav-beginning-of-defun or
python-imenu-prev-index-position will move past the start of the first
defun in a buffer, if the defun begins past point-min (ie, if there are
blank lines before the first defun, it does not move over them to
point-min). I don't know if this matters, but it is inconsistent with eg
Prolog and Emacs Lisp modes.


*** lisp/imenu.el	2013-01-23 21:55:46 +0000
--- lisp/imenu.el	2013-01-27 02:02:39 +0000
***************
*** 678,688 ****
    ;; in these major modes.  But save that change for later.
    (cond ((and imenu-prev-index-position-function
  	      imenu-extract-index-name-function)
! 	 (let ((index-alist '()) (pos (point))
  	       name)
  	   (goto-char (point-max))
  	   ;; Search for the function
  	   (while (funcall imenu-prev-index-position-function)
               (setq pos (point))
  	     (save-excursion
  	       (setq name (funcall imenu-extract-index-name-function)))
--- 678,690 ----
    ;; in these major modes.  But save that change for later.
    (cond ((and imenu-prev-index-position-function
  	      imenu-extract-index-name-function)
! 	 (let ((index-alist '()) (pos -1)
  	       name)
  	   (goto-char (point-max))
  	   ;; Search for the function
  	   (while (funcall imenu-prev-index-position-function)
+              (when (= pos (point))
+                (error "Infinite loop at %s:%d: imenu-prev-index-position-function does not move point" (buffer-name) pos))
               (setq pos (point))
  	     (save-excursion
  	       (setq name (funcall imenu-extract-index-name-function)))