bug#4996: problem found

bug#4996: problem found

[Mark Lillibridge]

    Ah!  I have figured out the problem.  report-emacs-bug adds the
intangible property to the instructions.  Normally, forward-line skips
past text with the intangible property and this is what happens when
linum-update is called normally, hence those lines do not receive line
numbers normally.  [UPDATE: this is a different bug in linum as it
normally reports incorrect line numbers in the presence of intangible
text; ironically, the case I was reporting is one of the few when it
reports the *correct* line numbers.  Consider this a bug report for the
behavior of the window-scroll-functions callers.]

    However, for some reason inhibit-point-motion-hooks is set to t when
the window-scroll-functions hooks are called.  This causes linum-update,
which uses forward-line, to number the intangible lines only when called
from the scrolling hook.

    The following code change, which fixes the bug, demonstrates this:
[UPDATE: this actually breaks linum further]

(defun linum-after-scroll (win start)
  (let ((old inhibit-point-motion-hooks))
    (setq inhibit-point-motion-hooks nil)
    (linum-update (window-buffer win))
    (setq inhibit-point-motion-hooks old)))


    I am not sure if the actual bug here is with the callers of
window-scroll-functions incorrectly setting inhibit-point-motion-hooks
or with linum-after-scroll (in which case, the documentation for
window-scroll-functions should mention this behavior).  If the later, a
better patch should be used that uses unwind-protect or the like.

- Mark

bug#4996: problem found

[Mark Lillibridge]

    Ok, I have a proposed patch for linum.el that solves the problem
with it displaying incorrect line numbers in the presence of intangible
text.

The original version of linum.el has:

linum.el:129:
(defun linum-update-window (win)
  "Update line numbers for the portion visible in window WIN."
  (goto-char (window-start win))
  (let ((line (line-number-at-pos))
        (limit (window-end win t))
        (fmt (cond ((stringp linum-format) linum-format)
                   ((eq linum-format 'dynamic)
                    (let ((w (length (number-to-string
                                      (count-lines (point-min) (point-max))))))
                      (concat "%" (number-to-string w) "d")))))
        (width 0))
    (run-hooks 'linum-before-numbering-hook)
    ;; Create an overlay (or reuse an existing one) for each
    ;; line visible in this window, if necessary.
    (while (and (not (eobp)) (<= (point) limit))
      (let* ((str (if fmt
                      (propertize (format fmt line) 'face 'linum)
                    (funcall linum-format line)))
             (visited (catch 'visited
                        (dolist (o (overlays-in (point) (point)))
                          (when (equal-including-properties
				 (overlay-get o 'linum-str) str)
                            (unless (memq o linum-overlays)
                              (push o linum-overlays))
                            (setq linum-available (delq o linum-available))
                            (throw 'visited t))))))
        (setq width (max width (length str)))
        (unless visited
          (let ((ov (if (null linum-available)
                        (make-overlay (point) (point))
                      (move-overlay (pop linum-available) (point) (point)))))
            (push ov linum-overlays)
            (overlay-put ov 'before-string
                         (propertize " " 'display `((margin left-margin) ,str)))
            (overlay-put ov 'linum-str str))))
      (forward-line)
      (setq line (1+ line)))
    (set-window-margins win width)))


The problem here is the third to last line,

linum.el:164:
      (forward-line)

Linum assumes this will move at most one logical line forward, when
in the presence of intangible text it can move an arbitrary number of
logical lines forward.  *BUG*


I propose changing that line to:

      (let ((old-inhibit inhibit-point-motion-hooks))
	(setq inhibit-point-motion-hooks t)
	(forward-line)
	(setq inhibit-point-motion-hooks old-inhibit))

    I have verified that this causes the correct line numbers to be
shown, both with and without being called via the scrolling hook.


Comments?

- Mark

bug#4996: problem found

[Markus Triska]

Mark Lillibridge <mark.lillibridge@hp.com> writes:

> I propose changing that line to:
>
>       (let ((old-inhibit inhibit-point-motion-hooks))
> 	(setq inhibit-point-motion-hooks t)
> 	(forward-line)
> 	(setq inhibit-point-motion-hooks old-inhibit))

I suggest:

      (let ((inhibit-point-motion-hooks t))
        (forward-line))