On Wed, Sep 16, 2009 at 9:05 PM, Stefan Monnier
<address@hidden> wrote:
> I wrote a patch to add point-left and point-entered to overlays. I ended up
> adding the implementation in command_loop_1. The patch is attached - let me
> know if anything needs to be fixed.
It looks like a good starting point. Here are some comments, based on
a cursory examination of your patch:
- since the semantics are fundamentally very different from the ones of
the point-left and point-entered text properties, this new feature
should use other property names. I also expect it's simpler to use
a single property, which is called both when entering and
when leaving (like the modification-hooks property).
- a corollary is that this new feature should also be implemented for
text properties.
- the function you patch is already overly long, so better move the new
code in a new function.
- you use last_point_position without checking whether it applied to the
same buffer as the current one (i.e. you don't pay attention to
prev_buffer).
- you don't take into account the fact that the buffer may have been
changed since the beginning of the command, so last_point_position
(which is an int rather than a marker) may not point to the right
place any more.
- similarly overlays may have been added/moved/deleted, so your check
for "overlays at last_point_position" may find overlays which in
reality were not there when last_point_position was recorded (or may
fail to find the overlay(s) that were there).
- it doesn't seem easy/possible for the user to control whether a given
overlay boundary is considered to be "inside" or "outside".
I think an approach that may solve most of the above problems and yet be
somewhat simple to implement could be the following:
use a new property `motion-functions'. This property is called whenever
a command ends with point at a place where the property is different
(i.e. you compare the value of the property before the command to the
value of the property after the command). The comparison is made with
`eq' (since the property contains a list, is should be easy for elisp
authors to make it do the right thing by simply avoiding reusing the
same list, and constructing a new one instead). The property is looked
up with get_pos_property, so it automatically works for overlays as well
as text properties, and also provides ways to control what happens at
the boundaries (whether the position at the end/beginning of an
overlays is "inside" or "outside"
One problem with this approach is that if you have several overlays at
the same place with a `motion-functions' property, they'll end up
fighting each other and only one of them will work, which is kind of
a bummer. IOW, this would work well for text properties, but not so
well for overlays.
Stefan