text_property_stickiness() ignores `text-property-default-nonsticky'

text_property_stickiness() ignores `text-property-default-nonsticky'

[Dmitry Kurochkin]

Hello.

I am trying to make `keymap' property rear-nonsticky, so that it does
not affect the next character after the region the property is applied
to.  If I do it by setting rear-nonsticky text property to 't or to a
list that contains 'keymap, it works fine:

  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "RET") 'bug)
    (switch-to-buffer "test")
    (insert "123456")
    (put-text-property 1 4 'keymap map)
    (put-text-property (point-min) (point-max) 'rear-nonsticky '(keymap))
    (goto-char 4)
    (message "keymap: %s" (get-text-property (point) 'keymap))
    (message "key-binding: %s" (key-binding (kbd "RET"))))

But if I set `text-property-default-nonsticky' variable instead, it does
not work:

  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "RET") 'bug)
    (switch-to-buffer "test")
    (insert "123456")
    (put-text-property 1 4 'keymap map)
    (make-local-variable 'text-property-default-nonsticky)
    (add-to-list 'text-property-default-nonsticky '(keymap . t))
    (goto-char 4)
    (message "keymap: %s" (get-text-property (point) 'keymap))
    (message "key-binding: %s" (key-binding (kbd "RET"))))

Looking through the code, I got down to text_property_stickiness()
function in src/textprop.c, which is used by get_pos_property(), which
is used by get_local_map().  There are checks for `front-sticky' and
`rear-nonsticky' properties, but no checks for
`text-property-default-nonsticky' variable.  Before opening false bug
reports, I would like to confirm that this is an issue indeed, and it is
not me doing something stupid.

Regards,
  Dmitry

Re: text_property_stickiness() ignores `text-property-default-nonsticky'

[Stefan Monnier]

>>>>> "Dmitry" == Dmitry Kurochkin <dmitry.kurochkin@gmail.com> writes:

> Hello.
> I am trying to make `keymap' property rear-nonsticky, so that it does
> not affect the next character after the region the property is applied
> to.  If I do it by setting rear-nonsticky text property to 't or to a
> list that contains 'keymap, it works fine:

>   (let ((map (make-sparse-keymap)))
>     (define-key map (kbd "RET") 'bug)
>     (switch-to-buffer "test")
>     (insert "123456")
>     (put-text-property 1 4 'keymap map)
>     (put-text-property (point-min) (point-max) 'rear-nonsticky '(keymap))
>     (goto-char 4)
>     (message "keymap: %s" (get-text-property (point) 'keymap))
>     (message "key-binding: %s" (key-binding (kbd "RET"))))

> But if I set `text-property-default-nonsticky' variable instead, it does
> not work:

>   (let ((map (make-sparse-keymap)))
>     (define-key map (kbd "RET") 'bug)
>     (switch-to-buffer "test")
>     (insert "123456")
>     (put-text-property 1 4 'keymap map)
>     (make-local-variable 'text-property-default-nonsticky)
>     (add-to-list 'text-property-default-nonsticky '(keymap . t))
>     (goto-char 4)
>     (message "keymap: %s" (get-text-property (point) 'keymap))
>     (message "key-binding: %s" (key-binding (kbd "RET"))))

> Looking through the code, I got down to text_property_stickiness()
> function in src/textprop.c, which is used by get_pos_property(), which
> is used by get_local_map().  There are checks for `front-sticky' and
> `rear-nonsticky' properties, but no checks for
> `text-property-default-nonsticky' variable.

Indeed.  Does the patch below fix your problem?

> Before opening false bug reports, I would like to confirm that this is
> an issue indeed, and it is not me doing something stupid.

"False" bug reports are not something you should be afraid of.
I.e. feel free to use M-x report-emacs-bug when something looks like
a bug *to you*, even if we decide that it's a not a bug from our point
of view.


        Stefan


=== modified file 'src/textprop.c'
--- src/textprop.c	2011-06-25 13:00:55 +0000
+++ src/textprop.c	2011-07-04 16:49:42 +0000
@@ -1707,10 +1707,14 @@
 {
   Lisp_Object prev_pos, front_sticky;
   int is_rear_sticky = 1, is_front_sticky = 0; /* defaults */
+  Lisp_Object default = Fassq (prop, Vtext_property_default_nonsticky);
 
   if (NILP (buffer))
     XSETBUFFER (buffer, current_buffer);
 
+  if (CONSP (default) && NILP (XCDR (default)))
+    is_rear_sticky = 0;
+
   if (XINT (pos) > BUF_BEGV (XBUFFER (buffer)))
     /* Consider previous character.  */
     {

Re: text_property_stickiness() ignores `text-property-default-nonsticky'

[Dmitry Kurochkin]

On Mon, 04 Jul 2011 12:53:00 -0400, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >>>>> "Dmitry" == Dmitry Kurochkin <dmitry.kurochkin@gmail.com> writes:
> 
> > Hello.
> > I am trying to make `keymap' property rear-nonsticky, so that it does
> > not affect the next character after the region the property is applied
> > to.  If I do it by setting rear-nonsticky text property to 't or to a
> > list that contains 'keymap, it works fine:
> 
> >   (let ((map (make-sparse-keymap)))
> >     (define-key map (kbd "RET") 'bug)
> >     (switch-to-buffer "test")
> >     (insert "123456")
> >     (put-text-property 1 4 'keymap map)
> >     (put-text-property (point-min) (point-max) 'rear-nonsticky '(keymap))
> >     (goto-char 4)
> >     (message "keymap: %s" (get-text-property (point) 'keymap))
> >     (message "key-binding: %s" (key-binding (kbd "RET"))))
> 
> > But if I set `text-property-default-nonsticky' variable instead, it does
> > not work:
> 
> >   (let ((map (make-sparse-keymap)))
> >     (define-key map (kbd "RET") 'bug)
> >     (switch-to-buffer "test")
> >     (insert "123456")
> >     (put-text-property 1 4 'keymap map)
> >     (make-local-variable 'text-property-default-nonsticky)
> >     (add-to-list 'text-property-default-nonsticky '(keymap . t))
> >     (goto-char 4)
> >     (message "keymap: %s" (get-text-property (point) 'keymap))
> >     (message "key-binding: %s" (key-binding (kbd "RET"))))
> 
> > Looking through the code, I got down to text_property_stickiness()
> > function in src/textprop.c, which is used by get_pos_property(), which
> > is used by get_local_map().  There are checks for `front-sticky' and
> > `rear-nonsticky' properties, but no checks for
> > `text-property-default-nonsticky' variable.
> 
> Indeed.  Does the patch below fix your problem?
> 

The patch below would not work because it checks for nil in
`text-property-default-nonsticky' instead of 't.  But your commit
r104949 fixes the issue.  Thank you!  Will it get included in some
bugfix emacs release anytime soon?  Or only emacs24?

It seems that text_property_stickiness() or get_pos_property() or some
other elisp interface to get "what inherited property value for PROP is
at this POS" would be useful.  For example, `widget-field-activate'
should work at the end of widget.  Currently, `widget-field-activate' is
called at the end of widget, because keymap is rear-sticky but it does
not call widget's action because `widget-field-at' does not respect
stickiness.  What do you think?

Regards,
  Dmitry

> > Before opening false bug reports, I would like to confirm that this is
> > an issue indeed, and it is not me doing something stupid.
> 
> "False" bug reports are not something you should be afraid of.
> I.e. feel free to use M-x report-emacs-bug when something looks like
> a bug *to you*, even if we decide that it's a not a bug from our point
> of view.
> 
> 
>         Stefan
> 
> 
> === modified file 'src/textprop.c'
> --- src/textprop.c	2011-06-25 13:00:55 +0000
> +++ src/textprop.c	2011-07-04 16:49:42 +0000
> @@ -1707,10 +1707,14 @@
>  {
>    Lisp_Object prev_pos, front_sticky;
>    int is_rear_sticky = 1, is_front_sticky = 0; /* defaults */
> +  Lisp_Object default = Fassq (prop, Vtext_property_default_nonsticky);
>  
>    if (NILP (buffer))
>      XSETBUFFER (buffer, current_buffer);
>  
> +  if (CONSP (default) && NILP (XCDR (default)))
> +    is_rear_sticky = 0;
> +
>    if (XINT (pos) > BUF_BEGV (XBUFFER (buffer)))
>      /* Consider previous character.  */
>      {
> 

Re: text_property_stickiness() ignores `text-property-default-nonsticky'

[Stefan Monnier]

> The patch below would not work because it checks for nil in
> `text-property-default-nonsticky' instead of 't.

Yes, as you noticed, I caught it before committing.

> But your commit r104949 fixes the issue.  Thank you!  Will it get
> included in some bugfix emacs release anytime soon?  Or only Emacs24?

The next Emacs release should be 24.1 (there are no more planned 23.N).

> It seems that text_property_stickiness() or get_pos_property() or some
> other elisp interface to get "what inherited property value for PROP is
> at this POS" would be useful.  For example, `widget-field-activate'
> should work at the end of widget.  Currently, `widget-field-activate' is
> called at the end of widget, because keymap is rear-sticky but it does
> not call widget's action because `widget-field-at' does not respect
> stickiness.  What do you think?

At the C level, we have `get_pos_property' which returns the value of
a property at that position (i.e. between the char before the position
and the char after that position).  Note that since stickiness can vary
between properties, get_pos_property could still return a value
inconsistent with the keymap used.
But I guess we could export get_pos_property to Elisp.


        Stefan

Re: text_property_stickiness() ignores `text-property-default-nonsticky'

[Dmitry Kurochkin]

On Mon, 04 Jul 2011 23:50:41 -0400, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > The patch below would not work because it checks for nil in
> > `text-property-default-nonsticky' instead of 't.
> 
> Yes, as you noticed, I caught it before committing.
> 
> > But your commit r104949 fixes the issue.  Thank you!  Will it get
> > included in some bugfix emacs release anytime soon?  Or only Emacs24?
> 
> The next Emacs release should be 24.1 (there are no more planned 23.N).
> 
> > It seems that text_property_stickiness() or get_pos_property() or some
> > other elisp interface to get "what inherited property value for PROP is
> > at this POS" would be useful.  For example, `widget-field-activate'
> > should work at the end of widget.  Currently, `widget-field-activate' is
> > called at the end of widget, because keymap is rear-sticky but it does
> > not call widget's action because `widget-field-at' does not respect
> > stickiness.  What do you think?
> 
> At the C level, we have `get_pos_property' which returns the value of
> a property at that position (i.e. between the char before the position
> and the char after that position).  Note that since stickiness can vary
> between properties, get_pos_property could still return a value
> inconsistent with the keymap used.

Hm... I guess I miss something here.  Can you provide an example here?

> But I guess we could export get_pos_property to Elisp.
> 

Perhaps there should be `get-char-property-sticky' (and similar)
functions that do the same as non-sticky counterparts but check for
sticky properties additionally.

Then `widget-field-at' would use such function to get the `field'
property instead of `get-char-property'.  That should fix the issue with
`widget-field-activate' I described above.

Regards,
  Dmitry

> 
>         Stefan