bug#36190: 27.0.50; `put-text-property' etc. with buffer argument calls current buffer's `after-change-functions'

[Pip Cet]

On Thu, Jun 13, 2019 at 7:06 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Thu, 13 Jun 2019 18:48:23 +0000
> > Cc: 36190@debbugs.gnu.org
> >
> > > > As far as I can tell, this makes `put-text-property' with a buffer
> > > > argument pretty useless.
> > >
> > > Only if you have a buffer-local value of after-change-functions.
> >
> > I'm not sure what you're saying.
>
> I'm saying that the buffer argument to put-text-property is pretty
> useless only if you consider after-change-functions.  The primary
> purpose of put-text-property is to modify text properties, not to call
> after-change-functions.  For that primary purpose, the buffer argument
> is not useless.

Thanks for clarifying. I suppose you could say that it's
after-change-functions (local, global, plus overlay modification
hooks) that have become useless, or will be called spuriously and with
potentially nonsensical arguments.

For example, this would break:

(push (lambda (beg end len)
        (message "%S" (buffer-substring beg end)))
      after-change-functions)

> > That seems pretty wrong to me. In which cases do you think we're
> > seeing the right behavior?
>
> Where did I say that this behavior was right?

You said "only if", so I assumed you were asserting the contrapositive.

> > Here's a first patch, which adds a "buffer" argument to
> > signal_after_change, to be explicit about where the change happens. It
> > should be pretty cheap in the case where we don't switch buffers.
>
> Not sure I have a clear idea of how you intend to use that additional
> argument.  Are you suggesting that we switch to that buffer?

Yes:

@@ -2183,6 +2184,9 @@ signal_after_change (ptrdiff_t charpos,
ptrdiff_t lendel, ptrdiff_t lenins)
   if (inhibit_modification_hooks)
     return;

+  record_unwind_current_buffer ();
+  set_buffer_internal (buffer);
+
   /* If we are deferring calls to the after-change functions
      and there are no before-change functions,
      just record the args that we were going to use.  */

> If so,
> how is that different from not using the buffer argument at all, and
> instead wrapping the call to put-text-property with
> with-current-buffer?

I don't think they're usefully different, but put-text-property
doesn't appear to check the buffer is still live.

> Also, passing current_buffer sounds redundant to me anyway, because in
> that case signal_after_change will not need to do anything that it
> doesn't already do.  I would pass NULL instead.

May I ask why? I think passing current_buffer is the clearest signal
we can send to someone reusing the code that they might have to change
this if they're dealing with more than one buffer.

As a practical matter, it's hard to change the text property functions
to use NULL when passed a nil argument, so we'd have functions using
current_buffer and others using NULL, and that seems needlessly
inconsistent.