text_property_stickiness

text_property_stickiness

[Chong Yidong]

Regarding the following FOR-RELEASE item:

  ** text_property_stickiness can be called with a POS value that is
     before BEGV.

  text_property_stickiness is called from get_pos_property, which is
  called from find_field, which is called from various user-level
  functions in editfns.c.

This little change to text_property_stickiness should be all that's
needed to fix it, no?

*** emacs/src/textprop.c.~1.147.~	2006-05-17 13:33:46.000000000 -0400
--- emacs/src/textprop.c	2006-06-24 16:24:46.000000000 -0400
***************
*** 1787,1792 ****
--- 1787,1794 ----
  	/* PROP is rear-non-sticky.  */
  	is_rear_sticky = 0;
      }
+   else
+     return 0;
  
    /* Consider following character.  */
    front_sticky = Fget_text_property (pos, Qfront_sticky, buffer);

Re: text_property_stickiness

[Richard Stallman]

    *** emacs/src/textprop.c.~1.147.~	2006-05-17 13:33:46.000000000 -0400
    --- emacs/src/textprop.c	2006-06-24 16:24:46.000000000 -0400
    ***************
    *** 1787,1792 ****
    --- 1787,1794 ----
	    /* PROP is rear-non-sticky.  */
	    is_rear_sticky = 0;
	  }
    +   else
    +     return 0;

This would make text_property_stickiness return 0 when POS is before
BEGV.  Does that give correct results in calls from find_field?  I
doubt it.

Meanwhile, if POS is after ZV, Fget_text_property will get an error
in the following code, right?

	/* Consider following character.  */
	front_sticky = Fget_text_property (pos, Qfront_sticky, buffer);

This problem really is tricky, for the reasons I explained.
It could be that we need to make sure find_field is never called
for positions outside BEGV..ZV.  That may require changes in callers
at higher level, and changes in the specs of the field functions.

Re: text_property_stickiness

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> This problem really is tricky, for the reasons I explained.
> It could be that we need to make sure find_field is never called
> for positions outside BEGV..ZV.  That may require changes in callers
> at higher level, and changes in the specs of the field functions.

I see.  I've checked the higher-level functions, and here are the
results:

text_property_stickiness is called by:
  * get_pos_property, which is called by:
      ** find_field, which is called by:
    	   Fdelete_field
	   Ffield_string
	   Ffield_string_no_properties
	   Ffield_beginning
	   Ffield_end
      ** Fconstrain_to_field, which is called by:
    	   Fline_beginning_position
	   Fline_end_position
      ** get_local_map, for only valid positions.
      ** adjust_point_for_property, for only valid positions.
  * adjust_for_invis_intang, which is called by:
      ** set_point_both, for only valid values of pos.

So the only thing we have to worry about are a bunch of lisp-visible
functions, Fdelete_field and so forth.  I think it is acceptable for
us to handle this situation by the signalling of an args_out_of_range
error, which, as you pointed out, already happens with the call to
Fget_text_property in text_property_stickiness.  So maybe all that we
need to do is to add a comment to the code.  (Or should we initialize
prev_pos to stop the checker from complaining?)

*** emacs/src/textprop.c.~1.147.~	2006-05-22 20:28:23.000000000 -0400
--- emacs/src/textprop.c	2006-06-25 16:50:17.000000000 -0400
***************
*** 1789,1794 ****
--- 1789,1796 ----
      }
  
    /* Consider following character.  */
