bug#8274: [PATCH] Use Frun_hooks rather than calling Vrun_hooks manually

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> I've seen a little comment in lisp.h indicating that Frun_hooks should be
> the good way to call hooks, which make sense. This patch is a try to
> factorize the remaining code not using that function and remove their direct
> calls to Vrun_hooks.

Thanks, this looks like a good (tho not important) change.
A few comments below.

> +2011-03-17  Julien Danjou  <julien@danjou.info>
> +
> +	* term.c (Fsuspend_tty, Fresume_tty): Use Frun_hooks.
> +
> +	* minibuf.c (read_minibuf, run_exit_minibuf_hook): Use Frun_hooks.
> +
> +	* window.c (temp_output_buffer_show): Use Frun_hooks.
> +
> +	* keyboard.c (Fcommand_execute, Fsuspend_emacs, safe_run_hooks_1):
> +	Use Frun_hooks.
> +	(command_loop_1): Use Frun_hooks. Call safe_run_hooks
> +	unconditionnaly since it does the check itself.
> +
> +	* insdel.c (signal_before_change): Use Frun_hooks.
> +
> +	* frame.c (Fhandle_switch_frame): Use Frun_hooks.
> +
> +	* fileio.c (Fdo_auto_save): Use Frun_hooks.
> +
> +	* emacs.c (Fkill_emacs): Use Frun_hooks.
> +
> +	* editfns.c (save_excursion_restore): Use Frun_hooks.
> +
> +	* cmds.c (internal_self_insert): Use Frun_hooks.
> +
> +	* callint.c (Fcall_interactively): Use Frun_hooks.
> +
> +	* buffer.c (Fkill_all_local_variables): Use Frun_hooks.

You don't need the empty lines between each one of the above, since they
logically belong together.  You can also leave a text empty after the
":" to mean "same as the next".  E.g.

	* cmds.c (internal_self_insert):
	* callint.c (Fcall_interactively):
	* buffer.c (Fkill_all_local_variables): Use Frun_hooks.

I dislike this form when there's an empty line between the items, but
otherwise I like it.
        
> @@ -928,18 +928,21 @@ save_excursion_restore (Lisp_Object info)
>    tem1 = BVAR (current_buffer, mark_active);
>    BVAR (current_buffer, mark_active) = tem;
 
> -  if (!NILP (Vrun_hooks))
> +  /* If mark is active now, and either was not active
> +     or was at a different place, run the activate hook.  */
> +  if (! NILP (BVAR (current_buffer, mark_active)))

[ This is not introduced by your change, but it's a good opportunity to
  clean it up. ]
The above "BVAR (current_buffer, mark_active)" should necessarily be
equal to `tem', so please use `tem' here to clarify.

>      {
> -      /* If mark is active now, and either was not active
> -	 or was at a different place, run the activate hook.  */
> -      if (! NILP (BVAR (current_buffer, mark_active)))
> -	{
> -	  if (! EQ (omark, nmark))
> -	    call1 (Vrun_hooks, intern ("activate-mark-hook"));
> -	}
> -      /* If mark has ceased to be active, run deactivate hook.  */
> -      else if (! NILP (tem1))
> -	call1 (Vrun_hooks, intern ("deactivate-mark-hook"));
> +      if (! EQ (omark, nmark))
> +        {
> +          tem = intern ("activate-mark-hook");
> +          Frun_hooks (1, &tem);
> +        }
> +    }
> +  /* If mark has ceased to be active, run deactivate hook.  */
> +  else if (! NILP (tem1))
> +    {
> +      tem = intern ("deactivate-mark-hook");
> +      Frun_hooks (1, &tem);
>      }

When sending patches for review, you can use "-w" so reindentation does
not interfere.

>    /* If buffer is unmodified, run a special hook for that case.  */
> -  if (SAVE_MODIFF >= MODIFF
> -      && !NILP (Vfirst_change_hook)
> -      && !NILP (Vrun_hooks))
> +  if (SAVE_MODIFF >= MODIFF)

Please leave the !NILP (Vfirst_change_hook) test which is an
optimization (this hook is almost never used).  You could add a quick
comment mentioning it's just an optimization.

> @@ -2597,13 +2594,10 @@ frame's terminal). */)
>        init_sys_modes (t->display_info.tty);
 
>        /* Run `resume-tty-functions'.  */
> -      if (!NILP (Vrun_hooks))
> -        {
> -          Lisp_Object args[2];
> -          args[0] = intern ("resume-tty-functions");
> -          XSETTERMINAL (args[1], t);
> -          Frun_hook_with_args (2, args);
> -        }
> +      Lisp_Object args[2];
> +      args[0] = intern ("resume-tty-functions");
> +      XSETTERMINAL (args[1], t);
> +      Frun_hook_with_args (2, args);
>      }

This declares a variable in the middle of a block, which is a "recent"
feature of C on which we do not want to rely yet.


        Stefan