GNU bug report logs - #25111
How modification-hooks let-bind inhibit-modification-hooks?

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25111 in the body.
You can then email your comments to 25111 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Phillip Lord" <phillip.lord <at> russet.org.uk>
To: bug-gnu-emacs <at> gnu.org
Date: Sun, 4 Dec 2016 20:53:24 -0000

The documentation for "modification-hooks" on overlays says:

     If these functions modify the buffer, they should bind
     ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
     confusing the internal mechanism that calls these hooks.

But as far as I can see, the only place these gets called
"signal_after_change"
and "signal_before_change", inhibit-modification-hooks is already specbound
to t, so this advice is unnecessary.

Also, the documentation for inhibit-modification-hooks says:

     If you do want modification hooks to be run in a particular
     piece of code that is itself run from a modification hook, then
     rebind locally ‘inhibit-modification-hooks’ to ‘nil’.

which suggests that, in fact, it is possible to call the modification
hooks from inside another call to these functions.


This is true for both emacs-25 and master.

Message #10 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Phillip Lord" <phillip.lord <at> russet.org.uk>
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Mon, 05 Dec 2016 17:33:25 +0200

> Date: Sun, 4 Dec 2016 20:53:24 -0000
> From: "Phillip Lord" <phillip.lord <at> russet.org.uk>
> 
> The documentation for "modification-hooks" on overlays says:
> 
>      If these functions modify the buffer, they should bind
>      ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
>      confusing the internal mechanism that calls these hooks.
> 
> But as far as I can see, the only place these gets called
> "signal_after_change"
> and "signal_before_change", inhibit-modification-hooks is already specbound
> to t, so this advice is unnecessary.
> 
> Also, the documentation for inhibit-modification-hooks says:
> 
>      If you do want modification hooks to be run in a particular
>      piece of code that is itself run from a modification hook, then
>      rebind locally ‘inhibit-modification-hooks’ to ‘nil’.
> 
> which suggests that, in fact, it is possible to call the modification
> hooks from inside another call to these functions.

Given these two excerpts, it seems to me that there's no inaccuracies
in the manual, perhaps we just need to tell both stories in the same
place or something?  Or do you still think there's something incorrect
in these two fragments?

Thanks.

Message #13 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25111 <at> debbugs.gnu.org, Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: How modification-hooks let-bind
 inhibit-modification-hooks?
Date: Mon, 5 Dec 2016 12:39:29 -0500

On Mon, Dec 5, 2016 at 10:33 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Sun, 4 Dec 2016 20:53:24 -0000
>> From: "Phillip Lord" <phillip.lord <at> russet.org.uk>
>>
>> The documentation for "modification-hooks" on overlays says:
>>
>>      If these functions modify the buffer, they should bind
>>      ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
>>      confusing the internal mechanism that calls these hooks.
>>
>> But as far as I can see, the only place these gets called
>> "signal_after_change"
>> and "signal_before_change", inhibit-modification-hooks is already specbound
>> to t, so this advice is unnecessary.
>>
>> Also, the documentation for inhibit-modification-hooks says:
>>
>>      If you do want modification hooks to be run in a particular
>>      piece of code that is itself run from a modification hook, then
>>      rebind locally ‘inhibit-modification-hooks’ to ‘nil’.
>>
>> which suggests that, in fact, it is possible to call the modification
>> hooks from inside another call to these functions.
>
> Given these two excerpts, it seems to me that there's no inaccuracies
> in the manual, perhaps we just need to tell both stories in the same
> place or something?  Or do you still think there's something incorrect
> in these two fragments?

Would following the advice in the second fragment confuse the
"internal mechanism" (as suggested in the first fragment) or not?

Message #16 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: 25111 <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#25111: How modification-hooks let-bind
 inhibit-modification-hooks?
Date: Mon, 05 Dec 2016 20:37:09 +0200

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Mon, 5 Dec 2016 12:39:29 -0500
> Cc: Phillip Lord <phillip.lord <at> russet.org.uk>, 25111 <at> debbugs.gnu.org
> 
> >> The documentation for "modification-hooks" on overlays says:
> >>
> >>      If these functions modify the buffer, they should bind
> >>      ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
> >>      confusing the internal mechanism that calls these hooks.
> >>
> >> But as far as I can see, the only place these gets called
> >> "signal_after_change"
> >> and "signal_before_change", inhibit-modification-hooks is already specbound
> >> to t, so this advice is unnecessary.
> >>
> >> Also, the documentation for inhibit-modification-hooks says:
> >>
> >>      If you do want modification hooks to be run in a particular
> >>      piece of code that is itself run from a modification hook, then
> >>      rebind locally ‘inhibit-modification-hooks’ to ‘nil’.
> >>
> >> which suggests that, in fact, it is possible to call the modification
> >> hooks from inside another call to these functions.
> >
> > Given these two excerpts, it seems to me that there's no inaccuracies
> > in the manual, perhaps we just need to tell both stories in the same
> > place or something?  Or do you still think there's something incorrect
> > in these two fragments?
> 
> Would following the advice in the second fragment confuse the
> "internal mechanism" (as suggested in the first fragment) or not?

Only if the other hooks that modify buffer, and do NOT want hooks to
be run, don't bind inhibit-modification-hooks to t.  AFAIU, at least.

