Subject: bug#25111: (Inaccurate documentation of inhibit-modification-hooks)

From: Alan Mackenzie <[email-masked]>
Date: Sun, 9 Jun 2019 20:45:25 +0000

Hello, Eli.

On Sun, Jun 09, 2019 at 15:00:16 +0300, Eli Zaretskii wrote:
> > Date: Tue, 04 Jun 2019 17:36:12 +0300
> > From: Eli Zaretskii <[email-masked]>
> > Cc: [email-masked], [email-masked], [email-masked]

> > > Date: Tue, 4 Jun 2019 09:32:41 +0000
> > > Cc: Eli Zaretskii <[email-masked]>, Phillip Lord <[email-masked]>,
> > >   [email-masked]
> > > From: Alan Mackenzie <[email-masked]>

> > > > > This is difficult to document coherently.

> > > > And confusing, as evidenced by the fact that we both got confused about
> > > > it in this very thread :)

> > > > > My proposal of last week was to fix the code, also to bind
> > > > > inhibit-modification-hooks to t for the modification-hooks overlay
> > > > > property, even though this would be an incompatibility in Lisp.

> > > How about this?

> > Please wait with this for a few days.

> OK, after re-reading the discussions and the code, I don't think we
> should make the incompatible change suggested by Alan.  We haven't
> bound inhibit-modification-hooks to t in the text-property hooks since
> the day the code was written, 24 years ago, so it makes no sense to me
> to do that now.  Let's document the exception and move on.

Thanks for looking at this and taking the decision.

> Noam's last patch LGTM, with the single minor gotcha:

> > +When Emacs calls these functions, @code{inhibit-modification-hooks} is
> > +set to @code{nil}, unlike for change hooks.

> This is from the part that changes the "Special Properties" node, and
> it's inaccurate: we don't bind inhibit-modification-hooks to nil, we
> just leave it at its previous binding.  This distinction is important
> in recursive calls, when the caller caused inhibit-modification-hooks
> to be bound to non-nil.

Yes.  The "is set" formulation is ambiguous.  It could mean "gets set
(bound)", which is how you read it; it could also mean "happens to be set
to (at the time)", which was how I intended it.  Ambiguous writing isn't
good, so I suggest:

"When Emacs calls this function, @code{inhibit-modification-hooks} is
left at its current value; unlike for change hooks, it does not get bound
to non-@code{nil}.

> Another minor comment, although not to the proposed text, is that the
> fact that inhibit-modification-hooks is bound to t when the hook
> specified by the overlay properties are called is because those hooks
> are called from within signal_before_change and signal_after_change,
> which perform these bindings, and the bindings stay in effect both for
> before/after-change-functions and for hooks specified by overlay
> properties.  By contrast, the hooks specified by text properties are
> called before that binding becomes in effect (which is why we need a
> separate check whether inhibit_modification_hooks are non-nil inside
> verify_interval_modification, which calls the text-property hooks).

Thanks, that's helpful.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).