Overalays and point-entered

[Nathaniel Flath]

Is there any particular reason that overlays don't have point-entered and point-left properties like text does?  I ended up wanting this functionality in some code I was writing, and was wondering if this was something that had been discussed before and decided against or if it just hasn't been implemented yet.

Thanks,
Nathaniel Flath

[Stefan Monnier]

> Is there any particular reason that overlays don't have point-entered and
> point-left properties like text does?

Not that I know.  The only tricky part I can think of is what to do with
overlays that have a `window' property.

> I ended up wanting this functionality in some code I was writing, and
> was wondering if this was something that had been discussed before and
> decided against or if it just hasn't been implemented yet.

Care to describe your case?  Such point-motion hooks tend to be pretty
difficult to use.


        Stefan

[Nathaniel Flath]

Using help-at-pt and setting the delay to 0 does end up doing what I want, thanks.  I agree that this is a globally useful feature, which is why I was thinking of working to fix it at a lower level instead of just hacking around the deficiencies.  Help-at-pt isn't the most elegant of solutions either, but as long as it's part of emacs I suppose I'm fine with it.

On Thu, Sep 10, 2009 at 8:04 PM, Stefan Monnier <monnier@...> wrote:

> Essentially, I wanted to add echo-area messages to Flymake when the point
> was in a section highlighted as an error.  js2-mode does this using text
> properties when it defines overlays, which is another option I could look
> at, but it seems much better to be able to just have an overlay for all of
> this related information.  Flymake does use tooltips to display th error
> message, but those seem to be only able to display when the mouse is over
> the text, unless I'm missing something.

This is a globally useful feature.  You might like to take a look at
help-at-pt.el.  Notice that the feature you're trying to implement does
not require hooking into the point-motion itself (like
point-enter/point-leave/intangible properties) but only into the cursor
movement (i.e. the position of point betwen commands).  This is an
important difference, since one is implemented at a very low level and
affects all kinds of unrelated operations, whereas the other is
implemented at a very high level and interacts with very little
other code.


       Stefan

[Nathaniel Flath]

I started working on implementing these properties for overlays in case I end up wanting to do something else with them.  If these are properties of overlays and not just solving my specific case, should they be added to point-motion instead of cursor-motion?  This shouldn't have any adverse effects, unless people have been adding point-entered and point-left properties to their overlays.  I'd think that the hooks should be run only if in the window specified, if the window property is set.  Another question should be whether to just use inhibit-point-motion-hooks or add a new variable to inhibit overlay hooks, but it seems logical to use inhibit-point-motion-hooks.   

Thanks,
Nathan

On Thu, Sep 10, 2009 at 9:08 PM, Nathaniel Flath <flat0103@...> wrote:

Using help-at-pt and setting the delay to 0 does end up doing what I want, thanks.  I agree that this is a globally useful feature, which is why I was thinking of working to fix it at a lower level instead of just hacking around the deficiencies.  Help-at-pt isn't the most elegant of solutions either, but as long as it's part of emacs I suppose I'm fine with it.

On Thu, Sep 10, 2009 at 8:04 PM, Stefan Monnier <monnier@...> wrote:

> Essentially, I wanted to add echo-area messages to Flymake when the point
> was in a section highlighted as an error.  js2-mode does this using text
> properties when it defines overlays, which is another option I could look
> at, but it seems much better to be able to just have an overlay for all of
> this related information.  Flymake does use tooltips to display th error
> message, but those seem to be only able to display when the mouse is over
> the text, unless I'm missing something.

This is a globally useful feature.  You might like to take a look at
help-at-pt.el.  Notice that the feature you're trying to implement does
not require hooking into the point-motion itself (like
point-enter/point-leave/intangible properties) but only into the cursor
movement (i.e. the position of point betwen commands).  This is an
important difference, since one is implemented at a very low level and
affects all kinds of unrelated operations, whereas the other is
implemented at a very high level and interacts with very little
other code.


       Stefan

[Stefan Monnier]

> I started working on implementing these properties for overlays in case I
> end up wanting to do something else with them.  If these are properties of
> overlays and not just solving my specific case, should they be added to
> point-motion instead of cursor-motion?  This shouldn't have any adverse
> effects, unless people have been adding point-entered and point-left
> properties to their overlays.  I'd think that the hooks should be run only
> if in the window specified, if the window property is set.  Another question
> should be whether to just use inhibit-point-motion-hooks or add a new
> variable to inhibit overlay hooks, but it seems logical to use
> inhibit-point-motion-hooks.

inhibit-point-motion-hooks should definitely inhibit it.
But really, such low-level hooks have proved to be terribly difficult to
use (because they affect too many low-level commands in ways which
break obvious and intuitive assumptions), so I'm not too happy to
improve support for them.  I'd rather move towards obsoleting them.

OTOH if you want to implement a new kind of hook that reacts to cursor
motion rather than point motion, that would be OK.


        Stefan

[Nathaniel Flath]

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.

Thanks,
Nathaniel Flath

On Mon, Sep 14, 2009 at 3:22 PM, Stefan Monnier <monnier@...> wrote:

> By cursor motion, do you mean such as the cursor_to function in terminal.c?

No, not exactly.  I mean more generally to trigger the code based on the
motion that happen from command to command.  It could mean a hook in
something like cursor_to, but it could also mean a hook in
command_loop_1 (along the lines of what's done with
adjust_point_for_property.

> Adding these hook to that function seems to behave similarly to when I add

> them to the point motion commands,

The different between point-motion and cursor-motion can only be seen
for operations that do a lot of internal movement.
E.g. diff-context->unified.  Point-motion hooks will be triggered many
times during a run of diff-context->unified (and may accidentally cause
it to fail) whereas cursor motion hooks should only be triggered once
after the command is done, so it can't cause it to fail.

> in that modes or commands that do not appear to move the cursor( any
> mode that traverses the buffer) will still trigger the hooks.

If such a hook is run a bit more often than strictly needed, it's
usually not considered a bug (tho it is recognized as a misfeature).
So it might still be acceptable.


       Stefan

[overlay-properties.patch]

---

--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1970,5 +1970,63 @@ command_loop_1 ()

     finalize:

+      /* Run point-entered and point-left hooks for overlays */
+      if (NILP (Vinhibit_point_motion_hooks))
+        {
+          int i, j;
+          int noverlays_cur, noverlays_prev;
+          Lisp_Object *overlay_cur_vec;
+          Lisp_Object *overlay_prev_vec;
+          extern Lisp_Object Qpoint_entered, Qpoint_left, Qwindow;
+          Lisp_Object point_left, point_entered;
+          Lisp_Object overlay_window;
+
+          GET_OVERLAYS_AT (current_buffer->pt,
+                           overlay_cur_vec,
+                           noverlays_cur,
+                           NULL,
+                           0);
+          GET_OVERLAYS_AT (last_point_position,
+                           overlay_prev_vec,
+                           noverlays_prev,
+                           NULL,
+                           0);
+
+          /* Go through each overlay that overlapped last_point_position and
+             run it's point_left hook only if the current location is not
+             overlapped by that overlay. */
+          for (i = 0; i < noverlays_prev; i++)
+            {
+              for (j = 0; j < noverlays_cur; j++)
+                {
+                  if (overlay_prev_vec[i] == overlay_cur_vec[j])
+                    goto next_overlay;
+                }
+              point_left = Foverlay_get (overlay_prev_vec[i], Qpoint_left);
+              overlay_window = Foverlay_get (overlay_prev_vec[i], Qwindow);
+              if (!NILP (point_left)
+                  && (NILP (overlay_window)
+                      || !NILP (Feq (overlay_window, Fselected_window()))))
+                call2 (point_left,
+                       make_number (last_point_position),
+                       make_number (current_buffer->pt));
+
+            next_overlay:
+              i = i;
+            }
+
+          /* Run point_entered functions for all overlays overlapping the
+             current point */
+          for (i = 0; i < noverlays_cur; i++)
+            {
+              point_entered = Foverlay_get (overlay_cur_vec[i], Qpoint_entered);
+              overlay_window = Foverlay_get (overlay_cur_vec[i], Qwindow);
+              if (!NILP (point_entered)
+                  && (NILP (overlay_window)
+                      || !NILP (Feq (overlay_window, Fselected_window()))))
+                call2 (point_entered, make_number (last_point_position), make_number (current_buffer->pt));
+            }
+        }
+
       if (current_buffer == prev_buffer
   && last_point_position != PT
   && NILP (Vdisable_point_adjustment)

---

[Stefan Monnier]

> 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

[Nathaniel Flath]

On Wed, Sep 16, 2009 at 9:05 PM, Stefan Monnier <monnier@...> 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

Since the main point I was implementing this was so I could add these properties to overlay, I'd prefer a solution that works better with them.  Would it be better to store overlays at point at the end of this loop, and use this instead of looking up old points?  This would also require storing the new propertu of the text at the current-location.  To solve the boundary-control issue, I could look at get-pos-property and use the same mechanism it does for controlling boundary behaviour.

Thanks for the feedback, and sorry it took me so long to respond- I just got back to school and had a few reports I needed to write.

Thanks,
Nathaniel Flath

[Stefan Monnier]

> Since the main point I was implementing this was so I could add these
> properties to overlay, I'd prefer a solution that works better with them.

Yes, I understand that, and I agree it should work well with overlays
as well.

> Would it be better to store overlays at point at the end of this loop, and
> use this instead of looking up old points?

Might be, yes.

> To solve the boundary-control issue, I could look at get-pos-property
> and use the same mechanism it does for controlling boundary behaviour.

Yes, you'd need to implement a get-overlays-at-pos.


        Stefan

[Stephen J. Turnbull]

Stefan Monnier writes:

 > Yes, you'd need to implement a get-overlays-at-pos.

This is non-trivial to implement efficiently.  The XEmacs
implementation is so (perhaps unnecessarily) complex that Richard once
wrote:

    I looked at the Lucid Intervals code, intending to merge it in, but
    changed my mind because I couldn't understand it.

(Presumably this should be understood as a cost vs. benefit tradeoff,
not an absolute impossibility, of course.)

It may not be as hard as Jamie? and Ben have made it, but I assure you
performance of this function will be important.  The best I've been
able to think of in alternative implementations is (I hope) O(log
buffersize) for this function, but the space cost is quite large,
O(buffersize).  (The constant for XEmacs, which implements text
properties using these objects, is at least 0.25 in regex.c in CC Mode
on a 32-bit machine, and would be nearly twice as much on a 64-bit
machine because the additional data structures contain many pointers.)

[Nathaniel Flath]

On Wed, Sep 23, 2009 at 4:55 PM, Stefan Monnier <monnier@...> wrote:

> Since the main point I was implementing this was so I could add these
> properties to overlay, I'd prefer a solution that works better with them.

Yes, I understand that, and I agree it should work well with overlays
as well.

> Would it be better to store overlays at point at the end of this loop, and
> use this instead of looking up old points?

Might be, yes.

> To solve the boundary-control issue, I could look at get-pos-property
> and use the same mechanism it does for controlling boundary behaviour.

Yes, you'd need to implement a get-overlays-at-pos.
       Stefan

Actually, instead of this, why not add an extra optional argument that defaults to nil to get-pos-property?  If the argument is non-nil, get-pos-property would return a list of the values named property at pos instead of just one of them.  This way wouldn't  duplicate boundary-checking functionality.

Thanks,
Nathaniel Flath

[Nathaniel Flath]

Never mind, since get_pos_property is a C function and not a Lisp one this probably wouldn't work.

On Thu, Sep 24, 2009 at 9:47 AM, Nathaniel Flath <flat0103@...> wrote:

On Wed, Sep 23, 2009 at 4:55 PM, Stefan Monnier <monnier@...> wrote:

> Since the main point I was implementing this was so I could add these
> properties to overlay, I'd prefer a solution that works better with them.

Yes, I understand that, and I agree it should work well with overlays
as well.

> Would it be better to store overlays at point at the end of this loop, and
> use this instead of looking up old points?

Might be, yes.

> To solve the boundary-control issue, I could look at get-pos-property
> and use the same mechanism it does for controlling boundary behaviour.

Yes, you'd need to implement a get-overlays-at-pos.
       Stefan

Actually, instead of this, why not add an extra optional argument that defaults to nil to get-pos-property?  If the argument is non-nil, get-pos-property would return a list of the values named property at pos instead of just one of them.  This way wouldn't  duplicate boundary-checking functionality.

Thanks,
Nathaniel Flath

[Stefan Monnier]

> Never mind, since get_pos_property is a C function and not a Lisp one this
> probably wouldn't work.

That's OK.  It can still return a list, or else an array.
But if it can return the overlays rather than the property's values,
then you could pass the overlay back to the hook functions, which would
probably be convnient for those functions.


        Stefan

[Stefan Monnier]

>> Yes, you'd need to implement a get-overlays-at-pos.
> This is non-trivial to implement efficiently.  The XEmacs
> implementation is so (perhaps unnecessarily) complex that Richard once
> wrote:

It shouldn't be difficult to change the get_pos_property function we
already have to return the list of relevant overlays.

Our overlay code is not efficient, but it's indeed not easy to make it
significantly better.


        Stefan

[Nathaniel Flath]

Sorry for taking so long on this - I ended up being pretty busy with school and couldn't work on it for a while.

I added a function get_overlays_at_pos which will return a list of overlays active at the given position and buffer - the design is based on get_pos_property.  Then at the end of command_loop_1,  run_point_motion_hooks is called which will retrieve the overlays at the current position, run the 'point-motion' property with the old point, new point, and overlay, and then run property for all overlays which were just executed.  The 'point-motion property is also executed for the text property at current point, and the previous point's text-property if it is different from the one at current point.

The diff for this is below - let me know your thoughts.

Thanks,
Nathaniel Flath

diff --git a/src/editfns.c b/src/editfns.c
index e52c3c2..7f343f0 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -499,6 +499,48 @@ get_pos_property (position, prop, object)
     }
 }
 
+/* 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. */
+
+Lisp_Object*
+get_overlays_at_pos (position, buffer, num_overlays )
+     Lisp_Object position, buffer;
+     int* num_overlays;
+{
+  CHECK_NUMBER_COERCE_MARKER (position);
+
+  if (NILP (buffer))
+    XSETBUFFER (buffer, current_buffer);
+
+  int posn = XINT (position);
+  int i, noverlays;
+  Lisp_Object* overlay_vec;
+  struct buffer *obuf = current_buffer;
+  set_buffer_temp (XBUFFER (buffer));
+
+  noverlays = overlays_around (posn, overlay_vec, 0);
+  overlay_vec = xmalloc (sizeof(Lisp_Object) * noverlays);
+  noverlays = overlays_around (posn, overlay_vec, noverlays);
+  noverlays = sort_overlays (overlay_vec, noverlays, NULL);
+
+  set_buffer_temp (obuf);
+  for (i = 0; i < noverlays; i++)
+    {
+      Lisp_Object ol = overlay_vec[i];
+      Lisp_Object start = OVERLAY_START (ol), finish = OVERLAY_END (ol);
+      if ((OVERLAY_POSITION (start) == posn
+           && XMARKER (start)->insertion_type == 1)
+          || (OVERLAY_POSITION (finish) == posn
+              && XMARKER (finish)->insertion_type == 0))
+        {
+          overlay_vec[i] = overlay_vec[noverlays];
+          noverlays--; i--;
+        }
+    }
+  *num_overlays = noverlays;
+  return overlay_vec;
+}
+
 /* Find the field surrounding POS in *BEG and *END.  If POS is nil,
    the value of point is used instead.  If BEG or END is null,
    means don't store the beginning or end of the field.




diff --git a/src/textprop.c b/src/textprop.c
index 0018088..5708040 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -53,6 +53,7 @@ Lisp_Object Qpoint_left;
 Lisp_Object Qpoint_entered;
 Lisp_Object Qcategory;
 Lisp_Object Qlocal_map;
+Lisp_Object Qpoint_motion;
 
 /* Visual properties text (including strings) may have.  */
 Lisp_Object Qforeground, Qbackground, Qfont, Qunderline, Qstipple;
@@ -2348,6 +2349,9 @@ inherits it if NONSTICKINESS is nil.  The `front-sticky' and
   Qpoint_left = intern ("point-left");
   staticpro (&Qpoint_entered);
   Qpoint_entered = intern ("point-entered");
