Re: Lisp primitives and their calling of the change hooks

[Alan Mackenzie]

Hello, Stefan and Eli.

On Sun, Jan 07, 2018 at 14:04:46 -0500, Stefan Monnier wrote:
> > I think only the former makes sense, because this function is actually
> > a replacement function.

> When we perform a "replacement" from Elisp, we do it with insert +
> delete-region so there are two calls to each of b-c-f and a-c-f.
> So two calls also makes sense.

> We could even have one call to at the beginning b-c-f followed by two
> calls to a-c-f (one right after inserting the new text and one after
> deleting the old text).  That would also be acceptable.

> And in reality, this choice doesn't really matter (as evidenced by the
> fact that noone noticed those calls were missing until now).

> We should just pick the one that's easier to implement (and in case we
> still a preference because the implementation is just as easy, the
> "single calls to b-c-f/a-c-f" is of course the better option).

Yes, that's what I've done.  Each zlib-decompress-region now has two
separate pairs of b/a-c-f, one for insert, then the other for delete.
This is the way that other "coding" primitives, like
base64-encode-region work.

> > Also note that zlib-decompress-region works only in unibyte buffers,
> > so in practice almost every caller will immediately call
> > decode-coding-region or its ilk, which calls the hooks again.  But
> > unlike zlib-decompress-region, the decoding stuff will be able to0
> > report character positions, not byte positions.

> In practice, I'd also expect both b-c-f and a-c-f to be nil (or
> equivalent) when we call zlib-decompress-region.

Yes.  But it's nice to have things consistent.  :-)

Anyhow, here's the patch, which appears to work as described above.  Any
comments?  Otherwise I'll commit it.



diff --git a/src/decompress.c b/src/decompress.c
index 41de6da1dd..07545544aa 100644
--- a/src/decompress.c
+++ b/src/decompress.c
@@ -141,6 +141,10 @@ This function can be called only in unibyte buffers.  */)
      the same.  */
   istart = XINT (start);
   iend = XINT (end);
+
+  /* Do the following before manipulating the gap. */
+  modify_text (iend, iend);
+
   move_gap_both (iend, iend);
 
   stream.zalloc = Z_NULL;
@@ -190,6 +194,8 @@ This function can be called only in unibyte buffers.  */)
     }
   while (inflate_status == Z_OK);
 
+  signal_after_change (iend, 0, unwind_data.nbytes);
+
   if (inflate_status != Z_STREAM_END)
     return unbind_to (count, Qnil);
 


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).