From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Aug 2016 18:40:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: 24340@debbugs.gnu.org X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.147258237229302 (code B ref -1); Tue, 30 Aug 2016 18:40:01 +0000 Received: (at submit) by debbugs.gnu.org; 30 Aug 2016 18:39:32 +0000 Received: from localhost ([127.0.0.1]:44674 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benwa-0007cY-9T for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:32 -0400 Received: from eggs.gnu.org ([208.118.235.92]:52412) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benwZ-0007cL-23 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benwS-0001lC-SX for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:26 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:58445) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwS-0001kr-P5 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43348) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwQ-0000mk-BK for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benwL-0001io-88 for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:21 -0400 Received: from chene.dit.umontreal.ca ([132.204.246.20]:51594) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwL-0001ia-3D for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:17 -0400 Received: from ceviche.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id u7UIdtsT003418 for ; Tue, 30 Aug 2016 14:39:55 -0400 Received: by ceviche.home (Postfix, from userid 20848) id 7F8A966274; Tue, 30 Aug 2016 14:42:19 -0400 (EDT) From: Stefan Monnier Date: Tue, 30 Aug 2016 14:42:19 -0400 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-NAI-Spam-Flag: NO X-NAI-Spam-Level: X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0.6 X-NAI-Spam-Rules: 4 Rules triggered BEC_TRC1=0.2, BEC_TRC1_W_GEN_SPAM_FEATRE=0.2, GEN_SPAM_FEATRE=0.2, RV5782=0 X-NAI-Spam-Version: 2.3.0.9418 : core <5782> : inlines <5157> : streams <1692582> : uri <2278217> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.0 (----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.0 (----) 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 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. */ From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Aug 2016 19:11:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Stefan Monnier Cc: 24340@debbugs.gnu.org Reply-To: Eli Zaretskii Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147258425732157 (code B ref 24340); Tue, 30 Aug 2016 19:11:01 +0000 Received: (at 24340) by debbugs.gnu.org; 30 Aug 2016 19:10:57 +0000 Received: from localhost ([127.0.0.1]:44682 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1beoQy-0008Ma-Uh for submit@debbugs.gnu.org; Tue, 30 Aug 2016 15:10:57 -0400 Received: from eggs.gnu.org ([208.118.235.92]:33305) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1beoQx-0008MN-9z for 24340@debbugs.gnu.org; Tue, 30 Aug 2016 15:10:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1beoQp-0000KK-DU for 24340@debbugs.gnu.org; Tue, 30 Aug 2016 15:10:50 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_20,RP_MATCHES_RCVD autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:59610) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1beoQp-0000KA-Ab; Tue, 30 Aug 2016 15:10:47 -0400 Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:2029 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1beoQj-00048s-HH; Tue, 30 Aug 2016 15:10:45 -0400 Date: Tue, 30 Aug 2016 22:10:24 +0300 Message-Id: <8360qim5sv.fsf@gnu.org> From: Eli Zaretskii In-reply-to: (message from Stefan Monnier on Tue, 30 Aug 2016 14:42:19 -0400) References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -6.5 (------) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.5 (------) > From: Stefan Monnier > 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. From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Aug 2016 21:19:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Eli Zaretskii Cc: 24340@debbugs.gnu.org Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147259192211772 (code B ref 24340); Tue, 30 Aug 2016 21:19:01 +0000 Received: (at 24340) by debbugs.gnu.org; 30 Aug 2016 21:18:42 +0000 Received: from localhost ([127.0.0.1]:44746 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1beqQc-00033n-91 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 17:18:42 -0400 Received: from alt13.smtp-out.videotron.ca ([135.19.0.26]:47014 helo=alt12.smtp-out.videotron.ca) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1beqQa-00033b-AW for 24340@debbugs.gnu.org; Tue, 30 Aug 2016 17:18:40 -0400 Received: from ceviche.home ([184.161.231.20]) by Videotron with SMTP id eqQTbtvQeHh2deqQUbYwRx; Tue, 30 Aug 2016 17:18:34 -0400 X-Authority-Analysis: v=2.1 cv=Lv0ysipc c=1 sm=1 tr=0 a=RXLRw6V2y9MFlfnpr5gyhA==:117 a=RXLRw6V2y9MFlfnpr5gyhA==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=7z1cN_iqozsA:10 a=HPZwGtT0M34nKraRG8AA:9 a=MNxCqmdJpjbzcpao:21 a=uyArZf1Cez8_ZioL:21 Received: by ceviche.home (Postfix, from userid 20848) id 8099A66274; Tue, 30 Aug 2016 17:21:38 -0400 (EDT) From: Stefan Monnier Message-ID: References: <8360qim5sv.fsf@gnu.org> Date: Tue, 30 Aug 2016 17:21:38 -0400 In-Reply-To: <8360qim5sv.fsf@gnu.org> (Eli Zaretskii's message of "Tue, 30 Aug 2016 22:10:24 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-CMAE-Envelope: MS4wfMSkJh2LRKiwAn+7d0CPKUUFzbW+sjp5cf0vDtpuiGrf8n5gbUijERTVRBSszlvxoEOguNVfBKoAEpzZRP7Jir7m4tyGGtHpYiMdnRp6ItXyauiBeMJQ KhA6H00HOflMtVbSVR+q8Yv578wDxU/HerRefkrcbh+x/tes5xGvbktUQh/xxTkc8iDUmRoO2vvaI74OBBzdQxvoCJnEtwsA5KG1D8g7XXcyQ2mZfoSuX1UZ fZ8VTrV0Go5lGpus+h2o4g== X-Spam-Score: 0.3 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.3 (/) > Thanks for the patch, but I don't want to make such pervasive changes > for this problem. It was for another problem (the fact that b-c-f was not called at all in a corner case). This is a problem (it breaks the promise that b-c-f covers all following changes) that shows up everytime insert-file-contents is called with a non-nil `replace' argument, which is a completely different situation. It's a common case. > I thought we agreed that a single call to before-change hooks for the > entire buffer would be an okay solution for this scenario. Kind of, but I think it'd be a kludge compared to the patch I submitted. This said, if you insist on fixing it some other way, that's fine by me. It's just that this problem is a lot more common than the one we knew about, so I think it makes it more important to fix it sooner rather than later. > 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. Its effect is to call prepare_to_modify_buffer one more time in a function which already calls it moments later. Seems safer than about 90% of the changes I installed in the past. >> + /* 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. OMG, indeed, so my patch makes even more sense and is even cleaner. See new patch below, which actually simplifies the code. I agree that the let-binding is a bit ugly, but that's the kludge we use currently, so my patch doesn't make it much uglier in this respect. A cleaner way to handle this issue might be to introduce a new buffer-local variable (like inhibit-file-supersession-check) which we'd let-bind to t around the whole function (either inside insert-file-contents when `replace' is non-nil, or around revert-buffer's call to insert-file-contents). But for now, I can live with the current kludge. Stefan diff --git a/src/fileio.c b/src/fileio.c index bf6bf62..a63deee 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,19 @@ by calling `format-decode', which see. */) beg_offset += same_at_start - BEGV_BYTE; end_offset -= ZV_BYTE - same_at_end; - 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 or del_range_bytes (via + prepare_to_modify_buffer). + AFAICT we could avoid ask-user-about-supersession-threat by setting + current_buffer->modtime earlier, but we could still end up calling + ask-user-about-supersession-threat if the file is modified while + we read it, so we bind buffer-file-name instead. */ + specbind (intern ("buffer-file-name"), Qnil); + del_range_byte (same_at_start, same_at_end); /* 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 +3986,9 @@ by calling `format-decode', which see. */) /* Truncate the buffer to the size of the file. */ if (same_at_start != same_at_end) { - 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); + /* See previous specbind for the reason behind this. */ + specbind (intern ("buffer-file-name"), Qnil); + del_range_byte (same_at_start, same_at_end); } inserted = 0; @@ -4030,12 +4036,11 @@ by calling `format-decode', which see. */) we are taking from the decoded string. */ inserted -= (ZV_BYTE - same_at_end) + (same_at_start - BEGV_BYTE); + /* See previous specbind for the reason behind this. */ + specbind (intern ("buffer-file-name"), Qnil); 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); + del_range_byte (same_at_start, same_at_end); temp = GPT; eassert (same_at_start == GPT_BYTE); same_at_start = GPT_BYTE; @@ -4056,10 +4061,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. */ diff --git a/src/fns.c b/src/fns.c index 31f0fd2..d3f9a6b 100644 --- a/src/fns.c +++ b/src/fns.c @@ -3192,7 +3192,7 @@ into shorter lines. */) SET_PT_BOTH (XFASTINT (beg), ibeg); insert (encoded, encoded_length); SAFE_FREE (); - del_range_byte (ibeg + encoded_length, iend + encoded_length, 1); + del_range_byte (ibeg + encoded_length, iend + encoded_length); /* If point was outside of the region, restore it exactly; else just move to the beginning of the region. */ diff --git a/src/insdel.c b/src/insdel.c index 5d3884b..ed914ec 100644 --- a/src/insdel.c +++ b/src/insdel.c @@ -1690,7 +1690,7 @@ del_range_1 (ptrdiff_t from, ptrdiff_t to, bool prepare, bool ret_string) /* Like del_range_1 but args are byte positions, not char positions. */ void -del_range_byte (ptrdiff_t from_byte, ptrdiff_t to_byte, bool prepare) +del_range_byte (ptrdiff_t from_byte, ptrdiff_t to_byte) { ptrdiff_t from, to; @@ -1706,23 +1706,22 @@ del_range_byte (ptrdiff_t from_byte, ptrdiff_t to_byte, bool prepare) from = BYTE_TO_CHAR (from_byte); to = BYTE_TO_CHAR (to_byte); - if (prepare) - { - ptrdiff_t old_from = from, old_to = Z - to; - ptrdiff_t range_length = to - from; - prepare_to_modify_buffer (from, to, &from); - to = from + range_length; - - if (old_from != from) - from_byte = CHAR_TO_BYTE (from); - if (to > ZV) - { - to = ZV; - to_byte = ZV_BYTE; - } - else if (old_to == Z - to) - to_byte = CHAR_TO_BYTE (to); - } + { + ptrdiff_t old_from = from, old_to = Z - to; + ptrdiff_t range_length = to - from; + prepare_to_modify_buffer (from, to, &from); + to = from + range_length; + + if (old_from != from) + from_byte = CHAR_TO_BYTE (from); + if (to > ZV) + { + to = ZV; + to_byte = ZV_BYTE; + } + else if (old_to == Z - to) + to_byte = CHAR_TO_BYTE (to); + } del_range_2 (from, from_byte, to, to_byte, 0); signal_after_change (from, to - from, 0); diff --git a/src/lisp.h b/src/lisp.h index 97c8d9f..b757e7b 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3514,7 +3514,7 @@ extern void insert_from_string_before_markers (Lisp_Object, ptrdiff_t, ptrdiff_t, bool); extern void del_range (ptrdiff_t, ptrdiff_t); extern Lisp_Object del_range_1 (ptrdiff_t, ptrdiff_t, bool, bool); -extern void del_range_byte (ptrdiff_t, ptrdiff_t, bool); +extern void del_range_byte (ptrdiff_t, ptrdiff_t); extern void del_range_both (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, bool); extern Lisp_Object del_range_2 (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, bool); From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 31 Aug 2016 14:23:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Stefan Monnier Cc: 24340@debbugs.gnu.org Reply-To: Eli Zaretskii Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147265337231526 (code B ref 24340); Wed, 31 Aug 2016 14:23:02 +0000 Received: (at 24340) by debbugs.gnu.org; 31 Aug 2016 14:22:52 +0000 Received: from localhost ([127.0.0.1]:45537 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf6Pg-0008CM-Ev for submit@debbugs.gnu.org; Wed, 31 Aug 2016 10:22:52 -0400 Received: from eggs.gnu.org ([208.118.235.92]:57819) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf6Pb-0008C6-02 for 24340@debbugs.gnu.org; Wed, 31 Aug 2016 10:22:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bf6PR-0007Gm-9T for 24340@debbugs.gnu.org; Wed, 31 Aug 2016 10:22:37 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:44106) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bf6PR-0007Ga-5w; Wed, 31 Aug 2016 10:22:33 -0400 Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:2862 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bf6PN-0002og-M9; Wed, 31 Aug 2016 10:22:31 -0400 Date: Wed, 31 Aug 2016 17:22:24 +0300 Message-Id: <83vayhkogv.fsf@gnu.org> From: Eli Zaretskii In-reply-to: (message from Stefan Monnier on Tue, 30 Aug 2016 17:21:38 -0400) References: <8360qim5sv.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -6.5 (------) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.5 (------) > From: Stefan Monnier > Cc: 24340@debbugs.gnu.org > Date: Tue, 30 Aug 2016 17:21:38 -0400 > > > Thanks for the patch, but I don't want to make such pervasive changes > > for this problem. > > It was for another problem (the fact that b-c-f was not called at all in > a corner case). > > This is a problem (it breaks the promise that b-c-f covers all > following changes) that shows up everytime insert-file-contents is > called with a non-nil `replace' argument, which is a completely > different situation. It's a common case. Then you misunderstood me. My suggestion was meant for any use of insert-file-contents with REPLACE non-nil. > > I thought we agreed that a single call to before-change hooks for the > > entire buffer would be an okay solution for this scenario. > > Kind of, but I think it'd be a kludge compared to the patch I submitted. > This said, if you insist on fixing it some other way, that's fine by me. I'm sorry, but I must insist. The change I proposed is the only one in insert-file-contents for that use case that I'm prepared to consider in the current situation. (We could also leave this bug open, until such time as a more thorough refactoring is done of the related functionalities.) Deeper changes are exactly the can of worms that I don't want to open to fix just this one use case, especially since Emacs 25.2 will almost certainly be branched from what now is the master branch. Such deeper changes were what Alan proposed in the first place (with a similar patch), and I already said I didn't want to do that. > Its effect is to call prepare_to_modify_buffer one more time in > a function which already calls it moments later. Seems safer than about > 90% of the changes I installed in the past. I disagree about the safety. The various things that prepare_to_modify_buffer does is a hodge-podge of unrelated jobs that were lumped there for "histerical raisins". Just look at this list of what it does: . bitches if the buffer is read-only . signals an undoable change . sets the buffer's redisplay flag . optionally preserves a buffer position by holding it in a pointer . locks the file . saves the text of selected region . calls before-change-functions and modification hooks stored in text properties . sets deactivate-mark non-nil Doing this in the delete portions of insert-file-contents will send ripples everywhere in Emacs, and I don't want to risk that without being able to test that everything still works as before. Each one of these jobs should be carefully analyzed, decided whether we want to do it for each chunk of text we change, and separate controls designed and implemented for each of them we want to be able to control independently of the others. Anything else is just kludging patch upon patch and hoping they will somehow DTRT. > I agree that the let-binding is a bit ugly, but that's the kludge we use > currently, so my patch doesn't make it much uglier in this respect. > A cleaner way to handle this issue might be to introduce a new > buffer-local variable (like inhibit-file-supersession-check) which we'd > let-bind to t around the whole function I think a cleaner way is to change the model of how we partition piecemeal changes, when we signal the changes and when don't, when we ask the user about supersession-threat, etc. The current model is fundamentally flawed, if we want to use the buffer-change hooks in the ways that emerged from these discussions. E.g., locking the file should clearly be decoupled from signaling a change, because you generally lock the file only once for a batch of changes, and the same for the read-only checks. Thanks. From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 31 Aug 2016 14:46:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Eli Zaretskii Cc: 24340@debbugs.gnu.org Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.14726547241213 (code B ref 24340); Wed, 31 Aug 2016 14:46:02 +0000 Received: (at 24340) by debbugs.gnu.org; 31 Aug 2016 14:45:24 +0000 Received: from localhost ([127.0.0.1]:45559 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf6lS-0000JN-CK for submit@debbugs.gnu.org; Wed, 31 Aug 2016 10:45:24 -0400 Received: from ironport2-out.teksavvy.com ([206.248.154.181]:34401) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf6lK-0000J4-Rt for 24340@debbugs.gnu.org; Wed, 31 Aug 2016 10:45:16 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0ByEgCJI6tX/+V1oWxdGwEBAYMnhEuFULJahhcEAgKBXTwRAQEBAQEBAV0nhF8BAQMBViMFCws0EhQYDSSIPAjBXAEBAQEGAiWKd4l+HQWZPJhZhXyOcIE8NCCCEhyBaCCHXgEBAQ X-IPAS-Result: A0ByEgCJI6tX/+V1oWxdGwEBAYMnhEuFULJahhcEAgKBXTwRAQEBAQEBAV0nhF8BAQMBViMFCws0EhQYDSSIPAjBXAEBAQEGAiWKd4l+HQWZPJhZhXyOcIE8NCCCEhyBaCCHXgEBAQ X-IronPort-AV: E=Sophos;i="5.28,500,1464667200"; d="scan'208";a="270429804" Received: from 108-161-117-229.dsl.teksavvy.com (HELO ceviche.home) ([108.161.117.229]) by smtp.teksavvy.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 31 Aug 2016 10:45:05 -0400 Received: by ceviche.home (Postfix, from userid 20848) id BBFCB66287; Wed, 31 Aug 2016 10:48:09 -0400 (EDT) From: Stefan Monnier Message-ID: References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> Date: Wed, 31 Aug 2016 10:48:09 -0400 In-Reply-To: <83vayhkogv.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 31 Aug 2016 17:22:24 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.3 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.3 (/) > I'm sorry, but I must insist. The change I proposed is the only one > in insert-file-contents for that use case that I'm prepared to > consider in the current situation. (We could also leave this bug > open, until such time as a more thorough refactoring is done of the > related functionalities.) Deeper changes are exactly the can of worms > that I don't want to open to fix just this one use case, especially > since Emacs 25.2 will almost certainly be branched from what now is > the master branch. Such deeper changes were what Alan proposed in the > first place (with a similar patch), and I already said I didn't want > to do that. Right: I do not intend for my patch to go to 25.2. But for Emacs-26, it seems there's plenty of time to find and fix any possible fallout. > I think a cleaner way is to change the model of how we partition > piecemeal changes, when we signal the changes and when don't, when we > ask the user about supersession-threat, etc. The current model is > fundamentally flawed, if we want to use the buffer-change hooks in the > ways that emerged from these discussions. All I care about is for any change to the buffer to be announced by a prior b-c-f. Stefan From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 31 Aug 2016 15:04:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Stefan Monnier Cc: 24340@debbugs.gnu.org Reply-To: Eli Zaretskii Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.14726558132999 (code B ref 24340); Wed, 31 Aug 2016 15:04:01 +0000 Received: (at 24340) by debbugs.gnu.org; 31 Aug 2016 15:03:33 +0000 Received: from localhost ([127.0.0.1]:45572 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf732-0000mC-Sp for submit@debbugs.gnu.org; Wed, 31 Aug 2016 11:03:33 -0400 Received: from eggs.gnu.org ([208.118.235.92]:40283) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf72w-0000lq-GI for 24340@debbugs.gnu.org; Wed, 31 Aug 2016 11:03:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bf72n-00080S-M0 for 24340@debbugs.gnu.org; Wed, 31 Aug 2016 11:03:17 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:44611) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bf72n-00080O-J7; Wed, 31 Aug 2016 11:03:13 -0400 Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:2969 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bf72m-0005kT-N0; Wed, 31 Aug 2016 11:03:13 -0400 Date: Wed, 31 Aug 2016 18:03:12 +0300 Message-Id: <83inuhkmkv.fsf@gnu.org> From: Eli Zaretskii In-reply-to: (message from Stefan Monnier on Wed, 31 Aug 2016 10:48:09 -0400) References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -6.5 (------) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.5 (------) > From: Stefan Monnier > Cc: 24340@debbugs.gnu.org > Date: Wed, 31 Aug 2016 10:48:09 -0400 > > > I'm sorry, but I must insist. The change I proposed is the only one > > in insert-file-contents for that use case that I'm prepared to > > consider in the current situation. (We could also leave this bug > > open, until such time as a more thorough refactoring is done of the > > related functionalities.) Deeper changes are exactly the can of worms > > that I don't want to open to fix just this one use case, especially > > since Emacs 25.2 will almost certainly be branched from what now is > > the master branch. Such deeper changes were what Alan proposed in the > > first place (with a similar patch), and I already said I didn't want > > to do that. > > Right: I do not intend for my patch to go to 25.2. But for Emacs-26, > it seems there's plenty of time to find and fix any possible fallout. Since I believe master will be used for 25.2, we cannot currently push anything that must wait for 26. > > I think a cleaner way is to change the model of how we partition > > piecemeal changes, when we signal the changes and when don't, when we > > ask the user about supersession-threat, etc. The current model is > > fundamentally flawed, if we want to use the buffer-change hooks in the > > ways that emerged from these discussions. > > All I care about is for any change to the buffer to be announced by > a prior b-c-f. My proposed simple change satisfies this requirement, albeit the effect is less optimal. From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 31 Aug 2016 17:48:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Eli Zaretskii Cc: 24340@debbugs.gnu.org Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147266562718300 (code B ref 24340); Wed, 31 Aug 2016 17:48:02 +0000 Received: (at 24340) by debbugs.gnu.org; 31 Aug 2016 17:47:07 +0000 Received: from localhost ([127.0.0.1]:45631 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf9bO-0004l5-C0 for submit@debbugs.gnu.org; Wed, 31 Aug 2016 13:47:06 -0400 Received: from ironport2-out.teksavvy.com ([206.248.154.181]:64388) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bf9bN-0004kW-Cs for 24340@debbugs.gnu.org; Wed, 31 Aug 2016 13:47:05 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0ByEgCJI6tX/+V1oWxdGwEBgyiES4VQsF2BfYYXBAICgV07EgEBAQEBAQFdJ4RfAQEDAVYjBQsLNBIUGA0kiDwIwVwBAQEBBgIlineJfh0BBJk8mFmFfI5wgTwlDiGCEhyBaCCHXgEBAQ X-IPAS-Result: A0ByEgCJI6tX/+V1oWxdGwEBgyiES4VQsF2BfYYXBAICgV07EgEBAQEBAQFdJ4RfAQEDAVYjBQsLNBIUGA0kiDwIwVwBAQEBBgIlineJfh0BBJk8mFmFfI5wgTwlDiGCEhyBaCCHXgEBAQ X-IronPort-AV: E=Sophos;i="5.28,500,1464667200"; d="scan'208";a="270545769" Received: from 108-161-117-229.dsl.teksavvy.com (HELO ceviche.home) ([108.161.117.229]) by smtp.teksavvy.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 31 Aug 2016 13:46:59 -0400 Received: by ceviche.home (Postfix, from userid 20848) id D244066287; Wed, 31 Aug 2016 13:50:03 -0400 (EDT) From: Stefan Monnier Message-ID: References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> <83inuhkmkv.fsf@gnu.org> Date: Wed, 31 Aug 2016 13:50:03 -0400 In-Reply-To: <83inuhkmkv.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 31 Aug 2016 18:03:12 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.3 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.3 (/) > Since I believe master will be used for 25.2, [ Making an effort not to comment on that ;-) ] > we cannot currently push anything that must wait for 26. I can wait for this issue to clear up. >> > I think a cleaner way is to change the model of how we partition >> > piecemeal changes, when we signal the changes and when don't, when we >> > ask the user about supersession-threat, etc. The current model is >> > fundamentally flawed, if we want to use the buffer-change hooks in the >> > ways that emerged from these discussions. >> All I care about is for any change to the buffer to be announced by >> a prior b-c-f. > My proposed simple change satisfies this requirement, albeit the > effect is less optimal. Yes, as I said, I prefer my patch, but your suggestion is acceptable as well. Stefan From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 02 Oct 2016 01:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Eli Zaretskii Cc: 24340@debbugs.gnu.org Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147537070310939 (code B ref 24340); Sun, 02 Oct 2016 01:12:02 +0000 Received: (at 24340) by debbugs.gnu.org; 2 Oct 2016 01:11:43 +0000 Received: from localhost ([127.0.0.1]:41408 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bqVJf-0002qN-K5 for submit@debbugs.gnu.org; Sat, 01 Oct 2016 21:11:43 -0400 Received: from ironport2-out.teksavvy.com ([206.248.154.181]:27966) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bqVJd-0002q8-Is for 24340@debbugs.gnu.org; Sat, 01 Oct 2016 21:11:42 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0DdBQALW9BX/4Pjr2xdGwEBAQMBAQGDLQEBAQEBHoRNhVCvdoIDhhYEAgKBaToTAQIBAQEBAQEBXieEYgEBAwFWIwULCzQSFBgNJIhVCLxVAQEBAQYCJYp9iX8dAQSZWZEVAYd3hguPDYE+IAMxhGwghgoBAQE X-IPAS-Result: A0DdBQALW9BX/4Pjr2xdGwEBAQMBAQGDLQEBAQEBHoRNhVCvdoIDhhYEAgKBaToTAQIBAQEBAQEBXieEYgEBAwFWIwULCzQSFBgNJIhVCLxVAQEBAQYCJYp9iX8dAQSZWZEVAYd3hguPDYE+IAMxhGwghgoBAQE X-IronPort-AV: E=Sophos;i="5.30,296,1470715200"; d="scan'208";a="274290300" Received: from 108-175-227-131.dsl.teksavvy.com (HELO ceviche.home) ([108.175.227.131]) by smtp.teksavvy.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 01 Oct 2016 21:11:35 -0400 Received: by ceviche.home (Postfix, from userid 20848) id 8805A66293; Sat, 1 Oct 2016 21:11:34 -0400 (EDT) From: Stefan Monnier Message-ID: References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> <83inuhkmkv.fsf@gnu.org> Date: Sat, 01 Oct 2016 21:11:34 -0400 In-Reply-To: <83inuhkmkv.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 31 Aug 2016 18:03:12 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.3 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.3 (/) > Since I believe master will be used for 25.2, we cannot currently push > anything that must wait for 26. Now that master has been labeled "for 26.1", can we push a patch to it? I'd happily push my patch to it, but if you prefer a different approach, feel free to push another patch there instead. Stefan From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 02 Oct 2016 07:20:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Stefan Monnier Cc: 24340@debbugs.gnu.org Reply-To: Eli Zaretskii Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147539274613999 (code B ref 24340); Sun, 02 Oct 2016 07:20:01 +0000 Received: (at 24340) by debbugs.gnu.org; 2 Oct 2016 07:19:06 +0000 Received: from localhost ([127.0.0.1]:41490 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bqb3C-0003di-LO for submit@debbugs.gnu.org; Sun, 02 Oct 2016 03:19:06 -0400 Received: from eggs.gnu.org ([208.118.235.92]:45166) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bqb3B-0003dD-0h for 24340@debbugs.gnu.org; Sun, 02 Oct 2016 03:19:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bqb32-0001pL-MZ for 24340@debbugs.gnu.org; Sun, 02 Oct 2016 03:18:59 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:43364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bqb32-0001ot-J6; Sun, 02 Oct 2016 03:18:56 -0400 Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:2968 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bqb30-0000wE-MA; Sun, 02 Oct 2016 03:18:55 -0400 Date: Sun, 02 Oct 2016 10:19:05 +0300 Message-Id: <83vaxbjk0m.fsf@gnu.org> From: Eli Zaretskii In-reply-to: (message from Stefan Monnier on Sat, 01 Oct 2016 21:11:34 -0400) References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> <83inuhkmkv.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -8.0 (--------) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -8.0 (--------) > From: Stefan Monnier > Cc: 24340@debbugs.gnu.org > Date: Sat, 01 Oct 2016 21:11:34 -0400 > > > Since I believe master will be used for 25.2, we cannot currently push > > anything that must wait for 26. > > Now that master has been labeled "for 26.1", can we push a patch to it? > I'd happily push my patch to it, but if you prefer a different approach, > feel free to push another patch there instead. I'm okay with your patch only if you promise to be there for fixing any fallout. It scares me quite a lot, especially the fact that it fixes a single use case by making broad changes across the board. Once again, my suggestion is instead to announce a change in the entire buffer, as that is what revert-buffer is all about. From unknown Tue Aug 19 14:22:17 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Stefan Monnier Subject: bug#24340: closed (Re: bug#24340: insert-file-contents calls before-change-functions too late) Message-ID: References: X-Gnu-PR-Message: they-closed 24340 X-Gnu-PR-Package: emacs Reply-To: 24340@debbugs.gnu.org Date: Mon, 03 Oct 2016 13:49:02 +0000 Content-Type: multipart/mixed; boundary="----------=_1475502542-23402-1" This is a multi-part message in MIME format... ------------=_1475502542-23402-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #24340: insert-file-contents calls before-change-functions too late which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 24340@debbugs.gnu.org. --=20 24340: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D24340 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1475502542-23402-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 24340-done) by debbugs.gnu.org; 3 Oct 2016 13:48:17 +0000 Received: from localhost ([127.0.0.1]:42709 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1br3bM-00063p-UR for submit@debbugs.gnu.org; Mon, 03 Oct 2016 09:48:17 -0400 Received: from chene.dit.umontreal.ca ([132.204.246.20]:55069) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1br3bL-00063i-Jl for 24340-done@debbugs.gnu.org; Mon, 03 Oct 2016 09:48:15 -0400 Received: from ceviche.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id u93DnBPa005374; Mon, 3 Oct 2016 09:49:11 -0400 Received: by ceviche.home (Postfix, from userid 20848) id 26EB36622D; Mon, 3 Oct 2016 09:48:14 -0400 (EDT) From: Stefan Monnier To: Eli Zaretskii Subject: Re: bug#24340: insert-file-contents calls before-change-functions too late Message-ID: References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> <83inuhkmkv.fsf@gnu.org> <83vaxbjk0m.fsf@gnu.org> Date: Mon, 03 Oct 2016 09:48:14 -0400 In-Reply-To: <83vaxbjk0m.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 02 Oct 2016 10:19:05 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-NAI-Spam-Flag: NO X-NAI-Spam-Level: X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0.1 X-NAI-Spam-Rules: 2 Rules triggered GEN_SPAM_FEATRE=0.1, RV5816=0 X-NAI-Spam-Version: 2.3.0.9418 : core <5816> : inlines <5303> : streams <1710375> : uri <2300697> X-Spam-Score: -4.0 (----) X-Debbugs-Envelope-To: 24340-done Cc: 24340-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.0 (----) > I'm okay with your patch only if you promise to be there for fixing > any fallout. It scares me quite a lot, especially the fact that it > fixes a single use case by making broad changes across the board. Great, installed, Stefan ------------=_1475502542-23402-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 30 Aug 2016 18:39:32 +0000 Received: from localhost ([127.0.0.1]:44674 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benwa-0007cY-9T for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:32 -0400 Received: from eggs.gnu.org ([208.118.235.92]:52412) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benwZ-0007cL-23 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benwS-0001lC-SX for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:26 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:58445) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwS-0001kr-P5 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43348) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwQ-0000mk-BK for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benwL-0001io-88 for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:21 -0400 Received: from chene.dit.umontreal.ca ([132.204.246.20]:51594) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwL-0001ia-3D for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:17 -0400 Received: from ceviche.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id u7UIdtsT003418 for ; Tue, 30 Aug 2016 14:39:55 -0400 Received: by ceviche.home (Postfix, from userid 20848) id 7F8A966274; Tue, 30 Aug 2016 14:42:19 -0400 (EDT) From: Stefan Monnier To: bug-gnu-emacs@gnu.org Subject: insert-file-contents calls before-change-functions too late Date: Tue, 30 Aug 2016 14:42:19 -0400 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-NAI-Spam-Flag: NO X-NAI-Spam-Level: X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0.6 X-NAI-Spam-Rules: 4 Rules triggered BEC_TRC1=0.2, BEC_TRC1_W_GEN_SPAM_FEATRE=0.2, GEN_SPAM_FEATRE=0.2, RV5782=0 X-NAI-Spam-Version: 2.3.0.9418 : core <5782> : inlines <5157> : streams <1692582> : uri <2278217> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.0 (----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.0 (----) 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 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. */ ------------=_1475502542-23402-1-- From unknown Tue Aug 19 14:22:17 2025 X-Loop: help-debbugs@gnu.org Subject: bug#24340: insert-file-contents calls before-change-functions too late Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 03 Oct 2016 14:14:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: To: Stefan Monnier Cc: 24340@debbugs.gnu.org Reply-To: Eli Zaretskii Received: via spool by 24340-submit@debbugs.gnu.org id=B24340.147550401627100 (code B ref 24340); Mon, 03 Oct 2016 14:14:02 +0000 Received: (at 24340) by debbugs.gnu.org; 3 Oct 2016 14:13:36 +0000 Received: from localhost ([127.0.0.1]:43105 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1br3zs-000732-DR for submit@debbugs.gnu.org; Mon, 03 Oct 2016 10:13:36 -0400 Received: from eggs.gnu.org ([208.118.235.92]:48373) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1br3zn-00072g-CK for 24340@debbugs.gnu.org; Mon, 03 Oct 2016 10:13:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1br3zf-0002IN-2D for 24340@debbugs.gnu.org; Mon, 03 Oct 2016 10:13:26 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_40,RP_MATCHES_RCVD autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:59960) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1br3ze-0002I3-Us; Mon, 03 Oct 2016 10:13:22 -0400 Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:1755 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1br3zc-0005ia-VO; Mon, 03 Oct 2016 10:13:21 -0400 Date: Mon, 03 Oct 2016 17:13:34 +0300 Message-Id: <83bmz1plkh.fsf@gnu.org> From: Eli Zaretskii In-reply-to: (message from Stefan Monnier on Mon, 03 Oct 2016 09:48:14 -0400) References: <8360qim5sv.fsf@gnu.org> <83vayhkogv.fsf@gnu.org> <83inuhkmkv.fsf@gnu.org> <83vaxbjk0m.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -7.7 (-------) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -7.7 (-------) > From: Stefan Monnier > Cc: 24340-done@debbugs.gnu.org > Date: Mon, 03 Oct 2016 09:48:14 -0400 > > > I'm okay with your patch only if you promise to be there for fixing > > any fallout. It scares me quite a lot, especially the fact that it > > fixes a single use case by making broad changes across the board. > > Great, installed, Thank you very much for considering my opinions.