On Thu, Dec 10, 2009 at 3:32 AM, Stefan Monnier
<address@hidden> wrote:
> The patch is attached - please let me know if you have any comments.
I'm wondering whether it should be described as complete or complex.
Especially for a feature which hsan't yet found a user.
Yes, I didn't think it would be nearly as complex when I started. The latest revision removes the argument for whether it was called due to a buffer change; this doesn't seem as necessary when the overlay is only run once instead of multiple times.
> run_point_motion_hooks (prev_c_buffer)
This seems to run the hooks for all overlays at the starting position
and all overlays at the ending position. If you add to that the fact
that it runs them for windows and for the buffer, you get that under the
usual circumstance of a command within the same buffer displayed in
a single window, moving within the same overlay we run the hook 4 times?
I think we need to be more careful and only run the hook when crossing
the boundary, i.e. when moving out of or into an overlay with that
property. But even if we decide to run it for all movements, we should
be careful to run it a bit less liberally. It's probably OK to
occasionally have a few spurious extra runs of the hook, but your
current code needs to be a lot more careful.
The code has been fixed so that the same overlay or text property should only run once, and only if a boundary has been crossed. I'm not really sure whether it should run on all motion; either way would be fairly easy to implement.
One more thing: I think the buffer and window object should only store
the few overlays that do have a point-motion property. Maybe they could
even limit themselves to storing "the" overlay with a point-motion
property (if there are several, it should take the one with highest
priority).
It seemed to be more consistent with the priority mechanism to only store and run one, so that's what it currently does. The patch with these fixes is attached; please let me know if there are any other comments.
Thanks,
Nathaniel Flath
point-motion.patch