+  staticpro (&Qpoint_entered);
+  Qpoint_motion = intern ("point-motion");
+
 
   defsubr (&Stext_properties_at);
   defsubr (&Sget_text_property);




diff --git a/src/keyboard.c b/src/keyboard.c
index 35c338c..a375daf 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1989,6 +1993,93 @@ command_loop_1 ()
     }
 }
 
+/*  Runs 'point-motion hooks on text properties and overlays.*/
+
+void
+run_point_motion_hooks ()
+{
+  static Lisp_Object* overlay_prev_vec;
+  static Lisp_Object prev_text_prop;
+  static int noverlays_prev;
+
+  int i, j, noverlays_cur;
+  Lisp_Object *overlay_cur_vec;
+  Lisp_Object point_motion, overlay_window;
+  extern Lisp_Object Qpoint_motion, Qwindow;
+
+  /* Retrieves vector of overlays in current location and runs 'point-motion
+     hook for those whose 'window property allows it to be displayed */
+  overlay_cur_vec = get_overlays_at_pos (make_number (current_buffer->pt),
+                                         current_buffer,
+                                         &noverlays_cur);
+  for (i = 0; i < noverlays_cur; i++)
+    {
+      point_motion = Foverlay_get (overlay_cur_vec[i], Qpoint_motion);
+      overlay_window = Foverlay_get (overlay_cur_vec[i], Qwindow);
+      if (!NILP (point_motion) &&
+          (NILP (overlay_window)
+           || !NILP (Feq (overlay_window, Fselected_window()))))
+        {
+          call3 (point_motion,
+                 make_number (last_point_position),
+                 make_number (current_buffer->pt),
+                 overlay_cur_vec[i]);
+        }
+    }
+
+  /* Runs hooks for all overlays that the point used to be in but no longer is */
+  for (i = 0; i < noverlays_prev; i++)
+      {
+        point_motion = Foverlay_get (overlay_prev_vec[i], Qpoint_motion);
+        overlay_window = Foverlay_get (overlay_prev_vec[i], Qwindow);
+        if (!NILP (point_motion) &&
+            (NILP (overlay_window)
+             || !NILP (Feq (overlay_window, Fselected_window()))))
+          {
+            for (j = 0; noverlays_cur; j++) {
+              if (!NILP (Feq (overlay_prev_vec[i], overlay_cur_vec[j])))
+                goto next;
+            }
+
+            call3 (point_motion,
+                   make_number (last_point_position),
+                   make_number (current_buffer->pt),
+                   overlay_prev_vec[i]);
+          next: i=i;
+          }
+      }
+
+  /* Runs hook for current text property */
+  point_motion = Fget_text_property (make_number (current_buffer->pt),
+                                     Qpoint_motion,
+                                     Qnil);
+  if (!NILP (point_motion))
+    {
+      call3 (point_motion,
+             make_number (last_point_position),
+             make_number (current_buffer->pt),
+             Fcurrent_buffer());
+    }
+
+  /* Runs hook for previous text property if it is different than the current text property */
+  if (prev_text_prop != 0 && !NILP (prev_text_prop)) {
+    if (NILP (Feq (prev_text_prop, point_motion)))
+      {
+        call3 (prev_text_prop,
+               make_number (last_point_position),
+               make_number (current_buffer->pt),
+               Fcurrent_buffer());
+      }
+  }
+
+  prev_text_prop = point_motion;
+
+  /* Frees previous overlays and sets them to the current list */
+  free (overlay_prev_vec);
+  overlay_prev_vec = overlay_cur_vec;
+  noverlays_prev = noverlays_cur;
+}
+
 extern Lisp_Object Qcomposition, Qdisplay;
 
 /* Adjust point to a boundary of a region that has such a property

On Thu, Sep 24, 2009 at 10:26 AM, Stefan Monnier <monnier@...> wrote:

> Never mind, since get_pos_property is a C function and not a Lisp one this
> probably wouldn't work.

That's OK.  It can still return a list, or else an array.
But if it can return the overlays rather than the property's values,
then you could pass the overlay back to the hook functions, which would
probably be convnient for those functions.


       Stefan

[Nathaniel Flath]

Any comments ont his?

Thanks,
Nathaniel Flath

On Tue, Oct 6, 2009 at 2:33 PM, Nathaniel Flath <flat0103@...> wrote:

Sorry for taking so long on this - I ended up being pretty busy with school and couldn't work on it for a while.

I added a function get_overlays_at_pos which will return a list of overlays active at the given position and buffer - the design is based on get_pos_property.  Then at the end of command_loop_1,  run_point_motion_hooks is called which will retrieve the overlays at the current position, run the 'point-motion' property with the old point, new point, and overlay, and then run property for all overlays which were just executed.  The 'point-motion property is also executed for the text property at current point, and the previous point's text-property if it is different from the one at current point.

The diff for this is below - let me know your thoughts.

Thanks,
Nathaniel Flath

diff --git a/src/editfns.c b/src/editfns.c
index e52c3c2..7f343f0 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -499,6 +499,48 @@ get_pos_property (position, prop, object)
     }
 }
 
+/* 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. */
+
+Lisp_Object*
+get_overlays_at_pos (position, buffer, num_overlays )
+     Lisp_Object position, buffer;
+     int* num_overlays;
+{
+  CHECK_NUMBER_COERCE_MARKER (position);
+
+  if (NILP (buffer))
+    XSETBUFFER (buffer, current_buffer);
+
+  int posn = XINT (position);
+  int i, noverlays;
+  Lisp_Object* overlay_vec;
+  struct buffer *obuf = current_buffer;
+  set_buffer_temp (XBUFFER (buffer));
+
+  noverlays = overlays_around (posn, overlay_vec, 0);
+  overlay_vec = xmalloc (sizeof(Lisp_Object) * noverlays);
+  noverlays = overlays_around (posn, overlay_vec, noverlays);
+  noverlays = sort_overlays (overlay_vec, noverlays, NULL);
+
+  set_buffer_temp (obuf);
+  for (i = 0; i < noverlays; i++)
+    {
+      Lisp_Object ol = overlay_vec[i];
+      Lisp_Object start = OVERLAY_START (ol), finish = OVERLAY_END (ol);
+      if ((OVERLAY_POSITION (start) == posn
+           && XMARKER (start)->insertion_type == 1)
+          || (OVERLAY_POSITION (finish) == posn
+              && XMARKER (finish)->insertion_type == 0))
+        {
+          overlay_vec[i] = overlay_vec[noverlays];
+          noverlays--; i--;
+        }
+    }
+  *num_overlays = noverlays;
+  return overlay_vec;
+}
+
 /* Find the field surrounding POS in *BEG and *END.  If POS is nil,
    the value of point is used instead.  If BEG or END is null,
    means don't store the beginning or end of the field.




diff --git a/src/textprop.c b/src/textprop.c
index 0018088..5708040 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -53,6 +53,7 @@ Lisp_Object Qpoint_left;
 Lisp_Object Qpoint_entered;
 Lisp_Object Qcategory;
 Lisp_Object Qlocal_map;
+Lisp_Object Qpoint_motion;
 
 /* Visual properties text (including strings) may have.  */
 Lisp_Object Qforeground, Qbackground, Qfont, Qunderline, Qstipple;
