all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* text_property_stickiness
@ 2006-06-24 20:27 Chong Yidong
  2006-06-25 15:33 ` text_property_stickiness Richard Stallman
  0 siblings, 1 reply; 11+ messages in thread
From: Chong Yidong @ 2006-06-24 20:27 UTC (permalink / raw)


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-24 20:27 text_property_stickiness Chong Yidong
@ 2006-06-25 15:33 ` Richard Stallman
  2006-06-25 20:59   ` text_property_stickiness Chong Yidong
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Stallman @ 2006-06-25 15:33 UTC (permalink / raw)
  Cc: emacs-devel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-25 15:33 ` text_property_stickiness Richard Stallman
@ 2006-06-25 20:59   ` Chong Yidong
  2006-06-26  4:33     ` text_property_stickiness Miles Bader
  2006-06-26 11:33     ` text_property_stickiness Richard Stallman
  0 siblings, 2 replies; 11+ messages in thread
From: Chong Yidong @ 2006-06-25 20:59 UTC (permalink / raw)
  Cc: emacs-devel

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)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-25 20:59   ` text_property_stickiness Chong Yidong
@ 2006-06-26  4:33     ` Miles Bader
  2006-06-26 17:50       ` text_property_stickiness Chong Yidong
  2006-06-27 10:34       ` text_property_stickiness Richard Stallman
  2006-06-26 11:33     ` text_property_stickiness Richard Stallman
  1 sibling, 2 replies; 11+ messages in thread
From: Miles Bader @ 2006-06-26  4:33 UTC (permalink / raw)
  Cc: rms, emacs-devel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-25 20:59   ` text_property_stickiness Chong Yidong
  2006-06-26  4:33     ` text_property_stickiness Miles Bader
@ 2006-06-26 11:33     ` Richard Stallman
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Stallman @ 2006-06-26 11:33 UTC (permalink / raw)
  Cc: emacs-devel

    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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-26  4:33     ` text_property_stickiness Miles Bader
@ 2006-06-26 17:50       ` Chong Yidong
  2006-06-27  2:03         ` text_property_stickiness Miles Bader
  2006-06-27 10:34       ` text_property_stickiness Richard Stallman
  1 sibling, 1 reply; 11+ messages in thread
From: Chong Yidong @ 2006-06-26 17:50 UTC (permalink / raw)
  Cc: rms, emacs-devel

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-26 17:50       ` text_property_stickiness Chong Yidong
@ 2006-06-27  2:03         ` Miles Bader
  2006-06-27 16:16           ` text_property_stickiness Richard Stallman
  0 siblings, 1 reply; 11+ messages in thread
From: Miles Bader @ 2006-06-27  2:03 UTC (permalink / raw)
  Cc: rms, emacs-devel

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]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-26  4:33     ` text_property_stickiness Miles Bader
  2006-06-26 17:50       ` text_property_stickiness Chong Yidong
@ 2006-06-27 10:34       ` Richard Stallman
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Stallman @ 2006-06-27 10:34 UTC (permalink / raw)
  Cc: cyd, emacs-devel

    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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-27  2:03         ` text_property_stickiness Miles Bader
@ 2006-06-27 16:16           ` Richard Stallman
  2006-06-28  0:04             ` text_property_stickiness Chong Yidong
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Stallman @ 2006-06-27 16:16 UTC (permalink / raw)
  Cc: cyd, emacs-devel

    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?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-27 16:16           ` text_property_stickiness Richard Stallman
@ 2006-06-28  0:04             ` Chong Yidong
  2006-06-29 12:59               ` text_property_stickiness Richard Stallman
  0 siblings, 1 reply; 11+ messages in thread
From: Chong Yidong @ 2006-06-28  0:04 UTC (permalink / raw)
  Cc: emacs-devel, Miles Bader

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: text_property_stickiness
  2006-06-28  0:04             ` text_property_stickiness Chong Yidong
@ 2006-06-29 12:59               ` Richard Stallman
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Stallman @ 2006-06-29 12:59 UTC (permalink / raw)
  Cc: emacs-devel, miles

    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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2006-06-29 12:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-24 20:27 text_property_stickiness Chong Yidong
2006-06-25 15:33 ` text_property_stickiness Richard Stallman
2006-06-25 20:59   ` text_property_stickiness Chong Yidong
2006-06-26  4:33     ` text_property_stickiness Miles Bader
2006-06-26 17:50       ` text_property_stickiness Chong Yidong
2006-06-27  2:03         ` text_property_stickiness Miles Bader
2006-06-27 16:16           ` text_property_stickiness Richard Stallman
2006-06-28  0:04             ` text_property_stickiness Chong Yidong
2006-06-29 12:59               ` text_property_stickiness Richard Stallman
2006-06-27 10:34       ` text_property_stickiness Richard Stallman
2006-06-26 11:33     ` text_property_stickiness Richard Stallman

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.