before-change-functions called twice at yank

before-change-functions called twice at yank

[Lars Hansen]

Do emacs -q and evaluate

(progn
  (add-hook 'before-change-functions
            (lambda (b e)
              (message "before-change: '%s'" (buffer-substring b e))) nil t)
  (add-hook 'after-change-functions
            (lambda (b e l)
              (message "after-change: '%s'" (buffer-substring b e))) nil t))


in the *scratch* buffer. If you press the letter a, the message buffer
will get

before-change: ''
after-change: 'a'

as expected. Now yank the string "foo". Then the message buffer gets

before-change: ''
after-change: 'foo'
before-change: 'foo'

This seems as a bug to me. Am I missing something?

Re: before-change-functions called twice at yank

[martin rudalics]

> Do emacs -q and evaluate
>
> (progn
>   (add-hook 'before-change-functions
>             (lambda (b e)
>               (message "before-change: '%s'" (buffer-substring b e))) nil t)
>   (add-hook 'after-change-functions
>             (lambda (b e l)
>               (message "after-change: '%s'" (buffer-substring b e))) nil t))
>
>
> in the *scratch* buffer. If you press the letter a, the message buffer
> will get
>
> before-change: ''
> after-change: 'a'
>
> as expected. Now yank the string "foo". Then the message buffer gets
>
> before-change: ''
> after-change: 'foo'
> before-change: 'foo'
>
> This seems as a bug to me. Am I missing something?

Indeed you should see another message from `after-change-functions' like:

before-change: ''
after-change: 'foo'
before-change: 'foo'
after-change: 'foo'

The last two messages are due to `remove-yank-excluded-properties' which
removes `yank-excluded-properties' from the string inserted.  You can
avoid running the hooks either by doing

   (let ((inhibit-read-only t)
	(inhibit-modification-hooks t))

in `remove-yank-excluded-properties' directly or by wrapping the calls to
`remove-yank-excluded-properties' in `insert-for-yank-1' (and possibly
`insert-buffer-substring-as-yank') as

   (let ((inhibit-modification-hooks t))	
      (remove-yank-excluded-properties opoint (point)))

Re: before-change-functions called twice at yank

[Lars Hansen]

martin rudalics wrote:

> Indeed you should see another message from `after-change-functions' like:
>
> before-change: ''
> after-change: 'foo'
> before-change: 'foo'
> after-change: 'foo'
>
> The last two messages are due to `remove-yank-excluded-properties' which
> removes `yank-excluded-properties' from the string inserted.  You can
> avoid running the hooks either by doing

Thanks for the explanation of `remove-yank-excluded-properties'.
But I do *not* get the last message! That's why I think something is
wrong (a bug).

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

[Lars Hansen]

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

remove-list-of-text-properties may run before-change-functions without
running after-change-functions. The attached patch fixes the problem.
The patch ensures that before-change-functions and
after-change-functions are not run at all when no changes are done.
If there are no objections, I will install in about a week.



[-- Attachment #2: textprop.diff --]
[-- Type: text/x-patch, Size: 2152 bytes --]

*** textprop.c.~1.145.~	2006-04-15 18:10:14.000000000 +0200
--- textprop.c	2006-04-15 20:46:19.116514478 +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,1641 ****
--- 1645,1653 ----
  	  return Qt;
  	}
  
+       if (!modified && BUFFERP (object)
+ 	  && interval_has_some_properties_list (properties, i))
+ 	modify_region (XBUFFER (object), XINT (start), XINT (end));
        len -= LENGTH (i);
        modified += remove_properties (Qnil, properties, i, object);
        i = next_interval (i);


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[Richard Stallman]

	      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?

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

[Lars Hansen]

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

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.


[-- Attachment #2: textprop.diff --]
[-- Type: text/x-patch, Size: 2347 bytes --]

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


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[Richard Stallman]

    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.

Would you please add that explanation as a comment?
And maybe this one too:

    The before-change-functions are run by modify_region called immediately
    before the changes are done by remove_properties.

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

[Lars Hansen]

>Would you please add that explanation as a comment?
>  
>
Done and installed.