Message #19 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Wed, 07 Dec 2016 16:40:02 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sun, 4 Dec 2016 20:53:24 -0000
>> From: "Phillip Lord" <phillip.lord <at> russet.org.uk>
>> 
>> The documentation for "modification-hooks" on overlays says:
>> 
>>      If these functions modify the buffer, they should bind
>>      ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
>>      confusing the internal mechanism that calls these hooks.
>> 
>> But as far as I can see, the only place these gets called
>> "signal_after_change"
>> and "signal_before_change", inhibit-modification-hooks is already specbound
>> to t, so this advice is unnecessary.
>> 
>> Also, the documentation for inhibit-modification-hooks says:
>> 
>>      If you do want modification hooks to be run in a particular
>>      piece of code that is itself run from a modification hook, then
>>      rebind locally ‘inhibit-modification-hooks’ to ‘nil’.
>> 
>> which suggests that, in fact, it is possible to call the modification
>> hooks from inside another call to these functions.
>
> Given these two excerpts, it seems to me that there's no inaccuracies
> in the manual, perhaps we just need to tell both stories in the same
> place or something?  Or do you still think there's something incorrect
> in these two fragments?

I think that the first of these is incorrect. There is no need to bind
`inhibit-modification-hooks' to `t'. More over, there may be reasons by
bind `inhibit-modification-hooks' to `nil' (i.e. "If you do want
modification hooks to be run..."). I am unclear whether this will
"confuse the internal mechanism", since I don't know exactly what this
means.

It possible that the documentation should say "Mostly, you should avoid
modifying the buffer on these hooks, any other functionality using these
modification-hooks will not be called."

The reason I ask all of this as a result of a concrete use
case. yasnippet modifies the buffer in these hooks, in turn breaks my
own package, lentic, which uses these hooks to respond to changes.


https://github.com/joaotavora/yasnippet/issues/756
https://github.com/phillord/lentic/issues/51

Phil

Message #22 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: phillip.lord <at> russet.org.uk (Phillip Lord)
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Thu, 08 Dec 2016 17:55:09 +0200

> From: phillip.lord <at> russet.org.uk (Phillip Lord)
> Cc: 25111 <at> debbugs.gnu.org
> Date: Wed, 07 Dec 2016 16:40:02 +0000
> 
> I think that the first of these is incorrect. There is no need to bind
> `inhibit-modification-hooks' to `t'. More over, there may be reasons by
> bind `inhibit-modification-hooks' to `nil' (i.e. "If you do want
> modification hooks to be run...").

So if we envision that some hook will bind inhibit-modification-hooks
to nil, then that is the reason to bind it to t in a hokk which
doesn't want such hooks to be run.

> It possible that the documentation should say "Mostly, you should avoid
> modifying the buffer on these hooks, any other functionality using these
> modification-hooks will not be called."

You mean, not mention the variable at all?  That'd be loss of useful
information, I think.

> The reason I ask all of this as a result of a concrete use
> case. yasnippet modifies the buffer in these hooks, in turn breaks my
> own package, lentic, which uses these hooks to respond to changes.

So how would you want the manual to help avert such calamities?

Message #25 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Fri, 09 Dec 2016 17:17:51 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: phillip.lord <at> russet.org.uk (Phillip Lord)
>> Cc: 25111 <at> debbugs.gnu.org
>> Date: Wed, 07 Dec 2016 16:40:02 +0000
>> 
>> I think that the first of these is incorrect. There is no need to bind
>> `inhibit-modification-hooks' to `t'. More over, there may be reasons by
>> bind `inhibit-modification-hooks' to `nil' (i.e. "If you do want
>> modification hooks to be run...").
>
> So if we envision that some hook will bind inhibit-modification-hooks
> to nil, then that is the reason to bind it to t in a hokk which
> doesn't want such hooks to be run.

Indeed, this is true, and it's a difficulty with the hook. I mean, it is
*meant* to run when the buffer is modified and yet here is an example of
where we cannot be sure that it will.


>> It possible that the documentation should say "Mostly, you should avoid
>> modifying the buffer on these hooks, any other functionality using these
>> modification-hooks will not be called."
>
> You mean, not mention the variable at all?  That'd be loss of useful
> information, I think.
>
>> The reason I ask all of this as a result of a concrete use
>> case. yasnippet modifies the buffer in these hooks, in turn breaks my
>> own package, lentic, which uses these hooks to respond to changes.
>
> So how would you want the manual to help avert such calamities?


My own feeling is that "inhibit-modification-hooks" should *only* be for
modifications that really should not be detected by anything else. I can
think of examples of this (I used to change the buffer to display a
completion string to the user for instance, although I now use an
"after-string" overlay property).

The simplest advice makes calls to the modification hooks consistent is
to say "You should not modify the buffer on these hooks". The potential
solution, for instance, for yasnippet is to record the changes on
after-change-function, and then change the buffer on
post-command-hook. I think this would work? Is this what the manual
should say?

Phil

Message #28 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: phillip.lord <at> russet.org.uk (Phillip Lord)
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Fri, 09 Dec 2016 19:26:46 +0200

> From: phillip.lord <at> russet.org.uk (Phillip Lord)
> Cc: 25111 <at> debbugs.gnu.org
> Date: Fri, 09 Dec 2016 17:17:51 +0000
> 
> > So how would you want the manual to help avert such calamities?
> 
> 
> My own feeling is that "inhibit-modification-hooks" should *only* be for
> modifications that really should not be detected by anything else. I can
> think of examples of this (I used to change the buffer to display a
> completion string to the user for instance, although I now use an
> "after-string" overlay property).
> 
> The simplest advice makes calls to the modification hooks consistent is
> to say "You should not modify the buffer on these hooks". The potential
> solution, for instance, for yasnippet is to record the changes on
> after-change-function, and then change the buffer on
> post-command-hook. I think this would work? Is this what the manual
> should say?

