(reset_var_on_error): New fun.

(signal_before_change, signal_after_change):
Use it to reset (after|before)-change-functions to nil in case of error.
Bind inhibit-modification-hooks to t.
Don't bind (after|before)-change-functions to nil while they run.

4 files changed, 58 insertions, 91 deletions

diff --git a/etc/NEWS b/etc/NEWS
index c652703bcf1..32b555f14c9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -79,6 +79,11 @@ in to make it use the scrollbars from the system theme.
 
 * Lisp Changes in Emacs 22.2.
 
++++
+** inhibit-modification-hooks is bound to t while running modification hooks.
+As a happy consequence, after-change-functions and before-change-functions
+are not bound to nil any more while running an (after|before)-change-function.
+
 ** New function `window-full-width-p' returns t if a window is as wide
 as its frame.
 
diff --git a/lispref/text.texi b/lispref/text.texi
index 4448e1cc402..bad68b7f8ae 100644
--- a/lispref/text.texi
+++ b/lispref/text.texi
@@ -4364,35 +4364,6 @@ because it may lead to inefficient behavior for some change hook
 functions.
 @end defmac
 
-The two variables above are temporarily bound to @code{nil} during the
-time that any of these functions is running.  This means that if one of
-these functions changes the buffer, that change won't run these
-functions.  If you do want a hook function to make changes that run
-these functions, make it bind these variables back to their usual
-values.
-
-One inconvenient result of this protective feature is that you cannot
-have a function in @code{after-change-functions} or
-@code{before-change-functions} which changes the value of that variable.
-But that's not a real limitation.  If you want those functions to change
-the list of functions to run, simply add one fixed function to the hook,
-and code that function to look in another variable for other functions
-to call.  Here is an example:
-
-@example
-(setq my-own-after-change-functions nil)
-(defun indirect-after-change-function (beg end len)
-  (let ((list my-own-after-change-functions))
-    (while list
-      (funcall (car list) beg end len)
-      (setq list (cdr list)))))
-
-@group
-(add-hooks 'after-change-functions
-           'indirect-after-change-function)
-@end group
-@end example
-
 @defvar first-change-hook
 This variable is a normal hook that is run whenever a buffer is changed
 that was previously in the unmodified state.
@@ -4404,6 +4375,13 @@ disabled; none of them run.  This affects all the hook variables
 described above in this section, as well as the hooks attached to
 certain special text properties (@pxref{Special Properties}) and overlay
 properties (@pxref{Overlay Properties}).
+
+Also, this variable is bound to non-@code{nil} while running those
+same hook variables, so that by default modifying the buffer from
+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}.
 @end defvar
 
 @ignore
diff --git a/src/ChangeLog b/src/ChangeLog
index add0663a648..bf015d6959f 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2007-08-21  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+        * insdel.c (reset_var_on_error): New fun.
+        (signal_before_change, signal_after_change):
+        Use it to reset (after|before)-change-functions to nil in case of error.
+        Bind inhibit-modification-hooks to t.
+        Don't bind (after|before)-change-functions to nil while they run.
+
 2007-08-19  Andreas Schwab  <schwab@suse.de>
 
         * alloc.c (pure): Round PURESIZE up.
diff --git a/src/insdel.c b/src/insdel.c
index 14e7478e69a..cd8e2738f9a 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -2137,6 +2137,21 @@ prepare_to_modify_buffer (start, end, preserve_ptr)
 #define FETCH_END                               \
   (! NILP (end_marker) ? Fmarker_position (end_marker) : end)
 
+/* Set a variable to nil if an error occurred.
+   Don't change the variable if there was no error.
+   VAL is a cons-cell (VARIABLE . NO-ERROR-FLAG).
+   VARIABLE is the variable to maybe set to nil.
+   NO-ERROR-FLAG is nil if there was an error,
+   anything else meaning no error (so this function does nothing).  */
+Lisp_Object
+reset_var_on_error (val)
+     Lisp_Object val;
+{
+  if (NILP (XCDR (val)))
+    Fset (XCAR (val), Qnil);
+  return Qnil;
+}
+
 /* Signal a change to the buffer immediately before it happens.
    START_INT and END_INT are the bounds of the text to be changed.
 
@@ -2152,6 +2167,7 @@ signal_before_change (start_int, end_int, preserve_ptr)
   Lisp_Object start_marker, end_marker;
   Lisp_Object preserve_marker;
   struct gcpro gcpro1, gcpro2, gcpro3;
+  int count = SPECPDL_INDEX ();
 
   if (inhibit_modification_hooks)
     return;
@@ -2163,6 +2179,8 @@ signal_before_change (start_int, end_int, preserve_ptr)
   end_marker = Qnil;
   GCPRO3 (preserve_marker, start_marker, end_marker);
 
+  specbind (Qinhibit_modification_hooks, Qt);
+
   /* If buffer is unmodified, run a special hook for that case.  */
   if (SAVE_MODIFF >= MODIFF
       && !NILP (Vfirst_change_hook)
@@ -2177,46 +2195,22 @@ signal_before_change (start_int, end_int, preserve_ptr)
   if (!NILP (Vbefore_change_functions))
     {
       Lisp_Object args[3];
-      Lisp_Object before_change_functions;
-      Lisp_Object after_change_functions;
-      struct gcpro gcpro1, gcpro2;
-      struct buffer *old = current_buffer;
-      struct buffer *new;
+      Lisp_Object rvoe_arg = Fcons (Qbefore_change_functions, Qnil);
 
       PRESERVE_VALUE;
       PRESERVE_START_END;
 
-      /* "Bind" before-change-functions and after-change-functions
-         to nil--but in a way that errors don't know about.
-         That way, if there's an error in them, they will stay nil.  */
-      before_change_functions = Vbefore_change_functions;
-      after_change_functions = Vafter_change_functions;
-      Vbefore_change_functions = Qnil;
-      Vafter_change_functions = Qnil;
-      GCPRO2 (before_change_functions, after_change_functions);
+      /* Mark before-change-functions to be reset to nil in case of error.  */
+      record_unwind_protect (reset_var_on_error, rvoe_arg);
 
       /* Actually run the hook functions.  */
       args[0] = Qbefore_change_functions;
       args[1] = FETCH_START;
       args[2] = FETCH_END;
-      run_hook_list_with_args (before_change_functions, 3, args);
+      Frun_hook_with_args (3, args);
 
-      /* "Unbind" the variables we "bound" to nil.  Beware a
-         buffer-local hook which changes the buffer when run (e.g. W3).  */
-      if (old != current_buffer)
-        {
-          new = current_buffer;
-          set_buffer_internal (old);
-          Vbefore_change_functions = before_change_functions;
-          Vafter_change_functions = after_change_functions;
-          set_buffer_internal (new);
-        }
-      else
-        {
-          Vbefore_change_functions = before_change_functions;
-          Vafter_change_functions = after_change_functions;
-        }
-      UNGCPRO;
+      /* There was no error: unarm the reset_on_error.  */
+      XSETCDR (rvoe_arg, Qt);
     }
 
   if (current_buffer->overlays_before || current_buffer->overlays_after)
@@ -2232,6 +2226,8 @@ signal_before_change (start_int, end_int, preserve_ptr)
     free_marker (end_marker);
   RESTORE_VALUE;
   UNGCPRO;
+
+  unbind_to (count, Qnil);
 }
 
 /* Signal a change immediately after it happens.
@@ -2245,6 +2241,7 @@ void
 signal_after_change (charpos, lendel, lenins)
      int charpos, lendel, lenins;
 {
+  int count = SPECPDL_INDEX ();
   if (inhibit_modification_hooks)
     return;
 
@@ -2275,48 +2272,25 @@ signal_after_change (charpos, lendel, lenins)
   if (!NILP (combine_after_change_list))
     Fcombine_after_change_execute ();
 
+  specbind (Qinhibit_modification_hooks, Qt);
+
   if (!NILP (Vafter_change_functions))
     {
       Lisp_Object args[4];
-      Lisp_Object before_change_functions;
-      Lisp_Object after_change_functions;
-      struct buffer *old = current_buffer;
-      struct buffer *new;
-      struct gcpro gcpro1, gcpro2;
-
-      /* "Bind" before-change-functions and after-change-functions
-         to nil--but in a way that errors don't know about.
-         That way, if there's an error in them, they will stay nil.  */
-      before_change_functions = Vbefore_change_functions;
-      after_change_functions = Vafter_change_functions;
-      Vbefore_change_functions = Qnil;
-      Vafter_change_functions = Qnil;
-      GCPRO2 (before_change_functions, after_change_functions);
+      Lisp_Object rvoe_arg = Fcons (Qafter_change_functions, Qnil);
+
+      /* Mark after-change-functions to be reset to nil in case of error.  */
+      record_unwind_protect (reset_var_on_error, rvoe_arg);
 
       /* Actually run the hook functions.  */
       args[0] = Qafter_change_functions;
       XSETFASTINT (args[1], charpos);
       XSETFASTINT (args[2], charpos + lenins);
       XSETFASTINT (args[3], lendel);
-      run_hook_list_with_args (after_change_functions,
-                               4, args);
+      Frun_hook_with_args (4, args);
 
-      /* "Unbind" the variables we "bound" to nil.  Beware a
-         buffer-local hook which changes the buffer when run (e.g. W3).  */
-      if (old != current_buffer)
-        {
-          new = current_buffer;
-          set_buffer_internal (old);
-          Vbefore_change_functions = before_change_functions;
-          Vafter_change_functions = after_change_functions;
-          set_buffer_internal (new);
-        }
-      else
-        {
-          Vbefore_change_functions = before_change_functions;
-          Vafter_change_functions = after_change_functions;
-        }
-      UNGCPRO;
+      /* There was no error: unarm the reset_on_error.  */
+      XSETCDR (rvoe_arg, Qt);
     }
 
   if (current_buffer->overlays_before || current_buffer->overlays_after)
@@ -2332,6 +2306,8 @@ signal_after_change (charpos, lendel, lenins)
   if (lendel == 0)
     report_interval_modification (make_number (charpos),
                                   make_number (charpos + lenins));
+
+  unbind_to (count, Qnil);
 }
 
 Lisp_Object