+   /* If pos is outside the accessible range of the buffer, this
+      signals an args_out_of_range error.  */
    front_sticky = Fget_text_property (pos, Qfront_sticky, buffer);
  
    if (EQ (front_sticky, Qt)

Re: text_property_stickiness

[Miles Bader]

Chong Yidong <cyd@stupidchicken.com> writes:
> So the only thing we have to worry about are a bunch of lisp-visible
> functions, Fdelete_field and so forth.  I think it is acceptable for
> us to handle this situation by the signalling of an args_out_of_range
> error, which, as you pointed out, already happens with the call to
> Fget_text_property in text_property_stickiness.

Why do you think an error is the correct thing here?

My feeling (when Richard brought this up earlier) was that perhaps the
out-of-range value should be constrained to be within BEGV-ZV.

-Miles

-- 
"Whatever you do will be insignificant, but it is very important that
 you do it."  Mahatma Gandhi

Re: text_property_stickiness

[Richard Stallman]

    So the only thing we have to worry about are a bunch of lisp-visible
    functions, Fdelete_field and so forth.  I think it is acceptable for
    us to handle this situation by the signalling of an args_out_of_range
    error, which, as you pointed out, already happens with the call to
    Fget_text_property in text_property_stickiness.

I don't see any problem with that, off hand, but if we go this way
we need to update various doc strings and the Lisp manual.

Re: text_property_stickiness

[Chong Yidong]

Miles Bader <miles.bader@necel.com> writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>> So the only thing we have to worry about are a bunch of lisp-visible
>> functions, Fdelete_field and so forth.  I think it is acceptable for
>> us to handle this situation by the signalling of an args_out_of_range
>> error, which, as you pointed out, already happens with the call to
>> Fget_text_property in text_property_stickiness.
>
> Why do you think an error is the correct thing here?
>
> My feeling (when Richard brought this up earlier) was that perhaps the
> out-of-range value should be constrained to be within BEGV-ZV.

I don't think it's the wrong thing to do.  It's also similar to the
current behavior for other Lisp-visible functions, for example:

C-u 10 a
M-: (narrow-to-region 3 4)
M-: (text-properties-at 1)

This signals an args-out-of-range error.

Re: text_property_stickiness

[Miles Bader]

Chong Yidong <cyd@stupidchicken.com> writes:
>> Why do you think an error is the correct thing here?
>>
>> My feeling (when Richard brought this up earlier) was that perhaps the
>> out-of-range value should be constrained to be within BEGV-ZV.
>
> I don't think it's the wrong thing to do.  It's also similar to the
> current behavior for other Lisp-visible functions, for example:
>
> C-u 10 a
> M-: (narrow-to-region 3 4)
> M-: (text-properties-at 1)

I just checked the code and since `get_pos_property' is only called with
the initial position being passed by the original caller, I think you're
right, it does make sense to signal an error if it's out of range.

-Miles
-- 
`Suppose Korea goes to the World Cup final against Japan and wins,' Moon said.
`All the past could be forgiven.'   [NYT]

Re: text_property_stickiness

[Richard Stallman]

    My feeling (when Richard brought this up earlier) was that perhaps the
    out-of-range value should be constrained to be within BEGV-ZV.

Maybe the right thing to do is different for different user-level
field functions.

You're the main expert on fields, so I asked you to look at this
and figure out what is TRT.  Would you please do that?
The rest of us couldn't do it as well as you can do it.

Re: text_property_stickiness

[Richard Stallman]

    I just checked the code and since `get_pos_property' is only called with
    the initial position being passed by the original caller, I think you're
    right, it does make sense to signal an error if it's out of range.

Since you agree with this, let's do it.  Would someone please do it?

Re: text_property_stickiness

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     I just checked the code and since `get_pos_property' is only called with
>     the initial position being passed by the original caller, I think you're
>     right, it does make sense to signal an error if it's out of range.
>
> Since you agree with this, let's do it.  Would someone please do it?

I've changed updated the relevant docstrings to reflect this.  I
didn't make any changes to the code of text_properties_stickiness to
silence the source code checker, though, just added a comment.

Re: text_property_stickiness

[Richard Stallman]

    I've changed updated the relevant docstrings to reflect this.  I
    didn't make any changes to the code of text_properties_stickiness to
    silence the source code checker, though, just added a comment.

Thanks.  You could add a conditional and a call to abort
as a way to silence the code checker.