From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not? Date: Tue, 24 Dec 2019 12:51:11 +0000 Message-ID: <20191224125111.GC3992@ACM> References: <20191221172324.GA8692@ACM> <83k16pzgzu.fsf@gnu.org> <20191221214751.GB8692@ACM> <83sglcxl1q.fsf@gnu.org> <20191224094724.GA3992@ACM> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="20452"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.10.1 (2018-07-13) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Dec 24 13:52:01 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1ijjfL-00057e-U4 for ged-emacs-devel@m.gmane.org; Tue, 24 Dec 2019 13:52:00 +0100 Original-Received: from localhost ([::1]:38750 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ijjfK-00076m-1D for ged-emacs-devel@m.gmane.org; Tue, 24 Dec 2019 07:51:58 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36003) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ijjeg-0006gE-0H for emacs-devel@gnu.org; Tue, 24 Dec 2019 07:51:19 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ijjee-0001Fg-PJ for emacs-devel@gnu.org; Tue, 24 Dec 2019 07:51:17 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:35886 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1ijjed-000199-7v for emacs-devel@gnu.org; Tue, 24 Dec 2019 07:51:16 -0500 Original-Received: (qmail 73002 invoked by uid 3782); 24 Dec 2019 12:51:13 -0000 Original-Received: from acm.muc.de (p4FE15B64.dip0.t-ipconnect.de [79.225.91.100]) by colin.muc.de (tmda-ofmipd) with ESMTP; Tue, 24 Dec 2019 13:51:11 +0100 Original-Received: (qmail 7242 invoked by uid 1000); 24 Dec 2019 12:51:11 -0000 Content-Disposition: inline In-Reply-To: <20191224094724.GA3992@ACM> X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x [fuzzy] X-Received-From: 193.149.48.1 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:243606 Archived-At: Hello, Eli. On Tue, Dec 24, 2019 at 09:47:24 +0000, Alan Mackenzie wrote: [ .... ] > The point is not to call prepare_to_modify_buffer twice at the same > position. prepared_position records the most recent place p_t_m_b was > called, thus enabling us to avoid calling it there again, should we > remove the already inserted text, decode it, and insert it again. [ .... ] > > This really ugly, IMO. And the code logic is very hard to follow and > > verify its correctness, given the various use cases. [ .... ] > I think the basic idea of my change is sound, but it is suboptimally > coded. My confusion, I think, arose from the use of the PREPARE > parameter in the call to insert_1_both, which creates several different > cases. This is a bad idea. If instead we put in a single call to > prepare_to_modify_buffer, this should be relatively easy to follow. One > or two comments would also be helpful. I think the following patch is better. What do you think? diff --git a/src/callproc.c b/src/callproc.c index b51594c2d5..0df0292633 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -746,6 +746,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, int carryover = 0; bool display_on_the_fly = display_p; struct coding_system saved_coding = process_coding; + ptrdiff_t prepared_pos = 0; /* prepare_to_modify_buffer was last + called here. */ while (1) { @@ -774,21 +776,29 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, break; } + /* Call `prepare_to_modify_buffer' exactly once for PT. */ + if ((prepared_pos < PT) && nread) + { + prepare_to_modify_buffer (PT, PT, NULL); + prepared_pos = PT; + } + /* Now NREAD is the total amount of data in the buffer. */ if (!nread) ; else if (NILP (BVAR (current_buffer, enable_multibyte_characters)) && ! CODING_MAY_REQUIRE_DECODING (&process_coding)) - insert_1_both (buf, nread, nread, 0, 1, 0); + { + insert_1_both (buf, nread, nread, 0, 0, 0); + signal_after_change (PT, 0, nread); + } else { /* We have to decode the input. */ Lisp_Object curbuf; ptrdiff_t count1 = SPECPDL_INDEX (); XSETBUFFER (curbuf, current_buffer); - /* FIXME: Call signal_after_change! */ - prepare_to_modify_buffer (PT, PT, NULL); /* We cannot allow after-change-functions be run during decoding, because that might modify the buffer, while we rely on process_coding.produced to @@ -822,6 +832,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, continue; } + signal_after_change (PT, 0, process_coding.produced_char); TEMP_SET_PT_BOTH (PT + process_coding.produced_char, PT_BYTE + process_coding.produced); carryover = process_coding.carryover_bytes; > > Thanks. -- Alan Mackenzie (Nuremberg, Germany).