IMO, the manual should advise the safe practices, and then tell how to
behave if the code really needs to play it less safe.  The former
would be what you say above, I think.  But since we know there are
packages out there that don't choose the safe approach, we should
cover those as well.

Message #31 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Sun, 11 Dec 2016 22:11:14 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: phillip.lord <at> russet.org.uk (Phillip Lord)
>> Cc: 25111 <at> debbugs.gnu.org
>> Date: Fri, 09 Dec 2016 17:17:51 +0000
>>
>> My own feeling is that "inhibit-modification-hooks" should *only* be for
>> modifications that really should not be detected by anything else. I can
>> think of examples of this (I used to change the buffer to display a
>> completion string to the user for instance, although I now use an
>> "after-string" overlay property).
>> 
>> The simplest advice makes calls to the modification hooks consistent is
>> to say "You should not modify the buffer on these hooks". The potential
>> solution, for instance, for yasnippet is to record the changes on
>> after-change-function, and then change the buffer on
>> post-command-hook. I think this would work? Is this what the manual
>> should say?
>
> IMO, the manual should advise the safe practices, and then tell how to
> behave if the code really needs to play it less safe.  The former
> would be what you say above, I think.  But since we know there are
> packages out there that don't choose the safe approach, we should
> cover those as well.


So, instead of this:

     If these functions modify the buffer, they should bind
     ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
     confusing the internal mechanism that calls these hooks.


We could have:

    These functions should avoid unnecessarily modifying the buffer.
    Emacs binds 'inhibit-modification-hooks' to `t' during their
    evaluation, which means that any modifications will not be signalled
    to other hook functions listening for them.


Perhaps a better solution would be:


    These functions should avoid unnecessarily modifying the buffer; see
    Change Hooks for further details.


Then a new paragraph can be added to the Change Hooks section talking
about the complexity of modifying buffers on these hooks, with
alternatives.

I am happy to draft something if you wish.

Phil

Message #34 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: phillip.lord <at> russet.org.uk (Phillip Lord)
Cc: 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111:
Date: Mon, 12 Dec 2016 18:06:34 +0200

> From: phillip.lord <at> russet.org.uk (Phillip Lord)
> Cc: 25111 <at> debbugs.gnu.org
> Date: Sun, 11 Dec 2016 22:11:14 +0000
> 
> So, instead of this:
> 
>      If these functions modify the buffer, they should bind
>      ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
>      confusing the internal mechanism that calls these hooks.
> 
> 
> We could have:
> 
>     These functions should avoid unnecessarily modifying the buffer.
>     Emacs binds 'inhibit-modification-hooks' to `t' during their
>     evaluation, which means that any modifications will not be signalled
>     to other hook functions listening for them.
> 
> 
> Perhaps a better solution would be:
> 
> 
>     These functions should avoid unnecessarily modifying the buffer; see
>     Change Hooks for further details.
> 
> 
> Then a new paragraph can be added to the Change Hooks section talking
> about the complexity of modifying buffers on these hooks, with
> alternatives.
> 
> I am happy to draft something if you wish.

Sure, please do.

Thanks.

Message #37 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Phillip Lord <phillip.lord <at> russet.org.uk>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111: How modification-hooks let-bind
 inhibit-modification-hooks?
Date: Thu, 9 Mar 2017 14:34:06 -0500

On Wed, Dec 7, 2016 at 11:40 AM, Phillip Lord
<phillip.lord <at> russet.org.uk> wrote:
>
> The reason I ask all of this as a result of a concrete use
> case. yasnippet modifies the buffer in these hooks, in turn breaks my
> own package, lentic, which uses these hooks to respond to changes.
>
>
> https://github.com/joaotavora/yasnippet/issues/756
> https://github.com/phillord/lentic/issues/51

By the way, text-clone--maintain in subr.el seems to have the same problem.

Message #40 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>, Noam Postavsky <npostavs <at> gmail.com>
Cc: 25111 <at> debbugs.gnu.org, Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Sun, 19 May 2019 20:31:19 +0000

Hello, Eli and Noam.

On Mon, Dec 12, 2016 at 18:06:34 +0200, Eli Zaretskii wrote:
> > From: phillip.lord <at> russet.org.uk (Phillip Lord)
> > Cc: 25111 <at> debbugs.gnu.org
> > Date: Sun, 11 Dec 2016 22:11:14 +0000

> > So, instead of this:

> >      If these functions modify the buffer, they should bind
> >      ‘inhibit-modification-hooks’ to ‘t’ around doing so, to avoid
> >      confusing the internal mechanism that calls these hooks.


> > We could have:

