[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#8274: [PATCH] Use Frun_hooks rather than calling Vrun_hooks manually
From: |
Stefan Monnier |
Subject: |
bug#8274: [PATCH] Use Frun_hooks rather than calling Vrun_hooks manually |
Date: |
Fri, 18 Mar 2011 13:39:25 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
> 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