[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#24340: insert-file-contents calls before-change-functions too late
From: |
Eli Zaretskii |
Subject: |
bug#24340: insert-file-contents calls before-change-functions too late |
Date: |
Tue, 30 Aug 2016 22:10:24 +0300 |
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Tue, 30 Aug 2016 14:42:19 -0400
>
> 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).
Thanks for the patch, but I don't want to make such pervasive changes
for this problem. I thought we agreed that a single call to
before-change hooks for the entire buffer would be an okay solution
for this scenario. What you suggest is exactly the slippery path
which I was afraid of: a lot of tricky/kludgey changes whose effect we
don't really understand, because we have no tests for the affected
functionality.
One comment to your FIXME comment:
> + /* FIXME: Shouldn't invalidate_buffer_caches be called by
> + del_range_byte or even directly by prepare_to_modify_buffer? */
prepare_to_modify_buffer already calls invalidate_buffer_caches. The
direct call here is because del_range_byte is called with its last
argument zero.