> >     These functions should avoid unnecessarily modifying the buffer.
> >     Emacs binds 'inhibit-modification-hooks' to `t' during their
> >     evaluation, which means that any modifications will not be signalled
> >     to other hook functions listening for them.


> > Perhaps a better solution would be:


> >     These functions should avoid unnecessarily modifying the buffer; see
> >     Change Hooks for further details.


> > Then a new paragraph can be added to the Change Hooks section talking
> > about the complexity of modifying buffers on these hooks, with
> > alternatives.

> > I am happy to draft something if you wish.

> Sure, please do.

> Thanks.

OK, it was a while ago, and I'm not Phillip, but I recently got caught
out with the inaccurate documentation of inhibit-modification-hooks, so I
suggest the following, to get the discussion rolling again:



diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index a2ed4b3891..b88702f2a2 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -1743,9 +1743,17 @@ Overlay Properties
 length is the number of characters deleted, and the post-change
 beginning and end are equal.)
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+@c If these functions modify the buffer, they should bind
+@c @code{inhibit-modification-hooks} to @code{t} around doing so, to
+@c avoid confusing the internal mechanism that calls these hooks.
+
+When these functions are called, @code{inhibit-modification-hooks} is
+bound to non-@code{nil}.  If the functions modify the buffer, you
+might want to bind @code{inhibit-modification-hooks} to nil, so as to
+cause the change hooks to run for these modifications.  @xref{Change
+Hooks}.  However, doing this can sometimes confuse the internal
+mechanism that calls all these hooks, leading, for example, to calling
+them recursively, which is usually unwanted.
 
 Text properties also support the @code{modification-hooks} property,
 but the details are somewhat different (@pxref{Special Properties}).
diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 278bc3c268..fd4338ecdb 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -3621,9 +3621,14 @@ Special Properties
 hook will only be run when removing some characters, replacing them
 with others, or changing their text-properties.
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+@c If these functions modify the buffer, they should bind
+@c @code{inhibit-modification-hooks} to @code{t} around doing so, to
+@c avoid confusing the internal mechanism that calls these hooks.
+
+When Emacs calls these functions, @code{inhibit-modification-hooks} is
+set to @code{nil}.  If the functions modify the buffer, you should
+consider binding this variable to non-@code{nil}, to avoid confusing
+the internal mechanism that calls these hooks.  @xref{Change Hooks}.
 
 Overlays also support the @code{modification-hooks} property, but the
 details are somewhat different (@pxref{Overlay Properties}).


-- 
Alan Mackenzie (Nuremberg, Germany).

Message #43 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Sat, 25 May 2019 08:39:39 -0400

[Message part 1 (text/plain, inline)]

Alan Mackenzie <acm <at> muc.de> writes:

> @@ -1743,9 +1743,17 @@ Overlay Properties
>  However, doing this can sometimes confuse the internal
> +mechanism that calls all these hooks, leading, for example, to calling
> +them recursively, which is usually unwanted.

> @@ -3621,9 +3621,14 @@ Special Properties

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

As Phillip mentioned in the OP, Emacs in fact binds it to t.

> If the functions modify the buffer, you should
> +consider binding this variable to non-@code{nil}, to avoid confusing
> +the internal mechanism that calls these hooks.  @xref{Change Hooks}.

I find this "confusing the internal mechanism" thing unhelpful, how
about this instead:

[0001-Clarify-elisp-ref-for-inhibit-modification-hooks-Bug.patch (text/x-diff, inline)]

From 568e640f962cd9a59d695351ded11c4a8e781f06 Mon Sep 17 00:00:00 2001
From: Alan Mackenzie <acm <at> muc.de>
Date: Sun, 19 May 2019 20:31:19 +0000
Subject: [PATCH] Clarify elisp ref for inhibit-modification-hooks (Bug#25111)

* doc/lispref/display.texi (Overlay Properties):
* doc/lispref/text.texi (Special Properties)
(Change Hooks): Explain that inhibit-modification-hooks is bound to t
while executing change hooks, and suggest binding to nil with suitable
precautions when modifying buffer from a change hook.

Co-authored-by: Noam Postavsky <npostavs <at> gmail.com>
---
 doc/lispref/display.texi |  9 ++++++---
 doc/lispref/text.texi    | 12 ++++++++----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index b07999432c..f7140f444e 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -1708,9 +1708,12 @@ Overlay Properties
 length is the number of characters deleted, and the post-change
 beginning and end are equal.)
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+When these functions are called, @code{inhibit-modification-hooks} is
+bound to non-@code{nil}.  If the functions modify the buffer, you
+might want to bind @code{inhibit-modification-hooks} to nil, so as to
+cause the change hooks to run for these modifications.  However, doing
+this may call your own change hook recursively, so be sure to prepare
+for that.  @xref{Change Hooks}.
 
 Text properties also support the @code{modification-hooks} property,
 but the details are somewhat different (@pxref{Special Properties}).
diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index f3d222b708..5954161fcf 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -3514,9 +3514,10 @@ Special Properties
 hook will only be run when removing some characters, replacing them
 with others, or changing their text-properties.
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+When Emacs calls these functions, @code{inhibit-modification-hooks} is
+set to non-@code{nil}.  If the functions modify the buffer, you should
+consider binding this variable to @code{nil}, but in that case you
+must be prepared for recursive calls.  @xref{Change Hooks}.
 
 Overlays also support the @code{modification-hooks} property, but the
 details are somewhat different (@pxref{Overlay Properties}).
@@ -5093,5 +5094,8 @@ Change Hooks
 a modification hook does not cause other modification hooks to be run.
 If you do want modification hooks to be run in a particular piece of
 code that is itself run from a modification hook, then rebind locally
-@code{inhibit-modification-hooks} to @code{nil}.
+@code{inhibit-modification-hooks} to @code{nil}.  However, doing this
+may cause recursive calls to the modification hooks, so be sure to
+prepare for that (for example, by binding some variable which tells
+your hook to do nothing).
 @end defvar
-- 
2.11.0

Message #46 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Sat, 25 May 2019 13:44:07 +0000

Hello, Noam.

On Sat, May 25, 2019 at 08:39:39 -0400, Noam Postavsky wrote:
> Alan Mackenzie <acm <at> muc.de> writes:

> > @@ -1743,9 +1743,17 @@ Overlay Properties
> >  However, doing this can sometimes confuse the internal
> > +mechanism that calls all these hooks, leading, for example, to calling
> > +them recursively, which is usually unwanted.

> > @@ -3621,9 +3621,14 @@ Special Properties

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

> As Phillip mentioned in the OP, Emacs in fact binds it to t.

Are you sure?  We're talking here about the text property (in which I
think inhibit-modification-hooks IS at nil) as opposed to the overlay
property (where inhibit-modification-hooks is bound to t).

I admit just to having examined the source code rather than actually
trying it out.  But in verify_interval_modification in textprop.c, near
the end, we have:

      if (!inhibit_modification_hooks)
        {
          hooks = Fnreverse (hooks);
          while (! NILP (hooks))
            {
              call_mod_hooks (Fcar (hooks), make_fixnum (start),
                              make_fixnum (end));
              hooks = Fcdr (hooks);
            }
        }

.  Here, mod_hooks is a list of modification-hooks combined with
positions.  call_mod_hooks only gets called when
inhibit-modification-hooks is nil.  call_mod_hooks just calls the hooks,
without binding i-m-h.

Are you really sure?

I'll answer the rest of your post later, I've got a lot on in Real Life
at the moment.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #49 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: acm <at> muc.de, 25111 <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Sat, 25 May 2019 16:49:59 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  25111 <at> debbugs.gnu.org,  Phillip Lord <phillip.lord <at> russet.org.uk>
> Date: Sat, 25 May 2019 08:39:39 -0400
> 
> I find this "confusing the internal mechanism" thing unhelpful, how
> about this instead:

Fine with me (although I'm not Alan), but please avoid saying the same
thing more than once.  It is better to describe the issue in one
place, and then have the N-1 other places have a cross-reference to
that one place.

Thanks.

Message #52 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Sat, 25 May 2019 10:36:55 -0400

[Message part 1 (text/plain, inline)]

Alan Mackenzie <acm <at> muc.de> writes:

>>> @@ -3621,9 +3621,14 @@ Special Properties
>
>>> +When Emacs calls these functions, @code{inhibit-modification-hooks} is
>>> +set to @code{nil}.
>
>> As Phillip mentioned in the OP, Emacs in fact binds it to t.
>
> Are you sure?  We're talking here about the text property (in which I
> think inhibit-modification-hooks IS at nil) as opposed to the overlay
> property (where inhibit-modification-hooks is bound to t).

Oh, you're quite right.  Here's some test code:

[bug-25111-binding-of-inhibit-mod-hooks.el (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

Which produces this:

mod-hook-text-prop (1 4), inhibit? nil
mod-hook-change-fun (1 4), inhibit? t
mod-hook-ov-prop (#<overlay from 1 to 5 in *test*> nil 1 4), inhibit? t
mod-hook-change-fun (1 1 3), inhibit? t
mod-hook-ov-prop (#<overlay from 1 to 2 in *test*> t 1 1 3), inhibit? t
mod-hook-change-fun (1 1), inhibit? t
mod-hook-change-fun (1 4 0), inhibit? t

I think we need to emphasize the difference in this case, it's rather
confusing.

> I'll answer the rest of your post later, I've got a lot on in Real Life
> at the moment.

No rush.  I've updated the patch based on your and Eli's feedback.

[0001-Clarify-elisp-ref-for-inhibit-modification-hooks-Bug.patch (text/x-diff, inline)]

From 7f6453596b7753af7704eaac7f27ebba8d03cfc4 Mon Sep 17 00:00:00 2001
From: Alan Mackenzie <acm <at> muc.de>
Date: Sun, 19 May 2019 20:31:19 +0000
Subject: [PATCH] Clarify elisp ref for inhibit-modification-hooks (Bug#25111)

* doc/lispref/display.texi (Overlay Properties):
* doc/lispref/text.texi (Change Hooks): Explain that
inhibit-modification-hooks is bound to t while executing change hooks,
and suggest binding to nil with suitable precautions when modifying
buffer from a change hook.
(Special Properties): Emphasize that inhibit-modification-hooks is
left set to nil when executing text property change hooks.

Co-authored-by: Noam Postavsky <npostavs <at> gmail.com>
---
 doc/lispref/display.texi |  6 +++---
 doc/lispref/text.texi    | 12 ++++++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index b07999432c..59d02d540a 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -1708,9 +1708,9 @@ Overlay Properties
 length is the number of characters deleted, and the post-change
 beginning and end are equal.)
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+Similar to change hooks, when these functions are called,
+@code{inhibit-modification-hooks} is bound to @code{t}.  @xref{Change
+Hooks}.
 
 Text properties also support the @code{modification-hooks} property,
 but the details are somewhat different (@pxref{Special Properties}).
diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index f3d222b708..c935cfe49b 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -3514,9 +3514,10 @@ Special Properties
 hook will only be run when removing some characters, replacing them
 with others, or changing their text-properties.
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+When Emacs calls these functions, @code{inhibit-modification-hooks} is
+set to @code{nil}, unlike for change hooks.  When writing a function
+which modifies the buffer, consider binding it @code{t}, to avoid
+recursive calls.  @xref{Change Hooks}.
 
 Overlays also support the @code{modification-hooks} property, but the
 details are somewhat different (@pxref{Overlay Properties}).
@@ -5093,5 +5094,8 @@ Change Hooks
 a modification hook does not cause other modification hooks to be run.
 If you do want modification hooks to be run in a particular piece of
 code that is itself run from a modification hook, then rebind locally
-@code{inhibit-modification-hooks} to @code{nil}.
+@code{inhibit-modification-hooks} to @code{nil}.  However, doing this
+may cause recursive calls to the modification hooks, so be sure to
+prepare for that (for example, by binding some variable which tells
+your hook to do nothing).
 @end defvar
-- 
2.11.0

Message #55 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Mon, 27 May 2019 14:31:09 +0000

Hello, Noam.

On Sat, May 25, 2019 at 10:36:55 -0400, Noam Postavsky wrote:
> Alan Mackenzie <acm <at> muc.de> writes:

> >>> @@ -3621,9 +3621,14 @@ Special Properties

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

> >> As Phillip mentioned in the OP, Emacs in fact binds it to t.

> > Are you sure?  We're talking here about the text property (in which I
> > think inhibit-modification-hooks IS at nil) as opposed to the overlay
> > property (where inhibit-modification-hooks is bound to t).

> Oh, you're quite right.  Here's some test code:

[ .... ]


> Which produces this:

> mod-hook-text-prop (1 4), inhibit? nil
> mod-hook-change-fun (1 4), inhibit? t
> mod-hook-ov-prop (#<overlay from 1 to 5 in *test*> nil 1 4), inhibit? t
> mod-hook-change-fun (1 1 3), inhibit? t
> mod-hook-ov-prop (#<overlay from 1 to 2 in *test*> t 1 1 3), inhibit? t
> mod-hook-change-fun (1 1), inhibit? t
> mod-hook-change-fun (1 4 0), inhibit? t

> I think we need to emphasize the difference in this case, it's rather
> confusing.

Alternatively, we could perhaps regard the first of these results (for
modification-hooks) as a bug in the code, which seems like it ought to be
binding inhibit-modification-hooks to non-nil like the others.  Maybe we
should amend the code, even though this would be a jarring
incompatibility with previous Emacs versions.  Eli?

[ .... ]

> I've updated the patch based on your and Eli's feedback.

Yes, I agree that "confusing the internal mechanism" is unhelpful here.
Thanks for getting rid of it.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #58 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Phillip Lord <phillip.lord <at> russet.org.uk>,
 Noam Postavsky <npostavs <at> gmail.com>, 25111 <at> debbugs.gnu.org
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Mon, 3 Jun 2019 19:15:49 +0000

Hello, Eli.

To recap, the problem we were talking about was the modification-hooks
overlay property, whose value is a function which gets called before and
after modification of the text under an overlay.

When such a function gets called, inhibit-modification-hooks is left at
nil.  When the other four similar overlay/text-property "change
functions" get called, inhibit-modification-hooks gets bound to t.

This is difficult to document coherently.  My proposal of last week was
to fix the code, also to bin inhibit-modification-hooks to t for the
modification-hooks overlay property, even though this would be an
incompatibility in Lisp.

Ping?  ----------------------------->-----------------------------------
                                                                       |
On Mon, May 27, 2019 at 14:31:09 +0000, Alan Mackenzie wrote:          |
> Hello, Noam.                                                         |
                                                                       |
> On Sat, May 25, 2019 at 10:36:55 -0400, Noam Postavsky wrote:        |
> > Alan Mackenzie <acm <at> muc.de> writes:                                |
                                                                       |
> > >>> @@ -3621,9 +3621,14 @@ Special Properties                      |
                                                                       |
> > >>> +When Emacs calls these functions, @code{inhibit-modification-hooks} is
> > >>> +set to @code{nil}.                                            |
                                                                       v
> > >> As Phillip mentioned in the OP, Emacs in fact binds it to t.    |
                                                                       |
> > > Are you sure?  We're talking here about the text property (in which I
> > > think inhibit-modification-hooks IS at nil) as opposed to the overlay
> > > property (where inhibit-modification-hooks is bound to t).       |
                                                                       |
> > Oh, you're quite right.  Here's some test code:                    |
                                                                       |
> [ .... ]                                                             |
                                                                       |
                                                                       |
> > Which produces this:                                               |
                                                                       |
> > mod-hook-text-prop (1 4), inhibit? nil                             |
> > mod-hook-change-fun (1 4), inhibit? t                              |
> > mod-hook-ov-prop (#<overlay from 1 to 5 in *test*> nil 1 4), inhibit? t
> > mod-hook-change-fun (1 1 3), inhibit? t                            |
> > mod-hook-ov-prop (#<overlay from 1 to 2 in *test*> t 1 1 3), inhibit? t
> > mod-hook-change-fun (1 1), inhibit? t                              |
> > mod-hook-change-fun (1 4 0), inhibit? t                            |
                                                                       |
> > I think we need to emphasize the difference in this case, it's rather
> > confusing.                                                         |
                                                                       V
> Alternatively, we could perhaps regard the first of these results (for
> modification-hooks) as a bug in the code, which seems like it ought to be
> binding inhibit-modification-hooks to non-nil like the others.  Maybe we
> should amend the code, even though this would be a jarring
> incompatibility with previous Emacs versions.  Eli?

> [ .... ]

> > I've updated the patch based on your and Eli's feedback.

> Yes, I agree that "confusing the internal mechanism" is unhelpful here.
> Thanks for getting rid of it.

> [ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #61 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> gmail.com
To: Alan Mackenzie <acm <at> muc.de>
Cc: 25111 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Noam Postavsky <npostavs <at> gmail.com>, Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Mon, 03 Jun 2019 15:26:38 -0400

Alan Mackenzie <acm <at> muc.de> writes:

> Hello, Eli.
>
> To recap, the problem we were talking about was the modification-hooks
> overlay property, whose value is a function which gets called before and
> after modification of the text under an overlay.
>
> When such a function gets called, inhibit-modification-hooks is left at
> nil.  When the other four similar overlay/text-property "change
> functions" get called, inhibit-modification-hooks gets bound to t.

Minor correction: it's the modification-hooks text property which have
inhibit-modification-hooks left at nil, when the overlay property
modification-hooks get called inhibit-modification-hooks is bound to t,
just like in the after/before-change-functions case.

> 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 bin
> inhibit-modification-hooks to t for the modification-hooks overlay
> property, even though this would be an incompatibility in Lisp.

Message #64 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: npostavs <at> gmail.com
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Tue, 4 Jun 2019 09:32:41 +0000

Hello, Noam.

On Mon, Jun 03, 2019 at 15:26:38 -0400, npostavs <at> gmail.com wrote:
> Alan Mackenzie <acm <at> muc.de> writes:

> > Hello, Eli.

> > To recap, the problem we were talking about was the modification-hooks
> > overlay property, whose value is a function which gets called before and
> > after modification of the text under an overlay.

> > When such a function gets called, inhibit-modification-hooks is left at
> > nil.  When the other four similar overlay/text-property "change
> > functions" get called, inhibit-modification-hooks gets bound to t.

> Minor correction: it's the modification-hooks text property which have
> inhibit-modification-hooks left at nil, when the overlay property
> modification-hooks get called inhibit-modification-hooks is bound to t,
> just like in the after/before-change-functions case.

Oh, bother.  ;-)

> > 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?



diff --git a/src/textprop.c b/src/textprop.c
index ae42c44185..607bd40676 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -2247,6 +2247,8 @@ verify_interval_modification (struct buffer *buf,
 
       if (!inhibit_modification_hooks)
 	{
+          int count = SPECPDL_INDEX ();
+          specbind (Qinhibit_modification_hooks, Qt);
 	  hooks = Fnreverse (hooks);
 	  while (! NILP (hooks))
 	    {
@@ -2254,6 +2256,7 @@ verify_interval_modification (struct buffer *buf,
 			      make_fixnum (end));
 	      hooks = Fcdr (hooks);
 	    }
+          unbind_to (count, Qnil);
 	}
     }
 }



-- 
Alan Mackenzie (Nuremberg, Germany).

Message #67 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 25111 <at> debbugs.gnu.org, npostavs <at> gmail.com, phillip.lord <at> russet.org.uk
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Tue, 04 Jun 2019 17:36:12 +0300

> Date: Tue, 4 Jun 2019 09:32:41 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Phillip Lord <phillip.lord <at> russet.org.uk>,
>   25111 <at> debbugs.gnu.org
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > > 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.  I must refresh my memory about
this old issue, and re-read the code, and I'm currently busy with more
urgent issues, like the harfbuzz branch and a couple of display bugs.

TIA, and sorry for being unable to respond quickly.

Message #70 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: acm <at> muc.de, phillip.lord <at> russet.org.uk
Cc: 25111 <at> debbugs.gnu.org, npostavs <at> gmail.com
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Sun, 09 Jun 2019 15:00:16 +0300

> Date: Tue, 04 Jun 2019 17:36:12 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: phillip.lord <at> russet.org.uk, 25111 <at> debbugs.gnu.org, npostavs <at> gmail.com
> 
> > Date: Tue, 4 Jun 2019 09:32:41 +0000
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, Phillip Lord <phillip.lord <at> russet.org.uk>,
> >   25111 <at> debbugs.gnu.org
> > From: Alan Mackenzie <acm <at> muc.de>
> > 
> > > > 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.

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.

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.

Message #73 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: npostavs <at> gmail.com, 25111 <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
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 <eliz <at> gnu.org>
> > Cc: phillip.lord <at> russet.org.uk, 25111 <at> debbugs.gnu.org, npostavs <at> gmail.com

> > > Date: Tue, 4 Jun 2019 09:32:41 +0000
> > > Cc: Eli Zaretskii <eliz <at> gnu.org>, Phillip Lord <phillip.lord <at> russet.org.uk>,
> > >   25111 <at> debbugs.gnu.org
> > > From: Alan Mackenzie <acm <at> muc.de>

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

Message #76 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>, npostavs <at> gmail.com
Cc: 25111 <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Mon, 24 Jun 2019 12:52:49 +0000

Hello, Eli and Noam (but mainly Noam).

It's about time we finally got this matter tidied up, so...

On Sun, Jun 09, 2019 at 15:00:16 +0300, Eli Zaretskii wrote:

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

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

I've corrected this bit by saying that "Unlike with other similar hooks,
when Emacs calls these functions, `inhibit-modification-hooks' does _not_
get bound to non-`nil'".

I've also added bits to the descriptions of
insert-{in-front,behind}-hooks, the text property version of them,
documenting that inhibit-modification-hooks gets bound to non-nil.

[ .... ]

I think the changes as now formulated are right.  Perhaps one or both of
you might like to give the following patch a quick review.  Thanks!



diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 7e8abb0440..68f40b55d8 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -1752,9 +1752,12 @@ Overlay Properties
 length is the number of characters deleted, and the post-change
 beginning and end are equal.)
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+When these functions are called, @code{inhibit-modification-hooks} is
+bound to non-@code{nil}.  If the functions modify the buffer, you
+might want to bind @code{inhibit-modification-hooks} to nil, so as to
+cause the change hooks to run for these modifications.  However, doing
+this may call your own change hook recursively, so be sure to prepare
+for that.  @xref{Change Hooks}.
 
 Text properties also support the @code{modification-hooks} property,
 but the details are somewhat different (@pxref{Special Properties}).
diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 2e7c497f57..95ed758914 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -3621,9 +3621,12 @@ Special Properties
 hook will only be run when removing some characters, replacing them
 with others, or changing their text-properties.
 
-If these functions modify the buffer, they should bind
-@code{inhibit-modification-hooks} to @code{t} around doing so, to
-avoid confusing the internal mechanism that calls these hooks.
+Unlike with other similar hooks, when Emacs calls these functions,
+@code{inhibit-modification-hooks} does @emph{not} get bound to
+non-@code{nil}.  If the functions modify the buffer, you should
+consider binding this variable to non-@code{nil} to prevent any buffer
+changes running the change hooks.  Otherwise, you must be prepared for
+recursive calls.  @xref{Change Hooks}.
 
 Overlays also support the @code{modification-hooks} property, but the
 details are somewhat different (@pxref{Overlay Properties}).
@@ -3639,6 +3642,13 @@ Special Properties
 beginning and end of the inserted text.  The functions are called
 @emph{after} the actual insertion takes place.
 
+When these functions are called, @code{inhibit-modification-hooks} is
+bound to non-@code{nil}.  If the functions modify the buffer, you
+might want to bind @code{inhibit-modification-hooks} to nil, so as to
+cause the change hooks to run for these modifications.  However, doing
+this may call your own change hook recursively, so be sure to prepare
+for that.
+
 See also @ref{Change Hooks}, for other hooks that are called
 when you change text in a buffer.
 
@@ -5650,5 +5660,8 @@ Change Hooks
 a modification hook does not cause other modification hooks to be run.
 If you do want modification hooks to be run in a particular piece of
 code that is itself run from a modification hook, then rebind locally
-@code{inhibit-modification-hooks} to @code{nil}.
+@code{inhibit-modification-hooks} to @code{nil}.  However, doing this
+may cause recursive calls to the modification hooks, so be sure to
+prepare for that (for example, by binding some variable which tells
+your hook to do nothing).
 @end defvar


-- 
Alan Mackenzie (Nuremberg, Germany).

Message #79 received at 25111 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 25111 <at> debbugs.gnu.org,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Mon, 24 Jun 2019 18:48:14 -0400

Alan Mackenzie <acm <at> muc.de> writes:

> Hello, Eli and Noam (but mainly Noam).
>
> It's about time we finally got this matter tidied up, so...

Yeah, sorry I dropped the ball on this.

> I think the changes as now formulated are right.  Perhaps one or both of
> you might like to give the following patch a quick review.  Thanks!

Minor formatting nitpick:

> +++ b/doc/lispref/display.texi
> @@ -1752,9 +1752,12 @@ Overlay Properties

> +When these functions are called, @code{inhibit-modification-hooks} is
> +bound to non-@code{nil}.  If the functions modify the buffer, you
> +might want to bind @code{inhibit-modification-hooks} to nil, so as to
                                                           ^^^
> +cause the change hooks to run for these modifications.  However, doing
> +this may call your own change hook recursively, so be sure to prepare
> +for that.  @xref{Change Hooks}.

> +++ b/doc/lispref/text.texi

> @@ -3639,6 +3642,13 @@ Special Properties
>  beginning and end of the inserted text.  The functions are called
>  @emph{after} the actual insertion takes place.
>  
> +When these functions are called, @code{inhibit-modification-hooks} is
> +bound to non-@code{nil}.  If the functions modify the buffer, you
> +might want to bind @code{inhibit-modification-hooks} to nil, so as to
                                                           ^^^

@code{nil} for both of these, right?  Otherwise looks good to me.

Message #84 received at 25111-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, phillip.lord <at> russet.org.uk,
 25111-done <at> debbugs.gnu.org
Subject: Re: bug#25111: (Inaccurate documentation of
 inhibit-modification-hooks)
Date: Tue, 25 Jun 2019 09:17:09 +0000

Hello, Noam.

On Mon, Jun 24, 2019 at 18:48:14 -0400, Noam Postavsky wrote:
> Alan Mackenzie <acm <at> muc.de> writes:

[ .... ]

> > I think the changes as now formulated are right.  Perhaps one or
> > both of you might like to give the following patch a quick review.
> > Thanks!

> Minor formatting nitpick:

> > +++ b/doc/lispref/display.texi
> > @@ -1752,9 +1752,12 @@ Overlay Properties

> > +When these functions are called, @code{inhibit-modification-hooks} is
> > +bound to non-@code{nil}.  If the functions modify the buffer, you
> > +might want to bind @code{inhibit-modification-hooks} to nil, so as to
>                                                            ^^^
> > +cause the change hooks to run for these modifications.  However, doing
> > +this may call your own change hook recursively, so be sure to prepare
> > +for that.  @xref{Change Hooks}.

> > +++ b/doc/lispref/text.texi

> > @@ -3639,6 +3642,13 @@ Special Properties
> >  beginning and end of the inserted text.  The functions are called
> >  @emph{after} the actual insertion takes place.

> > +When these functions are called, @code{inhibit-modification-hooks} is
> > +bound to non-@code{nil}.  If the functions modify the buffer, you
> > +might want to bind @code{inhibit-modification-hooks} to nil, so as to
>                                                            ^^^

> @code{nil} for both of these, right?  Otherwise looks good to me.

Whoops!  Thanks for spotting these.

I've fixed them and committed the changes.  I'm closing the bug with
this post.

-- 
Alan Mackenzie (Nuremberg, Germany).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.