Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?

[Alan Mackenzie <acm@muc.de>]

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