[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#24340: insert-file-contents calls before-change-functions too late
From: |
Stefan Monnier |
Subject: |
bug#24340: insert-file-contents calls before-change-functions too late |
Date: |
Tue, 30 Aug 2016 14:42:19 -0400 |
Package: Emacs
Version: 25.1.50
Recipe:
emacs -Q lisp/subr.el -f auto-revert-mode-hook
M-: (defun sm-foo (&rest args) (message "b-c-f: %S size=%S" args
(buffer-size))) RET
M-: (add-hook 'before-change-functions #'sm-foo nil t) RET
then
zile lisp/subr.el
<modify the file somehow, then save it>
finally check *Messages*: you should see that the first message will say
something like "b-c-f: (...) NNN" where NNN is *smaller* than the
original buffer size. IOW b-c-f is called after some part of the buffer
has already been removed.
Indeed, the bug is in Finsert_file_contents where we do:
if (same_at_end != same_at_start)
{
invalidate_buffer_caches (current_buffer,
BYTE_TO_CHAR (same_at_start),
same_at_end_charpos);
del_range_byte (same_at_start, same_at_end, 0);
[...]
/* This binding is to avoid ask-user-about-supersession-threat
being called in insert_from_buffer (via in
prepare_to_modify_buffer). */
specbind (intern ("buffer-file-name"), Qnil);
insert_from_buffer (XBUFFER (conversion_buffer),
same_at_start_charpos, inserted_chars, 0);
The call to del_range_byte has arg prepare=0 so it doesn't run b-c-f.
Instead b-c-f is called from insert_from_buffer, which is too late since
the deletion already took place.
This is also the source of the known bug that b-c-f is not called at all
if insert-file-contents ends up only deleting text (since in that case
del_range_byte is called but insert_from_buffer isn't).
I include a suggested patch (which also would have the consequence that
we could get rid of the `prepare' argument of del_range_byte).
Stefan
diff --git a/src/fileio.c b/src/fileio.c
index bf6bf62..55331d9 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3839,6 +3839,7 @@ by calling `format-decode', which see. */)
if (! giveup_match_end)
{
ptrdiff_t temp;
+ ptrdiff_t this_count = SPECPDL_INDEX ();
/* We win! We can handle REPLACE the optimized way. */
@@ -3868,13 +3869,15 @@ by calling `format-decode', which see. */)
beg_offset += same_at_start - BEGV_BYTE;
end_offset -= ZV_BYTE - same_at_end;
+ specbind (intern ("buffer-file-name"), Qnil);
invalidate_buffer_caches (current_buffer,
BYTE_TO_CHAR (same_at_start),
same_at_end_charpos);
- del_range_byte (same_at_start, same_at_end, 0);
+ del_range_byte (same_at_start, same_at_end, true);
/* Insert from the file at the proper position. */
temp = BYTE_TO_CHAR (same_at_start);
SET_PT_BOTH (temp, same_at_start);
+ unbind_to (this_count, Qnil);
/* If display currently starts at beginning of line,
keep it that way. */
@@ -3979,10 +3982,11 @@ by calling `format-decode', which see. */)
/* Truncate the buffer to the size of the file. */
if (same_at_start != same_at_end)
{
+ specbind (intern ("buffer-file-name"), Qnil);
invalidate_buffer_caches (current_buffer,
BYTE_TO_CHAR (same_at_start),
BYTE_TO_CHAR (same_at_end));
- del_range_byte (same_at_start, same_at_end, 0);
+ del_range_byte (same_at_start, same_at_end, true);
}
inserted = 0;
@@ -4030,12 +4034,23 @@ by calling `format-decode', which see. */)
we are taking from the decoded string. */
inserted -= (ZV_BYTE - same_at_end) + (same_at_start - BEGV_BYTE);
+ /* This binding is to avoid ask-user-about-supersession-threat
+ being called in insert_from_buffer or del_range_bytes (via in
+ prepare_to_modify_buffer).
+ AFAICT this is mostly needed because we change the buffer
+ before we set current_buffer->modtime, but if the file is modified
+ while we read from it, even if we set current_buffer->modtime first we
+ could still end up calling ask-user-about-supersession-threat
+ which wouldn't make much sense. */
+ specbind (intern ("buffer-file-name"), Qnil);
if (same_at_end != same_at_start)
{
+ /* FIXME: Shouldn't invalidate_buffer_caches be called by
+ del_range_byte or even directly by prepare_to_modify_buffer? */
invalidate_buffer_caches (current_buffer,
BYTE_TO_CHAR (same_at_start),
same_at_end_charpos);
- del_range_byte (same_at_start, same_at_end, 0);
+ del_range_byte (same_at_start, same_at_end, true);
temp = GPT;
eassert (same_at_start == GPT_BYTE);
same_at_start = GPT_BYTE;
@@ -4056,10 +4071,6 @@ by calling `format-decode', which see. */)
same_at_start + inserted - BEGV_BYTE
+ BUF_BEG_BYTE (XBUFFER (conversion_buffer)))
- same_at_start_charpos);
- /* This binding is to avoid ask-user-about-supersession-threat
- being called in insert_from_buffer (via in
- prepare_to_modify_buffer). */
- specbind (intern ("buffer-file-name"), Qnil);
insert_from_buffer (XBUFFER (conversion_buffer),
same_at_start_charpos, inserted_chars, 0);
/* Set `inserted' to the number of inserted characters. */
- bug#24340: insert-file-contents calls before-change-functions too late,
Stefan Monnier <=