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

[Alan Mackenzie]

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