Overalays and point-entered

Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 362 bytes --]

Re: Overalays and point-entered

[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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

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@iro.umontreal.ca>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
>
>

[-- Attachment #2: Type: text/html, Size: 1974 bytes --]

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 2371 bytes --]

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@gmail.com> 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@iro.umontreal.ca>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
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 3078 bytes --]

Re: Overalays and point-entered

[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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1.1: Type: text/plain, Size: 1538 bytes --]

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@iro.umontreal.ca>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
>

[-- Attachment #1.2: Type: text/html, Size: 2047 bytes --]

[-- Attachment #2: overlay-properties.patch --]
[-- Type: text/x-patch, Size: 2673 bytes --]

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

Re: Overalays and point-entered

[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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 3580 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 4194 bytes --]

Re: Overalays and point-entered

[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

Re: Overalays and point-entered

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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Wed, Sep 23, 2009 at 4:55 PM, Stefan Monnier <monnier@iro.umontreal.ca>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

[-- Attachment #2: Type: text/html, Size: 1472 bytes --]

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

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@gmail.com> wrote:

>
>
> On Wed, Sep 23, 2009 at 4:55 PM, Stefan Monnier <monnier@iro.umontreal.ca>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
>

[-- Attachment #2: Type: text/html, Size: 1972 bytes --]

Re: Overalays and point-entered

[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

Re: Overlays and point-entered

[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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 7129 bytes --]

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@iro.umontreal.ca>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
>

[-- Attachment #2: Type: text/html, Size: 8294 bytes --]

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 7671 bytes --]

Any comments ont his?

Thanks,
Nathaniel Flath

On Tue, Oct 6, 2009 at 2:33 PM, Nathaniel Flath <flat0103@gmail.com> 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@iro.umontreal.ca
> > 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
>>
>
>

[-- Attachment #2: Type: text/html, Size: 8790 bytes --]

Re: Overalays and point-entered

[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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

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@iro.umontreal.ca>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
>

[-- Attachment #2: Type: text/html, Size: 1802 bytes --]

Re: Overalays and point-entered

[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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

On Thu, Oct 22, 2009 at 11:37 AM, Stefan Monnier
<monnier@iro.umontreal.ca>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
>

[-- Attachment #2: Type: text/html, Size: 3199 bytes --]

Re: Overalays and point-entered

[Stefan Monnier]

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

Hmmm.... it doesn't seem clear to me why you'd want it for C-x o but not
for C-x b.  Aren't there other potential use-cases?


        Stefan

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

Well, you'd want it to display when you moved to the buffer - If you are
moving out of the overlay in question with C-x o it may not matter, but if
you are moving into a window with a buffer that the point is in one of these
overlays you'd want the action to trigger and display the error message.
The same applies to C-x b.  This was just the use case I had in mind - there
are probably others.

On Sat, Oct 24, 2009 at 10:30 PM, Stefan Monnier
<monnier@iro.umontreal.ca>wrote:

> > 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.
>
> Hmmm.... it doesn't seem clear to me why you'd want it for C-x o but not
> for C-x b.  Aren't there other potential use-cases?
>
>
>        Stefan
>

[-- Attachment #2: Type: text/html, Size: 1551 bytes --]

Re: Overalays and point-entered

[Stefan Monnier]

> Well, you'd want it to display when you moved to the buffer - If you are
> moving out of the overlay in question with C-x o it may not matter, but if
> you are moving into a window with a buffer that the point is in one of these
> overlays you'd want the action to trigger and display the error message.
> The same applies to C-x b.  This was just the use case I had in mind - there
> are probably others.

IIUC you're saying that it should be run both for C-x o and C-x b, right?
I'd understand that.  I read your earlier answer to say that you wanted
C-x o to do it but not C-x b, which I found puzzling.

Personally, I'd say that in those cases actually it doesn't matter that
much either way: for C-x o, you're going back to a in window that's
already displayed and to a point that's already displayed, so there's
a good chance you've already seen the message earlier.
For C-x b it's less likely, but there's still a fairly good chance as
well, and in either case if you miss a message in those cases it doesn't
seem like too big a problem.

So I think this use-case is unlikely to give us a good and strong
indication of how it should work.

Here's another potential use case: when doing completion inside a buffer
(e.g. completing an identifier in some programming mode), if the
completion brings up a *Completions* buffer, we'd want to hide the
*Completions* buffer when the completion is done, but detecting "when
the completion is done" is difficult, so one way to do it would be to
place a "cursor-leave" hook on the completion area.

Now, what the behavior should be upon C-x o or C-x b is again somewhat
unclear: for C-x b, actually I think it's pretty clear that it should
run the hook (which is a vote in favor of per-window data), but for "C-x
o" it's less clear: running the hook would often be a good idea, but
would mean that it's somewhere between difficult and impossible to let
the user go to the *Completions* buffer to select an entry with
choose-completion.


        Stefan

Re: Overalays and point-entered

[Miles Bader]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
> Now, what the behavior should be upon C-x o or C-x b is again somewhat
> unclear: for C-x b, actually I think it's pretty clear that it should
> run the hook (which is a vote in favor of per-window data), but for "C-x
> o" it's less clear: running the hook would often be a good idea, but
> would mean that it's somewhere between difficult and impossible to let
> the user go to the *Completions* buffer to select an entry with
> choose-completion.

It seems like it would be easier to handle the subtle variations among a
variety of cases if there were simply hooks for each type of movement --
one which is per-buffer, and only cares about point position, one which
runs when a window becomes selected/deselected (C-x o case), and one
which runs when a buffer is attached/detached from a window (C-x b case).

Then the programmer could add hooks to handle which things he cared
about, without having them be inadvertently triggered in cases he
doesn't care about.

-miles

-- 
"Though they may have different meanings, the cries of 'Yeeeee-haw!' and
 'Allahu akbar!' are, in spirit, not actually all that different."

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

This may be a good solution.  What are your thoughts, Stefan?

Nathaniel

On Tue, Oct 27, 2009 at 8:44 PM, Miles Bader <miles@gnu.org> wrote:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > Now, what the behavior should be upon C-x o or C-x b is again somewhat
> > unclear: for C-x b, actually I think it's pretty clear that it should
> > run the hook (which is a vote in favor of per-window data), but for "C-x
> > o" it's less clear: running the hook would often be a good idea, but
> > would mean that it's somewhere between difficult and impossible to let
> > the user go to the *Completions* buffer to select an entry with
> > choose-completion.
>
> It seems like it would be easier to handle the subtle variations among a
> variety of cases if there were simply hooks for each type of movement --
> one which is per-buffer, and only cares about point position, one which
> runs when a window becomes selected/deselected (C-x o case), and one
> which runs when a buffer is attached/detached from a window (C-x b case).
>
> Then the programmer could add hooks to handle which things he cared
> about, without having them be inadvertently triggered in cases he
> doesn't care about.
>
> -miles
>
> --
> "Though they may have different meanings, the cries of 'Yeeeee-haw!' and
>  'Allahu akbar!' are, in spirit, not actually all that different."
>

[-- Attachment #2: Type: text/html, Size: 1870 bytes --]

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1.1: Type: text/plain, Size: 1736 bytes --]

I started working on this - attached is a patch that will run hooks in
point-motion only when movement occurs inside the same buffer and window.
I'll work on buffer-change-motion-hook a bit later.  Let me know if there's
any issues.

Thanks,
Nathaniel Flath

On Sat, Oct 31, 2009 at 12:03 PM, Nathaniel Flath <flat0103@gmail.com>wrote:

> This may be a good solution.  What are your thoughts, Stefan?
>
> Nathaniel
>
>
> On Tue, Oct 27, 2009 at 8:44 PM, Miles Bader <miles@gnu.org> wrote:
>
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> > Now, what the behavior should be upon C-x o or C-x b is again somewhat
>> > unclear: for C-x b, actually I think it's pretty clear that it should
>> > run the hook (which is a vote in favor of per-window data), but for "C-x
>> > o" it's less clear: running the hook would often be a good idea, but
>> > would mean that it's somewhere between difficult and impossible to let
>> > the user go to the *Completions* buffer to select an entry with
>> > choose-completion.
>>
>> It seems like it would be easier to handle the subtle variations among a
>> variety of cases if there were simply hooks for each type of movement --
>> one which is per-buffer, and only cares about point position, one which
>> runs when a window becomes selected/deselected (C-x o case), and one
>> which runs when a buffer is attached/detached from a window (C-x b case).
>>
>> Then the programmer could add hooks to handle which things he cared
>> about, without having them be inadvertently triggered in cases he
>> doesn't care about.
>>
>> -miles
>>
>> --
>> "Though they may have different meanings, the cries of 'Yeeeee-haw!' and
>>  'Allahu akbar!' are, in spirit, not actually all that different."
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 2584 bytes --]

[-- Attachment #2: point-motion.patch --]
[-- Type: text/x-patch, Size: 7002 bytes --]

diff --git a/src/editfns.c b/src/editfns.c
index 8dfea1f..333db38 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -500,6 +500,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/keyboard.c b/src/keyboard.c
index de4e1c5..7c198e9 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1978,6 +1978,10 @@ command_loop_1 ()
 	  && !already_adjusted)
 	adjust_point_for_property (last_point_position, MODIFF != prev_modiff);

+      if (current_buffer == prev_buffer
+          && NILP (Vinhibit_point_motion_hooks))
+        run_point_motion_hooks();
+
       /* Install chars successfully executed in kbd macro.  */

       if (!NILP (current_kboard->defining_kbd_macro)
@@ -1990,6 +1994,98 @@ command_loop_1 ()
     }
 }

+/*  Runs 'point-motion hooks on text properties and overlays.*/
+void
+run_point_motion_hooks ()
+{
+  extern Lisp_Object Qpoint_motion, Qwindow;
+
+  static Lisp_Object* overlay_prev_vec;
+  static Lisp_Object prev_text_prop;
+  static int noverlays_prev;
+  static Lisp_Object prev_window;
+
+  int i, j, noverlays_cur;
+  Lisp_Object *overlay_cur_vec;
+  Lisp_Object point_motion, overlay_window;
+
+  if ( !NILP (Feq (prev_window, Fselected_window())))
+    {
+    /* 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;
+  }
+  prev_window = Fselected_window();
+}
+
+
 extern Lisp_Object Qcomposition, Qdisplay;

 /* Adjust point to a boundary of a region that has such a property
@@ -8101,10 +8197,10 @@ parse_menu_item (item, notreal, inmenubar)
 		  && !(SYMBOLP (def) && EQ (tem, XSYMBOL (def)->function))))
 	    keys = Qnil;
 	}
-
+
       if (NILP (keys))
 	keys = Fwhere_is_internal (def, Qnil, Qt, Qnil, Qnil);
-
+
       if (!NILP (keys))
 	{
 	  tem = Fkey_description (keys, Qnil);
@@ -8119,7 +8215,7 @@ parse_menu_item (item, notreal, inmenubar)
 	  /* tem = concat3 (build_string ("  ("), tem, build_string (")")); */
 	}
     }
-
+

   /* If we only want to precompute equivalent key bindings, stop here. */
   if (notreal)
diff --git a/src/textprop.c b/src/textprop.c
index e82af12..02a66c0 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -54,6 +54,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;
@@ -2349,6 +2350,7 @@ inherits it if NONSTICKINESS is nil.  The `front-sticky' and
   Qpoint_left = intern ("point-left");
   staticpro (&Qpoint_entered);
   Qpoint_entered = intern ("point-entered");
+  Qpoint_motion = intern ("point-motion");

   defsubr (&Stext_properties_at);
   defsubr (&Sget_text_property);

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

So after having my motherboard fried and having to finish projects, I
finally had enough time to look at this again.  I've bene paying attention
to this list, so I realize that the feature freeze is in effect, but if it's
possible to discuss this now so that the next time trunk is unfrozen it can
be put in I'd like to do that.

The current  solution I have for the problem we were discussing is to have
both buffers and windows have a list of overlays that had text properties
with 'point-motion at the last point position.  The value of 'point-motion
is a function that takes prev-pt-position, current-pt-position,  the
overlay/text property the function belongs to, the buffer that was active
last time run_point_motion_hooks was called, the previous window, and
whether the hook 'belongs' to a buffer or a window.  This is getting a bit
cumbersome, but also lets the writer of the hook specify exactly the
behavior they want.  The hooks for the current buffer, previous buffer,
current window, and previous window are all run.  I'm working on the patch
for this - I'll send it on when I'm finished, which will probably be later
today.  Does this sound like a good approach, or do you have some other
suggestion?

Thanks,
Nathaniel Flath

On Fri, Nov 6, 2009 at 9:54 AM, Nathaniel Flath <flat0103@gmail.com> wrote:

> I started working on this - attached is a patch that will run hooks in
> point-motion only when movement occurs inside the same buffer and window.
> I'll work on buffer-change-motion-hook a bit later.  Let me know if there's
> any issues.
>
> Thanks,
> Nathaniel Flath
>
>
> On Sat, Oct 31, 2009 at 12:03 PM, Nathaniel Flath <flat0103@gmail.com>wrote:
>
>> This may be a good solution.  What are your thoughts, Stefan?
>>
>> Nathaniel
>>
>>
>> On Tue, Oct 27, 2009 at 8:44 PM, Miles Bader <miles@gnu.org> wrote:
>>
>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> > Now, what the behavior should be upon C-x o or C-x b is again somewhat
>>> > unclear: for C-x b, actually I think it's pretty clear that it should
>>> > run the hook (which is a vote in favor of per-window data), but for
>>> "C-x
>>> > o" it's less clear: running the hook would often be a good idea, but
>>> > would mean that it's somewhere between difficult and impossible to let
>>> > the user go to the *Completions* buffer to select an entry with
>>> > choose-completion.
>>>
>>> It seems like it would be easier to handle the subtle variations among a
>>> variety of cases if there were simply hooks for each type of movement --
>>> one which is per-buffer, and only cares about point position, one which
>>> runs when a window becomes selected/deselected (C-x o case), and one
>>> which runs when a buffer is attached/detached from a window (C-x b case).
>>>
>>> Then the programmer could add hooks to handle which things he cared
>>> about, without having them be inadvertently triggered in cases he
>>> doesn't care about.
>>>
>>> -miles
>>>
>>> --
>>> "Though they may have different meanings, the cries of 'Yeeeee-haw!' and
>>>  'Allahu akbar!' are, in spirit, not actually all that different."
>>>
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 4307 bytes --]

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1.1: Type: text/plain, Size: 3388 bytes --]

The patch is attached - please let me know if you have any comments.

Thanks,
Nathaniel Flath

On Wed, Dec 9, 2009 at 6:41 PM, Nathaniel Flath <flat0103@gmail.com> wrote:

> So after having my motherboard fried and having to finish projects, I
> finally had enough time to look at this again.  I've bene paying attention
> to this list, so I realize that the feature freeze is in effect, but if it's
> possible to discuss this now so that the next time trunk is unfrozen it can
> be put in I'd like to do that.
>
> The current  solution I have for the problem we were discussing is to have
> both buffers and windows have a list of overlays that had text properties
> with 'point-motion at the last point position.  The value of 'point-motion
> is a function that takes prev-pt-position, current-pt-position,  the
> overlay/text property the function belongs to, the buffer that was active
> last time run_point_motion_hooks was called, the previous window, and
> whether the hook 'belongs' to a buffer or a window.  This is getting a bit
> cumbersome, but also lets the writer of the hook specify exactly the
> behavior they want.  The hooks for the current buffer, previous buffer,
> current window, and previous window are all run.  I'm working on the patch
> for this - I'll send it on when I'm finished, which will probably be later
> today.  Does this sound like a good approach, or do you have some other
> suggestion?
>
> Thanks,
> Nathaniel Flath
>
>
> On Fri, Nov 6, 2009 at 9:54 AM, Nathaniel Flath <flat0103@gmail.com>wrote:
>
>> I started working on this - attached is a patch that will run hooks in
>> point-motion only when movement occurs inside the same buffer and window.
>> I'll work on buffer-change-motion-hook a bit later.  Let me know if there's
>> any issues.
>>
>> Thanks,
>> Nathaniel Flath
>>
>>
>> On Sat, Oct 31, 2009 at 12:03 PM, Nathaniel Flath <flat0103@gmail.com>wrote:
>>
>>> This may be a good solution.  What are your thoughts, Stefan?
>>>
>>> Nathaniel
>>>
>>>
>>> On Tue, Oct 27, 2009 at 8:44 PM, Miles Bader <miles@gnu.org> wrote:
>>>
>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>> > Now, what the behavior should be upon C-x o or C-x b is again somewhat
>>>> > unclear: for C-x b, actually I think it's pretty clear that it should
>>>> > run the hook (which is a vote in favor of per-window data), but for
>>>> "C-x
>>>> > o" it's less clear: running the hook would often be a good idea, but
>>>> > would mean that it's somewhere between difficult and impossible to let
>>>> > the user go to the *Completions* buffer to select an entry with
>>>> > choose-completion.
>>>>
>>>> It seems like it would be easier to handle the subtle variations among a
>>>> variety of cases if there were simply hooks for each type of movement --
>>>> one which is per-buffer, and only cares about point position, one which
>>>> runs when a window becomes selected/deselected (C-x o case), and one
>>>> which runs when a buffer is attached/detached from a window (C-x b
>>>> case).
>>>>
>>>> Then the programmer could add hooks to handle which things he cared
>>>> about, without having them be inadvertently triggered in cases he
>>>> doesn't care about.
>>>>
>>>> -miles
>>>>
>>>> --
>>>> "Though they may have different meanings, the cries of 'Yeeeee-haw!' and
>>>>  'Allahu akbar!' are, in spirit, not actually all that different."
>>>>
>>>
>>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 4838 bytes --]

[-- Attachment #2: pointmotion.patch --]
[-- Type: text/x-patch, Size: 11593 bytes --]

Index: buffer.c
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.c,v
retrieving revision 1.591
diff -r1.591 buffer.c
399c399,401
< 
---
>   b->noverlays_prev = 0;
>   b->prev_text_prop = Qnil;
>   b->overlay_prev_vec = 0;
2208c2210
<     error ("Cannot swap a dead buffer's text");    
---
>     error ("Cannot swap a dead buffer's text");
4355c4357
<     last_overlay_modification_hooks = larger_vector 
---
>     last_overlay_modification_hooks = larger_vector
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;
> 
798a806,809
> 
>   /* Text property containing point-motion */
>   Lisp_Object prev_text_prop;
> 
Index: editfns.c
===================================================================
RCS file: /sources/emacs/emacs/src/editfns.c,v
retrieving revision 1.476
diff -r1.476 editfns.c
502a503,544
> /* 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;
> }
> 
Index: keyboard.c
===================================================================
RCS file: /sources/emacs/emacs/src/keyboard.c,v
retrieving revision 1.1024
diff -r1.1024 keyboard.c
1982a1983,1985
>       if (NILP (Vinhibit_point_motion_hooks))
>         run_point_motion_hooks (prev_buffer);
> 
1994a1998,2203
> /* Checks if it should run a 'point-motion hook for the given overlay or text property.  The
>  * property will be run if it exists, if the window property isn't set to disable it, and if
>  * the overlay is not in overlay_vec.
>  */
> void check_run_point_motion_hook (overlay,
>                                   prev_buffer,
>                                   prev_window,
>                                   is_buffer,
>                                   overlay_vec,
>                                   noverlay_vec)
>      Lisp_Object overlay, prev_buffer, prev_window, is_buffer;
>      Lisp_Object* overlay_vec;
>      int noverlay_vec;
> {
>   extern Lisp_Object Qpoint_motion, Qwindow;
>   Lisp_Object point_motion, overlay_window;
>   int j;
>   point_motion = Foverlay_get (overlay, Qpoint_motion);
>   overlay_window = Foverlay_get (overlay, Qwindow);
>   if (!NILP (point_motion) &&
>       (NILP (overlay_window)
>        || !NILP (Feq (overlay_window, Fselected_window()))))
>     {
>       for (j = 0; j < noverlay_vec; j++)
>         {
>           if (!NILP (Feq (overlay, overlay_vec[j] ) ) )
>             return;
>         }
> 
>       call6 (point_motion,
>              make_number (last_point_position),
>              make_number (current_buffer->pt),
>              overlay,
>              prev_buffer,
>              prev_window,
>              is_buffer
>              );
>     }
> }
> 
> /*  Runs 'point-motion hooks on text properties and overlays. */
> void
> run_point_motion_hooks (prev_c_buffer)
>      struct buffer* prev_c_buffer;
> {
>   extern Lisp_Object Qpoint_motion, Qwindow;
> 
>   static Lisp_Object* prev_window;
>   static Lisp_Object* prev_buffer;
>   static struct window* prev_c_window;
> 
>   int i, j, noverlays_cur;
>   Lisp_Object *overlay_cur_vec;
>   Lisp_Object point_motion, overlay_window;
>   struct window* current_window = XWINDOW (Fselected_window());
> 
>   /* Retrieves vector of overlays in current location and runs 'point-motion */
>      overlay_cur_vec = get_overlays_at_pos (make_number (current_buffer->pt),
>                                          current_buffer,
>                                          &noverlays_cur);
>   for (i = 0; i < noverlays_cur; i++)
>     {
>       check_run_point_motion_hook (overlay_cur_vec[i],
>                                    prev_buffer,
>                                    prev_window,
>                                    Qt,
>                                    0,
>                                    0);
>     }
> 
>   /* Runs 'point-motion for overlays being moved out of */
>   for (i = 0; i < current_buffer->noverlays_prev; i++)
>     {
>       check_run_point_motion_hook( current_buffer->overlay_prev_vec[i],
>                                    prev_buffer,
>                                    prev_window,
>                                    Qt,
>                                    overlay_cur_vec,
>                                    noverlays_cur );
>     }
> 
>   for (i = 0; i < current_window->noverlays_prev; i++)
>     {
>       check_run_point_motion_hook( current_window->overlay_prev_vec[i],
>                                    prev_buffer,
>                                    prev_window,
>                                    Qnil,
>                                    overlay_cur_vec,
>                                    noverlays_cur );
>     }
> 
>   /* Rest of commands handle changing windows/buffers */
>   if( current_buffer != prev_c_buffer ) {
>     for (i = 0; i < prev_c_buffer->noverlays_prev; i++ ) {
>       check_run_point_motion_hook( prev_c_buffer->overlay_prev_vec[i],
>                                    prev_buffer,
>                                    prev_window,
>                                    Qt,
>                                    overlay_cur_vec,
>                                    noverlays_cur );
>     }
>   }
> 
>   if( prev_c_window != 0 && current_window != prev_c_window ) {
>     for (i = 0; i < prev_c_window->noverlays_prev; i++)
>       {
>         check_run_point_motion_hook( prev_c_window->overlay_prev_vec[i],
>                                      prev_buffer,
>                                      prev_window,
>                                      Qnil,
>                                      overlay_cur_vec,
>                                      noverlays_cur );
>       }
>   }
> 
>   /* Runs hook for current text property */
>   point_motion = Fget_text_property (make_number (current_buffer->pt),
>                                      Qpoint_motion,
>                                      Qnil);
>   if (!NILP (point_motion))
>     {
>       call6 (point_motion,
>              make_number (last_point_position),
>              make_number (current_buffer->pt),
>              point_motion,
>              prev_buffer,
>              prev_window,
>              Qt );
>     }
> 
> 
> 
>   /* Runs 'point-motion for text property being moved out of */
>   if (current_buffer->prev_text_prop != 0
>       && !NILP (current_buffer->prev_text_prop)) {
>     call6 (current_buffer->prev_text_prop,
>            make_number (last_point_position),
>            make_number (current_buffer->pt),
>            current_buffer->prev_text_prop,
>            prev_buffer,
>            prev_window,
>            Qt );
>   }
> 
>   if (current_window->prev_text_prop != 0
>       && !NILP (current_window->prev_text_prop)) {
>     call6 (current_window->prev_text_prop,
>            make_number (last_point_position),
>            make_number (current_buffer->pt),
>            current_window->prev_text_prop,
>            prev_buffer,
>            prev_window,
>            Qt );
>   }
> 
>   /* These blocks handle changing buffer and window */
>   if (prev_c_buffer != current_buffer &&
>       prev_c_buffer->prev_text_prop != 0
>       && !NILP (prev_c_buffer->prev_text_prop)) {
>     call6 (prev_c_buffer->prev_text_prop,
>            make_number (last_point_position),
>            make_number (current_buffer->pt),
>            prev_c_buffer->prev_text_prop,
>            prev_buffer,
>            prev_window,
>            Qt );
>   }
> 
>   if (prev_c_window != 0 &&
>       prev_c_window != XWINDOW (Fselected_window()) &&
>       prev_c_window->prev_text_prop != 0
>       && !NILP (prev_c_window->prev_text_prop)) {
>     call6 (prev_c_window->prev_text_prop,
>            make_number (last_point_position),
>            make_number (current_buffer->pt),
>            prev_c_window->prev_text_prop,
>            prev_buffer,
>            prev_window,
>            Qnil );
>   }
> 
> 
>   /* Frees previous overlays and sets them to the current list */
>   if (current_window->overlay_prev_vec)
>     free (current_window->overlay_prev_vec);
>   if (current_buffer->overlay_prev_vec)
>     free (current_buffer->overlay_prev_vec);
> 
>   current_buffer->overlay_prev_vec = overlay_cur_vec;
>   current_buffer->noverlays_prev = noverlays_cur;
> 
>   current_window->overlay_prev_vec =
>     xmalloc( sizeof (overlay_cur_vec) );
>   memcpy (current_window->overlay_prev_vec,
>           overlay_cur_vec,
>           sizeof (overlay_cur_vec));
>   current_window->noverlays_prev = noverlays_cur;
> 
>   current_buffer->prev_text_prop = point_motion;
>   current_window->prev_text_prop = point_motion;
>   prev_window = Fselected_window();
>   prev_c_window = XWINDOW( Fselected_window() );
> 
>   prev_buffer = Fcurrent_buffer();
> }
> 
4734c4943
<   do 
---
>   do
Index: textprop.c
===================================================================
RCS file: /sources/emacs/emacs/src/textprop.c,v
retrieving revision 1.167
diff -r1.167 textprop.c
56a57
> Lisp_Object Qpoint_motion;
2351a2353,2354
>   staticpro (&Qpoint_motion);
>   Qpoint_motion = intern_c_string("point-motion");
Index: window.c
===================================================================
RCS file: /sources/emacs/emacs/src/window.c,v
retrieving revision 1.644
diff -r1.644 window.c
270c270,272
< 
---
>   p->prev_text_prop = 0;
>   p->overlay_prev_vec = 0;
>   p->noverlays_prev = 0;
Index: window.h
===================================================================
RCS file: /sources/emacs/emacs/src/window.h,v
retrieving revision 1.81
diff -r1.81 window.h
294a295,304
> 
>     /* 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;
> 
>     /* Text property containing point-motion */
>     Lisp_Object* prev_text_prop;

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

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1.1: Type: text/plain, Size: 2131 bytes --]

On Thu, Dec 10, 2009 at 3:32 AM, Stefan Monnier <monnier@iro.umontreal.ca>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

[-- Attachment #1.2: Type: text/html, Size: 2934 bytes --]

[-- Attachment #2: point-motion.patch --]
[-- Type: text/x-patch, Size: 11056 bytes --]

Index: buffer.c
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.c,v
retrieving revision 1.591
diff -u -r1.591 buffer.c
--- buffer.c	21 Nov 2009 11:52:26 -0000	1.591
+++ buffer.c	20 Dec 2009 23:36:42 -0000
@@ -396,7 +396,8 @@
   b->width_run_cache = 0;
   b->width_table = Qnil;
   b->prevent_redisplay_optimizations_p = 1;
-
+  b->prev_text_prop = Qnil;
+  b->overlay_prev = Qnil;
   /* Put this on the chain of all buffers including killed ones.  */
   b->next = all_buffers;
   all_buffers = b;
Index: buffer.h
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.h,v
retrieving revision 1.130
diff -u -r1.130 buffer.h
--- buffer.h	21 Nov 2009 11:52:26 -0000	1.130
+++ buffer.h	20 Dec 2009 23:36:42 -0000
@@ -796,6 +796,13 @@
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
   Lisp_Object cursor_in_non_selected_windows;
+
+  /* Text property containing point-motion */
+  Lisp_Object prev_text_prop;
+
+  /* Overlay containing point-motion */
+  Lisp_Object overlay_prev;
+
 };

 \f
Index: editfns.c
===================================================================
RCS file: /sources/emacs/emacs/src/editfns.c,v
retrieving revision 1.476
diff -u -r1.476 editfns.c
--- editfns.c	21 Nov 2009 11:52:27 -0000	1.476
+++ editfns.c	20 Dec 2009 23:36:42 -0000
@@ -500,6 +500,48 @@
     }
 }

+/* 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.
Index: keyboard.c
===================================================================
RCS file: /sources/emacs/emacs/src/keyboard.c,v
retrieving revision 1.1024
diff -u -r1.1024 keyboard.c
--- keyboard.c	17 Nov 2009 08:21:35 -0000	1.1024
+++ keyboard.c	20 Dec 2009 23:36:44 -0000
@@ -1522,6 +1522,8 @@

 extern int nonundocount;	/* Declared in cmds.c.  */

+void run_point_motion_hooks();
+
 Lisp_Object
 command_loop_1 ()
 {
@@ -1980,6 +1982,9 @@
 	  && !already_adjusted)
 	adjust_point_for_property (last_point_position, MODIFF != prev_modiff);

+      if (NILP (Vinhibit_point_motion_hooks))
+        run_point_motion_hooks (prev_buffer);
+
       /* Install chars successfully executed in kbd macro.  */

       if (!NILP (current_kboard->defining_kbd_macro)
@@ -1992,6 +1997,154 @@
     }
 }

+/*  Runs 'point-motion hooks on text properties and overlays. */
+void
+run_point_motion_hooks (prev_c_buffer)
+     struct buffer* prev_c_buffer;
+{
+  extern Lisp_Object Qpoint_motion, Qwindow, Qpriority;
+
+  static Lisp_Object prev_window;
+  static Lisp_Object prev_buffer;
+  static struct window* prev_c_window;
+
+  int i, j, noverlays_cur;
+  Lisp_Object point_motion, overlay_window;
+  struct window* current_window = XWINDOW (Fselected_window());
+  int run_overlay, run_text_prop;
+  Lisp_Object text_properties[5];
+  Lisp_Object overlays[5];
+
+  Lisp_Object *overlay_cur_vec;
+  Lisp_Object current_overlay;
+
+  /* Retrieves vector of overlays in current location and uses this to
+     find the 'current' overlay to run */
+  overlay_cur_vec = get_overlays_at_pos (make_number (current_buffer->pt),
+                                         current_buffer,
+                                         &noverlays_cur);
+  current_overlay = Qnil;
+  for (i = 0; i < noverlays_cur; i++)
+    {
+      Lisp_Object pm = Foverlay_get (overlay_cur_vec[i], Qpoint_motion);
+      if (!NILP (pm)) {
+        if (NILP (current_overlay))
+          {
+            current_overlay = overlay_cur_vec[i];
+            continue;
+          }
+        Lisp_Object cur_priority = Foverlay_get (current_overlay, Qpriority);
+        Lisp_Object tmp_priority = Foverlay_get (overlay_cur_vec[i], Qpriority);
+        if ( NILP (cur_priority)
+            || (!NILP (tmp_priority)
+                && XINT (cur_priority) < XINT (tmp_priority)))
+              {
+                current_overlay = overlay_cur_vec[i];
+              }
+        }
+    }
+
+  /* Runs the 'point-motion property for unique overlays */
+  if (NILP (Feq (Fcurrent_buffer(), prev_buffer))
+      || NILP (Feq (Fselected_window(), prev_window))
+      || NILP (Feq (current_overlay, prev_c_buffer->overlay_prev)))
+    {
+      overlays[0] = current_overlay;
+      overlays[1] = current_buffer->overlay_prev;
+      overlays[2] = current_window->overlay_prev;
+      overlays[3] = Qnil;
+      overlays[4] = Qnil;
+      if (prev_c_buffer)
+        overlays[3] = prev_c_buffer->overlay_prev;
+      if (prev_c_window)
+        overlays[4] = prev_c_window->overlay_prev;
+
+      for (i = 0; i < 5; i++)
+        {
+          run_overlay = 1;
+          if (NILP (overlays[i])) continue;
+          for (j = 0; j < i; j++)
+            {
+              if (!NILP (Feq (overlays[i], overlays[j]))) {
+                run_overlay = 0;
+                break;
+              }
+            }
+          if (run_overlay)
+            {
+              point_motion = Foverlay_get (overlays[i], Qpoint_motion);
+              overlay_window = Foverlay_get (overlays[i], Qwindow);
+              if (!NILP (point_motion)
+                  && (NILP (overlay_window)
+                      || !NILP (Feq (overlay_window, Fselected_window()))))
+                {
+                  call5 (point_motion,
+                         make_number (last_point_position),
+                         make_number (current_buffer->pt),
+                         overlays[i],
+                         prev_buffer,
+                         prev_window);
+                }
+            }
+        }
+    }
+
+  /* Get text properties to run */
+  text_properties[0] = Fget_text_property (make_number (current_buffer->pt),
+                                           Qpoint_motion,
+                                           Qnil);
+  text_properties[1] = current_buffer->prev_text_prop;
+  text_properties[2] = current_window->prev_text_prop;
+  text_properties[3] = Qnil;
+  text_properties[4] = Qnil;
+  if (prev_c_buffer) {
+    text_properties[3] = prev_c_buffer->prev_text_prop;
+  }
+  if (prev_c_window) {
+    text_properties[4] = prev_c_window->prev_text_prop;
+  }
+
+  /* Runs point-motion of text properties */
+  if (NILP (Feq (Fcurrent_buffer(), prev_buffer))
+      || NILP (Feq (Fselected_window(), prev_window))
+      || NILP (Feq (text_properties[0], prev_c_buffer->prev_text_prop)))
+    {
+      for (i = 0; i < 5; i++)
+        {
+          run_text_prop = 1;
+          for (j = 0; j < i; j++)
+            {
+              if (!NILP (Feq (text_properties[i], text_properties[j])))
+                {
+                  run_text_prop = 0;
+                  break;
+                }
+            }
+          if (run_text_prop == 1 && !NILP (text_properties[i]))
+            {
+              call5 (text_properties[i],
+                     make_number (last_point_position),
+                     make_number (current_buffer->pt),
+                     text_properties[i],
+                     prev_buffer,
+                     prev_window );
+            }
+        }
+    }
+
+  /* Sets previous overlays and text properties */
+  current_buffer->overlay_prev = current_overlay;
+  current_window->overlay_prev = current_overlay;
+
+  current_buffer->prev_text_prop = text_properties[0];
+  current_window->prev_text_prop = text_properties[0];
+
+  prev_window = Fselected_window();
+  prev_c_window = XWINDOW (Fselected_window());
+  prev_buffer = Fcurrent_buffer();
+}
+
 extern Lisp_Object Qcomposition, Qdisplay;

 /* Adjust point to a boundary of a region that has such a property
Index: textprop.c
===================================================================
RCS file: /sources/emacs/emacs/src/textprop.c,v
retrieving revision 1.167
diff -u -r1.167 textprop.c
--- textprop.c	6 Nov 2009 06:50:59 -0000	1.167
+++ textprop.c	20 Dec 2009 23:36:44 -0000
@@ -54,6 +54,7 @@
 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;
@@ -2349,6 +2350,8 @@
   Qpoint_left = intern_c_string ("point-left");
   staticpro (&Qpoint_entered);
   Qpoint_entered = intern_c_string ("point-entered");
+  staticpro (&Qpoint_motion);
+  Qpoint_motion = intern_c_string("point-motion");

   defsubr (&Stext_properties_at);
   defsubr (&Sget_text_property);
Index: window.c
===================================================================
RCS file: /sources/emacs/emacs/src/window.c,v
retrieving revision 1.644
diff -u -r1.644 window.c
--- window.c	6 Nov 2009 06:50:59 -0000	1.644
+++ window.c	20 Dec 2009 23:36:45 -0000
@@ -267,7 +267,8 @@
   p->scroll_bar_width = Qnil;
   p->vertical_scroll_bar_type = Qt;
   p->resize_proportionally = Qnil;
+  p->prev_text_prop = Qnil;
+  p->overlay_prev = Qnil;
+
   Vwindow_list = Qnil;
   return val;
 }
Index: window.h
===================================================================
RCS file: /sources/emacs/emacs/src/window.h,v
retrieving revision 1.81
diff -u -r1.81 window.h
--- window.h	8 Jan 2009 03:16:09 -0000	1.81
+++ window.h	20 Dec 2009 23:36:45 -0000
@@ -292,6 +292,12 @@
     /* Z_BYTE - the buffer position of the last glyph in the current matrix
        of W.  Only valid if WINDOW_END_VALID is not nil.  */
     int window_end_bytepos;
+
+    /* Last overlay run containing point-motion */
+    Lisp_Object overlay_prev;
+
+    /* Text property containing point-motion */
+    Lisp_Object prev_text_prop;
 };

 /* 1 if W is a minibuffer window.  */

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

What is happening with this?

Thanks,
Nathaniel Flath

On Sun, Dec 20, 2009 at 6:39 PM, Nathaniel Flath <flat0103@gmail.com> wrote:

>
>
> On Thu, Dec 10, 2009 at 3:32 AM, Stefan Monnier <monnier@iro.umontreal.ca>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
>

[-- Attachment #2: Type: text/html, Size: 3437 bytes --]

Re: Overalays and point-entered

[Nathaniel Flath]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

I'm going to go ahead and file this as a bug in the next few days, if
everyone is too busy with the bzr switchover to discuss - that way the
manual reminders won't be needed and this can be brought up at a later
date.

On Fri, Jan 1, 2010 at 10:34 PM, Nathaniel Flath <flat0103@gmail.com> wrote:

> What is happening with this?
>
> Thanks,
> Nathaniel Flath
>
>
> On Sun, Dec 20, 2009 at 6:39 PM, Nathaniel Flath <flat0103@gmail.com>wrote:
>
>>
>>
>> On Thu, Dec 10, 2009 at 3:32 AM, Stefan Monnier <monnier@iro.umontreal.ca
>> > 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
>>
>
>

[-- Attachment #2: Type: text/html, Size: 4059 bytes --]

Re: Overalays and point-entered

[Stefan Monnier]

> I'm going to go ahead and file this as a bug in the next few days, if

Please do,


        Stefan