* before-change-functions called twice at yank
@ 2006-04-10 22:37 Lars Hansen
2006-04-11 6:58 ` martin rudalics
2006-04-15 18:57 ` Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank] Lars Hansen
0 siblings, 2 replies; 8+ messages in thread
From: Lars Hansen @ 2006-04-10 22:37 UTC (permalink / raw)
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: before-change-functions called twice at yank
2006-04-10 22:37 before-change-functions called twice at yank Lars Hansen
@ 2006-04-11 6:58 ` martin rudalics
2006-04-11 7:23 ` Lars Hansen
2006-04-15 18:57 ` Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank] Lars Hansen
1 sibling, 1 reply; 8+ messages in thread
From: martin rudalics @ 2006-04-11 6:58 UTC (permalink / raw)
Cc: emacs-devel
> 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)))
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: before-change-functions called twice at yank
2006-04-11 6:58 ` martin rudalics
@ 2006-04-11 7:23 ` Lars Hansen
0 siblings, 0 replies; 8+ messages in thread
From: Lars Hansen @ 2006-04-11 7:23 UTC (permalink / raw)
Cc: emacs-devel
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).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank]
2006-04-10 22:37 before-change-functions called twice at yank Lars Hansen
2006-04-11 6:58 ` martin rudalics
@ 2006-04-15 18:57 ` Lars Hansen
2006-04-17 0:56 ` Richard Stallman
1 sibling, 1 reply; 8+ messages in thread
From: Lars Hansen @ 2006-04-15 18:57 UTC (permalink / raw)
Cc: rudalics
[-- 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank]
2006-04-15 18:57 ` Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank] Lars Hansen
@ 2006-04-17 0:56 ` Richard Stallman
2006-04-17 8:51 ` Lars Hansen
[not found] ` <44435682.4050409@soem.dk>
0 siblings, 2 replies; 8+ messages in thread
From: Richard Stallman @ 2006-04-17 0:56 UTC (permalink / raw)
Cc: rudalics, emacs-devel
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank]
2006-04-17 0:56 ` Richard Stallman
@ 2006-04-17 8:51 ` Lars Hansen
[not found] ` <44435682.4050409@soem.dk>
1 sibling, 0 replies; 8+ messages in thread
From: Lars Hansen @ 2006-04-17 8:51 UTC (permalink / raw)
[-- 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank]
[not found] ` <44435682.4050409@soem.dk>
@ 2006-04-18 1:42 ` Richard Stallman
2006-04-20 11:46 ` Lars Hansen
0 siblings, 1 reply; 8+ messages in thread
From: Richard Stallman @ 2006-04-18 1:42 UTC (permalink / raw)
Cc: rudalics, emacs-devel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank]
2006-04-18 1:42 ` Richard Stallman
@ 2006-04-20 11:46 ` Lars Hansen
0 siblings, 0 replies; 8+ messages in thread
From: Lars Hansen @ 2006-04-20 11:46 UTC (permalink / raw)
>Would you please add that explanation as a comment?
>
>
Done and installed.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2006-04-20 11:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-10 22:37 before-change-functions called twice at yank Lars Hansen
2006-04-11 6:58 ` martin rudalics
2006-04-11 7:23 ` Lars Hansen
2006-04-15 18:57 ` Bugs in remove-list-of-text-properties and a patch [WAS: before-change-functions called twice at yank] Lars Hansen
2006-04-17 0:56 ` Richard Stallman
2006-04-17 8:51 ` Lars Hansen
[not found] ` <44435682.4050409@soem.dk>
2006-04-18 1:42 ` Richard Stallman
2006-04-20 11:46 ` Lars Hansen
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.