Re: Possible `point-entered' `point-left' Text Property Bug

[Chong Yidong]

> Eval in emacs -Q:
>
> (defun prop-test (old new) (message "XXX: %d %d" old new))
> (let ((buffer (generate-new-buffer "*prop tst*")))
>   (with-current-buffer buffer
>     (insert "1234567890\n1234567890\n")
>     (put-text-property (point-min) (point-max) 'point-entered 'prop-test)
>     (put-text-property (point-min) (point-max) 'point-left 'prop-test)
>     (pop-to-buffer buffer)))
>
> Now move up two lines.  You'll see the `message' for every
> line you move up.  This doesn't happen if you move backward
> character wise with C-b.
>
> As far as i understand the documentation this shouldn't
> happen for the line movement either as all chars have the
> same `point-left' and `point-entered' text-property.

As far as I can tell, the problem arises when there is no character
before point.  From the Elisp manual,

     The same comparison [between point-entered and point-left]
     is made for the characters before the old and
     new locations.  The result may be to execute two `point-left'
     functions (which may be the same function) and/or two
     `point-entered' functions (which may be the same function).  In
     any case, all the `point-left' functions are called first,
     followed by all the `point-entered' functions.

When executing previous-line and next-line, a situation can arise
where no "character before point" is found for the point left, whereas
one exists for the point entered (or vice versa).  That's why the
point-entered/left hooks are called.  I don't know why this happens
even when moving vertically with point in the middle of a line, but I
am not familiar with the point motion code.

One fix is to ignore the hook when either the point left or the point
entered does not contain a "point before".  This patch does that.
Maybe we should also do the same thing for "point after".  (The patch
is a little longer than it should be because the existing code
contains a confusion between leave_after and leave_before, which has
to be corrected.)

What do people think?

*** emacs/src/intervals.c.~1.132.~      2006-05-09 10:08:04.000000000 -0400
--- emacs/src/intervals.c       2006-05-11 12:16:46.000000000 -0400
***************
*** 2196,2231 ****
        Lisp_Object leave_after, leave_before, enter_after, enter_before;
  
        if (fromprev)
!       leave_after = textget (fromprev->plist, Qpoint_left);
        else
!       leave_after = Qnil;
        if (from)
!       leave_before = textget (from->plist, Qpoint_left);
        else
!       leave_before = Qnil;
  
        if (toprev)
!       enter_after = textget (toprev->plist, Qpoint_entered);
        else
!       enter_after = Qnil;
        if (to)
!       enter_before = textget (to->plist, Qpoint_entered);
        else
!       enter_before = Qnil;
  
!       if (! EQ (leave_before, enter_before) && !NILP (leave_before))
!       call2 (leave_before, make_number (old_position),
!              make_number (charpos));
        if (! EQ (leave_after, enter_after) && !NILP (leave_after))
!       call2 (leave_after, make_number (old_position),
!              make_number (charpos));
  
!       if (! EQ (enter_before, leave_before) && !NILP (enter_before))
!       call2 (enter_before, make_number (old_position),
!              make_number (charpos));
        if (! EQ (enter_after, leave_after) && !NILP (enter_after))
!       call2 (enter_after, make_number (old_position),
!              make_number (charpos));
      }
  }
  
--- 2196,2235 ----
        Lisp_Object leave_after, leave_before, enter_after, enter_before;
  
        if (fromprev)
!       leave_before = textget (fromprev->plist, Qpoint_left);
        else
!       leave_before = Qnil;
! 
        if (from)
!       leave_after = textget (from->plist, Qpoint_left);
        else
!       leave_after = Qnil;
  
        if (toprev)
!       enter_before = textget (toprev->plist, Qpoint_entered);
        else
!       enter_before = Qnil;
! 
        if (to)
!       enter_after = textget (to->plist, Qpoint_entered);
        else
!       enter_after = Qnil;
  
!       if (fromprev && toprev
!         && ! EQ (leave_before, enter_before) && !NILP (leave_before))
!               call2 (leave_before, make_number (old_position),
!                      make_number (charpos));
        if (! EQ (leave_after, enter_after) && !NILP (leave_after))
!               call2 (leave_after, make_number (old_position),
!                      make_number (charpos));
  
!       if (fromprev && toprev
!         && ! EQ (enter_before, leave_before) && !NILP (enter_before))
!               call2 (enter_before, make_number (old_position),
!                      make_number (charpos));
        if (! EQ (enter_after, leave_after) && !NILP (enter_after))
!               call2 (enter_after, make_number (old_position),
!                      make_number (charpos));
      }
  }