From: Eli Zaretskii <eliz@gnu.org>
To: Stefan Monnier <monnier@IRO.UMontreal.CA>
Cc: 24340@debbugs.gnu.org
Subject: bug#24340: insert-file-contents calls before-change-functions too late
Date: Tue, 30 Aug 2016 22:10:24 +0300 [thread overview]
Message-ID: <8360qim5sv.fsf@gnu.org> (raw)
In-Reply-To: <jwv37lmxfn8.fsf@iro.umontreal.ca> (message from Stefan Monnier on Tue, 30 Aug 2016 14:42:19 -0400)
> 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.
next prev parent reply other threads:[~2016-08-30 19:10 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-30 18:42 bug#24340: insert-file-contents calls before-change-functions too late Stefan Monnier
2016-08-30 19:10 ` Eli Zaretskii [this message]
2016-08-30 21:21 ` Stefan Monnier
2016-08-31 14:22 ` Eli Zaretskii
2016-08-31 14:48 ` Stefan Monnier
2016-08-31 15:03 ` Eli Zaretskii
2016-08-31 17:50 ` Stefan Monnier
2016-10-02 1:11 ` Stefan Monnier
2016-10-02 7:19 ` Eli Zaretskii
2016-10-03 13:48 ` Stefan Monnier
2016-10-03 14:13 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8360qim5sv.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=24340@debbugs.gnu.org \
--cc=monnier@IRO.UMontreal.CA \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.