Re: Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank]

[Lars Hansen]

Richard Stallman wrote:

>             if (! interval_has_some_properties_list (properties, i))
>    !      if (modified)
>    !        {
>    !          if (BUFFERP (object))
>    !            signal_after_change (XINT (start), XINT (end) - XINT (start),
>    !                                 XINT (end) - XINT (start));
>
>I'm not confident about that code.  It runs the after-change hook, but
>in this case, where is the before-change hook run?
>  
>
The before-change-functions are run by modify_region called immediately
before the changes are done by remove_properties.
Actually, I have changed that part of the code a bit to make it more
readable, see the attached patch.

The general idea is:
Fremove_list_of_text_properties has a for (;;) loop that loops over the
parts of the interval. It updates the flag `modified' recording if
changes have been done. To make things work right, we must call
modify_region before calling remove_properties iff modified == 0, and we
must call signal_after_change before returning iff modified != 0.

*** textprop.c.~1.145.~ 2006-04-15 18:10:14.000000000 +0200
--- textprop.c  2006-04-16 18:44:25.318950490 +0200
***************
*** 1602,1610 ****
        }
      }
  
-   if (BUFFERP (object))
-     modify_region (XBUFFER (object), XINT (start), XINT (end));
- 
    /* We are at the beginning of an interval, with len to scan */
    for (;;)
      {
--- 1602,1607 ----
***************
*** 1614,1623 ****
        if (LENGTH (i) >= len)
        {
          if (! interval_has_some_properties_list (properties, i))
!           return modified ? Qt : Qnil;
  
          if (LENGTH (i) == len)
            {
              remove_properties (Qnil, properties, i, object);
              if (BUFFERP (object))
                signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1611,1630 ----
        if (LENGTH (i) >= len)
        {
          if (! interval_has_some_properties_list (properties, i))
!           if (modified)
!             {
!               if (BUFFERP (object))
!                 signal_after_change (XINT (start), XINT (end) - XINT (start),
!                                      XINT (end) - XINT (start));
!               return Qt;
!             }
!           else
!             return Qnil;
  
          if (LENGTH (i) == len)
            {
+             if (!modified && BUFFERP (object))
+               modify_region (XBUFFER (object), XINT (start), XINT (end));
              remove_properties (Qnil, properties, i, object);
              if (BUFFERP (object))
                signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1629,1634 ****
--- 1636,1643 ----
          unchanged = i;
          i = split_interval_left (i, len);
          copy_properties (unchanged, i);
+         if (!modified && BUFFERP (object))
+           modify_region (XBUFFER (object), XINT (start), XINT (end));
          remove_properties (Qnil, properties, i, object);
          if (BUFFERP (object))
            signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1636,1643 ****
          return Qt;
        }
  
        len -= LENGTH (i);
-       modified += remove_properties (Qnil, properties, i, object);
        i = next_interval (i);
      }
  }
--- 1645,1658 ----
          return Qt;
        }
  
+       if (interval_has_some_properties_list (properties, i))
+       {
+         if (!modified && BUFFERP (object))
+           modify_region (XBUFFER (object), XINT (start), XINT (end));
+         remove_properties (Qnil, properties, i, object);
+         modified = 1;
+       }
        len -= LENGTH (i);
        i = next_interval (i);
      }
  }