Re: Overalays and point-entered

[Stefan Monnier]

> 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.

But it might indeed be the case that most/all of that info will be needed.
Here are some comments on your patch:

Index: buffer.c
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.c,v
retrieving revision 1.591
diff -r1.591 buffer.c
399c399,401
< 
---

Please always use unified diff format (or context diff format).

Index: buffer.h
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.h,v
retrieving revision 1.130
diff -r1.130 buffer.h
560a561,567
>   /* Vector of overlays that were on this window the last time
>      run_point_motion_hooks was run and this window was current */
>   Lisp_Object* overlay_prev_vec;
> 
>   /* Number of overlays in overlay_prev_vec */
>   unsigned int noverlays_prev;

Only Lisp_Object fields are traced by the GC, so overlay_prev_vec as it
stands will probably lead to core dumps occasionally.

> /* Returns an array of overlays that are active at the indication position 
> and buffer.
>    The lenght of the array will be stored in num_overlays. */
             ^^
Please, state that the array is allocated with xmalloc.
 
>   if (!NILP (point_motion) &&

We like to put infix operators at the beginning rather than the end of lines.

> 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.

>       check_run_point_motion_hook (overlay_cur_vec[i],
>                                    prev_buffer,
>                                    prev_window,
>                                    Qt,
>                                    0,
>                                    0);

Please use NULL for pointers and keep 0 for integers.  Especially in
such contexts with numerous arguments, it helps figure out which
is which.

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).



        Stefan