@@ -2348,6 +2349,9 @@ inherits it if NONSTICKINESS is nil.  The `front-sticky' and
   Qpoint_left = intern ("point-left");
   staticpro (&Qpoint_entered);
   Qpoint_entered = intern ("point-entered");
+  staticpro (&Qpoint_entered);
+  Qpoint_motion = intern ("point-motion");
+
 
   defsubr (&Stext_properties_at);
   defsubr (&Sget_text_property);




diff --git a/src/keyboard.c b/src/keyboard.c
index 35c338c..a375daf 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1989,6 +1993,93 @@ command_loop_1 ()
     }
 }
 
+/*  Runs 'point-motion hooks on text properties and overlays.*/
+
+void
+run_point_motion_hooks ()
+{
+  static Lisp_Object* overlay_prev_vec;
+  static Lisp_Object prev_text_prop;
+  static int noverlays_prev;
+
+  int i, j, noverlays_cur;
+  Lisp_Object *overlay_cur_vec;
+  Lisp_Object point_motion, overlay_window;
+  extern Lisp_Object Qpoint_motion, Qwindow;
+
+  /* Retrieves vector of overlays in current location and runs 'point-motion
+     hook for those whose 'window property allows it to be displayed */
+  overlay_cur_vec = get_overlays_at_pos (make_number (current_buffer->pt),
+                                         current_buffer,
+                                         &noverlays_cur);
+  for (i = 0; i < noverlays_cur; i++)
+    {
+      point_motion = Foverlay_get (overlay_cur_vec[i], Qpoint_motion);
+      overlay_window = Foverlay_get (overlay_cur_vec[i], Qwindow);
+      if (!NILP (point_motion) &&
+          (NILP (overlay_window)
+           || !NILP (Feq (overlay_window, Fselected_window()))))
+        {
+          call3 (point_motion,
+                 make_number (last_point_position),
+                 make_number (current_buffer->pt),
+                 overlay_cur_vec[i]);
+        }
+    }
+
+  /* Runs hooks for all overlays that the point used to be in but no longer is */
+  for (i = 0; i < noverlays_prev; i++)
+      {
+        point_motion = Foverlay_get (overlay_prev_vec[i], Qpoint_motion);
+        overlay_window = Foverlay_get (overlay_prev_vec[i], Qwindow);
+        if (!NILP (point_motion) &&
+            (NILP (overlay_window)
+             || !NILP (Feq (overlay_window, Fselected_window()))))
+          {
+            for (j = 0; noverlays_cur; j++) {
+              if (!NILP (Feq (overlay_prev_vec[i], overlay_cur_vec[j])))
+                goto next;
+            }
+
+            call3 (point_motion,
+                   make_number (last_point_position),
+                   make_number (current_buffer->pt),
+                   overlay_prev_vec[i]);
+          next: i=i;
+          }
+      }
+
+  /* Runs hook for current text property */
+  point_motion = Fget_text_property (make_number (current_buffer->pt),
+                                     Qpoint_motion,
+                                     Qnil);
+  if (!NILP (point_motion))
+    {
+      call3 (point_motion,
+             make_number (last_point_position),
+             make_number (current_buffer->pt),
+             Fcurrent_buffer());
+    }
+
+  /* Runs hook for previous text property if it is different than the current text property */
+  if (prev_text_prop != 0 && !NILP (prev_text_prop)) {
+    if (NILP (Feq (prev_text_prop, point_motion)))
+      {
+        call3 (prev_text_prop,
+               make_number (last_point_position),
+               make_number (current_buffer->pt),
+               Fcurrent_buffer());
+      }
+  }
+
+  prev_text_prop = point_motion;
+
+  /* Frees previous overlays and sets them to the current list */
+  free (overlay_prev_vec);
+  overlay_prev_vec = overlay_cur_vec;
+  noverlays_prev = noverlays_cur;
+}
+
 extern Lisp_Object Qcomposition, Qdisplay;
 
 /* Adjust point to a boundary of a region that has such a property

On Thu, Sep 24, 2009 at 10:26 AM, Stefan Monnier <monnier@...> wrote:

> Never mind, since get_pos_property is a C function and not a Lisp one this
> probably wouldn't work.

That's OK.  It can still return a list, or else an array.
But if it can return the overlays rather than the property's values,
then you could pass the overlay back to the hook functions, which would
probably be convnient for those functions.


       Stefan

[Stefan Monnier]

> Any comments ont his?

I haven't had much time to look into it, but I wonder: what happens when
you switch buffer?

I get the impression that your code currently will consider
a buffer-switch as a kind of cursor movement to "very far away" (so it
will run the leave&enter hooks).  I think it would be better to keep
track of overlay_prev_vec as a per-buffer (or probably better
per-window, tho that again introduces some questions when
a window-buffer is changed) information.


        Stefan

[Nathaniel Flath]

I was working to implement your proposed solution - it seemed most logical to place overlay_prev_vec in the buffer struct, but placing the three necessary properties( overlay_prev_vec, noverlays_prev, prev_point_motion_hook ) in the structure caused the 'make' process to return a segfault.  I'm going to look into this more in a few days, but do you have any idea why this would be happening, or another implementation strategy that wouldn't run into this problem?  I followed the instructions about placing non-Lisp_Objects above the 'name' variable, so that is most likely not the issue.

The last few lines produced by running make were:

LC_ALL=C `/bin/pwd`/temacs -batch -l loadup dump
Segmentation fault
make: *** [emacs] Error 139

Thanks,
Nathaniel Flath

On Sat, Oct 17, 2009 at 9:09 PM, Stefan Monnier <monnier@...> wrote:

> Any comments ont his?

I haven't had much time to look into it, but I wonder: what happens when
you switch buffer?

I get the impression that your code currently will consider
a buffer-switch as a kind of cursor movement to "very far away" (so it
will run the leave&enter hooks).  I think it would be better to keep
track of overlay_prev_vec as a per-buffer (or probably better
per-window, tho that again introduces some questions when
a window-buffer is changed) information.


       Stefan

[Stefan Monnier]

> I was working to implement your proposed solution - it seemed most logical
> to place overlay_prev_vec in the buffer struct, but placing the three
> necessary properties( overlay_prev_vec, noverlays_prev,
> prev_point_motion_hook ) in the structure caused the 'make' process to
> return a segfault.

That was what I expected, yes.  But the more I think about it, the more
it seems it should be a property linked to a window rather than to
a buffer.  I think this deserves thought first.

- What should happen if the same buffer is shown in the windows and the
  user switches from one to the other?  Should the hooks be run at every
  window-switch, even though no cursor moves?
- What should happen if a buffer is display in a window and then the
  user does C-x b: should the hooks be run?

IIUC, if the data is per-window, then the answers will be "no; yes", if
it's per-buffer, then the answers will be "yes; no".

Whenm thinking about it, it's best to try and think of concrete examples
which would use this feature.  And keep in mind that it's usually better
for a hook to be run too many times than too few times (the code that's
run too many times, can try and detect the extra times and do nothing
in those cases).

> I'm going to look into this more in a few days, but do
> you have any idea why this would be happening, or another implementation
> strategy that wouldn't run into this problem?  I followed the instructions
> about placing non-Lisp_Objects above the 'name' variable, so that is most
> likely not the issue.

The slot placement would have been my first thought, otherwise,
I can't think of anything particularly likely.


        Stefan

[Nathaniel Flath]

On Thu, Oct 22, 2009 at 11:37 AM, Stefan Monnier <monnier@...> wrote:

> I was working to implement your proposed solution - it seemed most logical
> to place overlay_prev_vec in the buffer struct, but placing the three
> necessary properties( overlay_prev_vec, noverlays_prev,
> prev_point_motion_hook ) in the structure caused the 'make' process to
> return a segfault.

That was what I expected, yes.  But the more I think about it, the more
it seems it should be a property linked to a window rather than to
a buffer.  I think this deserves thought first.

- What should happen if the same buffer is shown in the windows and the
 user switches from one to the other?  Should the hooks be run at every
 window-switch, even though no cursor moves?
- What should happen if a buffer is display in a window and then the
 user does C-x b: should the hooks be run?

IIUC, if the data is per-window, then the answers will be "no; yes", if
it's per-buffer, then the answers will be "yes; no".

I believe this is correct.
 

Whenm thinking about it, it's best to try and think of concrete examples
which would use this feature.  And keep in mind that it's usually better
for a hook to be run too many times than too few times (the code that's
run too many times, can try and detect the extra times and do nothing
in those cases).

The use case I was looking at, as I mentioned earlier, was for flymake-like modes to display the actual error messages when point is on an error line.  This currently looks like it's usually implemented( or is in js2.el, at least ) by having both an overlay and a text property and keeping the two in sync.  In this case, I think that doing C-x o to the same buffer should run the hooks, and that C-x b should not, which would imply that maybe the buffer is the best place to put them.
 

> I'm going to look into this more in a few days, but do
> you have any idea why this would be happening, or another implementation
> strategy that wouldn't run into this problem?  I followed the instructions
> about placing non-Lisp_Objects above the 'name' variable, so that is most
> likely not the issue.

The slot placement would have been my first thought, otherwise,
I can't think of anything particularly likely.


       Stefan