bug#24340: insert-file-contents calls before-change-functions too late

[Eli Zaretskii]

> 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.