From: Stefan Monnier <monnier@IRO.UMontreal.CA>
To: Julien Danjou <julien@danjou.info>
Cc: 8274@debbugs.gnu.org
Subject: bug#8274: [PATCH] Use Frun_hooks rather than calling Vrun_hooks manually
Date: Fri, 18 Mar 2011 13:39:25 -0400 [thread overview]
Message-ID: <jwvmxksgxg7.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <1300373551-8071-1-git-send-email-julien@danjou.info> (Julien Danjou's message of "Thu, 17 Mar 2011 15:52:31 +0100")
> 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
next prev parent reply other threads:[~2011-03-18 17:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-17 14:52 bug#8274: [PATCH] Use Frun_hooks rather than calling Vrun_hooks manually Julien Danjou
2011-03-18 17:39 ` Stefan Monnier [this message]
2011-03-21 14:35 ` Julien Danjou
2011-03-23 10:20 ` Julien Danjou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvmxksgxg7.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=8274@debbugs.gnu.org \
--cc=julien@danjou.info \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.