From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#8274: [PATCH] Use Frun_hooks rather than calling Vrun_hooks manually Date: Fri, 18 Mar 2011 13:39:25 -0400 Message-ID: References: <1300373551-8071-1-git-send-email-julien@danjou.info> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1300471639 14417 80.91.229.12 (18 Mar 2011 18:07:19 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 18 Mar 2011 18:07:19 +0000 (UTC) Cc: 8274@debbugs.gnu.org To: Julien Danjou Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Mar 18 19:07:15 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Q0e4w-000512-P7 for geb-bug-gnu-emacs@m.gmane.org; Fri, 18 Mar 2011 19:07:15 +0100 Original-Received: from localhost ([127.0.0.1]:57158 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q0e4w-0005fj-0c for geb-bug-gnu-emacs@m.gmane.org; Fri, 18 Mar 2011 14:07:14 -0400 Original-Received: from [140.186.70.92] (port=38825 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q0e4m-0005au-UK for bug-gnu-emacs@gnu.org; Fri, 18 Mar 2011 14:07:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q0e4j-0007nF-HQ for bug-gnu-emacs@gnu.org; Fri, 18 Mar 2011 14:07:02 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:48839) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q0e4j-0007nB-Ek for bug-gnu-emacs@gnu.org; Fri, 18 Mar 2011 14:07:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1Q0dec-0005eK-UZ; Fri, 18 Mar 2011 13:40:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 18 Mar 2011 17:40:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 8274 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 8274-submit@debbugs.gnu.org id=B8274.130046997721682 (code B ref 8274); Fri, 18 Mar 2011 17:40:02 +0000 Original-Received: (at 8274) by debbugs.gnu.org; 18 Mar 2011 17:39:37 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q0deC-0005df-HK for submit@debbugs.gnu.org; Fri, 18 Mar 2011 13:39:36 -0400 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q0deA-0005dS-Hg for 8274@debbugs.gnu.org; Fri, 18 Mar 2011 13:39:35 -0400 Original-Received: from faina.iro.umontreal.ca (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id p2IHeUAQ002814; Fri, 18 Mar 2011 13:40:31 -0400 Original-Received: by faina.iro.umontreal.ca (Postfix, from userid 20848) id 08BA81302AB; Fri, 18 Mar 2011 13:39:25 -0400 (EDT) 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") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-NAI-Spam-Flag: NO X-NAI-Spam-Level: * X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 1 X-NAI-Spam-Rules: 2 Rules triggered INFO_TLD=1, RV3800=0 X-NAI-Spam-Version: 2.2.0.9286 : core <3800> : streams <609952> : uri <830167> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Fri, 18 Mar 2011 13:40:02 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:45150 Archived-At: > 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 > + > + * 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