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: Sat, 21 Dec 2019 21:47:52 +0000 Message-ID: <20191221214751.GB8692@ACM> References: <20191221172324.GA8692@ACM> <83k16pzgzu.fsf@gnu.org> 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="229151"; 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 Sat Dec 21 22:48:41 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 1iimc1-000xNl-CD for ged-emacs-devel@m.gmane.org; Sat, 21 Dec 2019 22:48:37 +0100 Original-Received: from localhost ([::1]:42346 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iimbz-0005xT-N8 for ged-emacs-devel@m.gmane.org; Sat, 21 Dec 2019 16:48:35 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:38177) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iimbW-0005YZ-NJ for emacs-devel@gnu.org; Sat, 21 Dec 2019 16:48:07 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iimbV-0008BC-0F for emacs-devel@gnu.org; Sat, 21 Dec 2019 16:48:06 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:16325 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1iimbU-0007bd-4W for emacs-devel@gnu.org; Sat, 21 Dec 2019 16:48:04 -0500 Original-Received: (qmail 87555 invoked by uid 3782); 21 Dec 2019 21:47:54 -0000 Original-Received: from acm.muc.de (p2E5D57F0.dip0.t-ipconnect.de [46.93.87.240]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sat, 21 Dec 2019 22:47:52 +0100 Original-Received: (qmail 28535 invoked by uid 1000); 21 Dec 2019 21:47:52 -0000 Content-Disposition: inline In-Reply-To: <83k16pzgzu.fsf@gnu.org> 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:243549 Archived-At: Hello, Eli. Sorry my last post was not thought through. I hope this one is better. On Sat, Dec 21, 2019 at 20:11:01 +0200, Eli Zaretskii wrote: > > Date: Sat, 21 Dec 2019 17:23:24 +0000 > > From: Alan Mackenzie > > > > /* FIXME: Call signal_after_change! */ > > + beg = PT; > Can you tell why you needed this variable and the assignment? AFAIK, > PT doesn't change when we call decode_coding_c_string. I'd misunderstood decode_coding_c_string. You're right, PT doesn't change with this macro. With this, the arguments I was getting to after-change-functions were wrong. However, I've kept BEG for the next version of the patch. > > decode_coding_c_string (&process_coding, > > (unsigned char *) buf, nread, curbuf); > > unbind_to (count1, Qnil); > > + signal_after_change (beg, 0, PT - beg); > > if (display_on_the_fly > > && CODING_REQUIRE_DETECTION (&saved_coding) > > && ! CODING_REQUIRE_DETECTION (&process_coding)) > > > > > > , and this appears to solve the OP's problem. > > > > However, a few lines further on, there's a del_range_2 call inside a condition > > I don't understand (though might, with a great deal of study). I suspect that > > the call to signal_after_change ought to take this del_range_2 into account, > > possibly coming after it. > > > > Would somebody who's familiar with this bit of callproc.c please help me out > > here, and explain what this call to del_range_2 is doing, and whether there's > > anything basically wrong with my simple-minded addition of > > signal_after_change. > I'm not sure what you want to hear. The del_range_2 call deletes the > just-inserted text, because the condition means that text was inserted > using the wrong coding-system to decode the incoming bytes. What does > that mean for the modification hooks, I don't know: the > before-change-functions were already called, but nothing was inserted > from the Lisp application's POV, so if you insist on having before and > after hooks to be called in pairs, you are in a conundrum. I think I've solved this with the new variable prepared_position. It records the position of BEG for prepare_to_modify_buffer, and only calls that function if it hasn't already done so for that BEG. It's not an elegant solution, but I think it will work. > It's possible that we should simplify all this by calling the before > hooks just once before the loop and the after hooks just once after > the loop, instead of calling them for each individual chunk inside the > loop, but again I don't know what that means for applications which > expects these hook calls to pair. I don't think this is necessary. (If we positively want to do it, that's a different matter.) Here's the latest version of my patch. It's only been slightly tested, but it doesn't produce any unexpected "successes" from 'make check'. diff --git a/src/callproc.c b/src/callproc.c index b51594c2d5..34da7af863 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_position = 0; + ptrdiff_t beg; while (1) { @@ -780,7 +782,13 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, ; else if (NILP (BVAR (current_buffer, enable_multibyte_characters)) && ! CODING_MAY_REQUIRE_DECODING (&process_coding)) - insert_1_both (buf, nread, nread, 0, 1, 0); + { + beg = PT; + insert_1_both (buf, nread, nread, 0, prepared_position < PT, 0); + if (prepared_position < PT) + prepared_position = PT; + signal_after_change (beg, 0, PT - beg); + } else { /* We have to decode the input. */ Lisp_Object curbuf; @@ -788,7 +796,11 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, XSETBUFFER (curbuf, current_buffer); /* FIXME: Call signal_after_change! */ - prepare_to_modify_buffer (PT, PT, NULL); + if (prepared_position < PT) + { + prepare_to_modify_buffer (PT, PT, NULL); + prepared_position = PT; + } /* 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 +834,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; -- Alan Mackenzie (Nuremberg, Germany).