GNU bug report logs - #8387
24.0.50; `run-hook-with-args' return value has changed

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8387 in the body.
You can then email your comments to 8387 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Welsh Duggan <md5i <at> md5i.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.0.50; `run-hook-with-args' return value has changed
Date: Thu, 31 Mar 2011 02:25:34 -0400

emacs -Q
(defun foo (arg) 'test) C-j
(defvar foo-hook '(foo)) C-j
(run-hook-with-args 'foo-hook 'foo) C-j

In the past, this would return 'test instead of nil.  This breaks some
code, in particular bbdb, which has the following:

(defun bbdb-canonicalize-address (net)
  ;; call the bbdb-canonicalize-net-hook repeatedly until it returns a
  ;; value eq to the value passed in.  This implies that it can't
  ;; destructively modify the string.

  ;; Hysterical Raisins: This is a function, not a hook. In order to
  ;; make this hook a hook, we'll quietly convert a single function
  ;; into a hook list.  We should really warn the user that we're
  ;; doing this, and advise them to update their configuration
  ;; accordingly. For the release, maybe.
  (if (functionp bbdb-canonicalize-net-hook)
      (setq bbdb-canonicalize-net-hook (list bbdb-canonicalize-net-hook)))

  ;; Now, do the hook run. Note, if you mess up, it's possible that
  ;; BBDB will get stuck here oscillating between various definitions
  ;; of the canonical address.
  (while (not (equal net (setq net (run-hook-with-args
                                    'bbdb-canonicalize-net-hook net)))))

  net)

I don't have time to test this right now, but I would guess this started
happening in the following commit:

2011-03-28  Stefan Monnier  <monnier <at> iro.umontreal.ca>

	* keyboard.c (safe_run_hook_funcall): New function.
	(safe_run_hooks_1, safe_run_hooks_error, safe_run_hooks): On error,
	don't set the hook to nil, but remove the offending function instead.
	(Qcommand_hook_internal): Remove, unused.
	(syms_of_keyboard): Don't initialize Qcommand_hook_internal nor define
	Vcommand_hook_internal.

	* eval.c (enum run_hooks_condition): Remove.
	(funcall_nil, funcall_not): New functions.
	(run_hook_with_args): Call each function through a `funcall' argument.
	Remove `cond' argument, now redundant.
	(Frun_hooks, Frun_hook_with_args, Frun_hook_with_args_until_success)
	(Frun_hook_with_args_until_failure): Adjust accordingly.
	(run_hook_wrapped_funcall, Frun_hook_wrapped): New functions.



In GNU Emacs 24.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.3)
 of 2011-03-30 on maru
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
configured using `configure  '--without-toolkit-scroll-bars''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

-- 
Michael Welsh Duggan
(md5i <at> md5i.com)

Message #8 received at 8387 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Welsh Duggan <md5i <at> md5i.com>
Cc: 8387 <at> debbugs.gnu.org
Subject: Re: bug#8387: 24.0.50; `run-hook-with-args' return value has changed
Date: Thu, 31 Mar 2011 11:02:49 -0400

> (defun foo (arg) 'test) C-j
> (defvar foo-hook '(foo)) C-j
> (run-hook-with-args 'foo-hook 'foo) C-j

> In the past, this would return 'test instead of nil.  This breaks some
[...]
> I don't have time to test this right now, but I would guess this started
> happening in the following commit:
[...]
> 	(run_hook_with_args): Call each function through a `funcall' argument.
> 	Remove `cond' argument, now redundant.
> 	(Frun_hooks, Frun_hook_with_args, Frun_hook_with_args_until_success)
> 	(Frun_hook_with_args_until_failure): Adjust accordingly.

No doubt it's the origin of the new behavior.
But note that the above change has not touched the docstring of
run-hook-with-args which says:

   It is best not to depend on the value returned by `run-hook-with-args',
   as that may change.

Looking at the BBDB code, I think we don't need to try and accomodate it
because it is fundamentally broken, not just because of the above
docstring warning:

  ;; Hysterical Raisins: This is a function, not a hook. In order to
  ;; make this hook a hook, we'll quietly convert a single function
  ;; into a hook list.  We should really warn the user that we're
  ;; doing this, and advise them to update their configuration
  ;; accordingly. For the release, maybe.
  (if (functionp bbdb-canonicalize-net-hook)
      (setq bbdb-canonicalize-net-hook (list bbdb-canonicalize-net-hook)))

  ;; Now, do the hook run. Note, if you mess up, it's possible that
  ;; BBDB will get stuck here oscillating between various definitions
  ;; of the canonical address.
  (while (not (equal net (setq net (run-hook-with-args
                                    'bbdb-canonicalize-net-hook net)))))

So bbdb-canonicalize-net-hook can start as a single function or a list
of functions.  But the code is explicit about wanting a list of
functions.  Now if this list contains N>1 function, then
run-hook-with-args has to call all those functions and discard at least
N-1 of the results.  I.e. only one of the functions on
bbdb-canonicalize-net-hook can work (in Emacs-23, one of them happened
to work, and on the current trunk, none of them work).


        Stefan

Message #13 received at 8387-done <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Michael Welsh Duggan <md5i <at> md5i.com>
Cc: 8387-done <at> debbugs.gnu.org
Subject: Re: bug#8387: 24.0.50; `run-hook-with-args' return value has changed
Date: Thu, 31 Mar 2011 17:02:21 +0200

On Thu, Mar 31, 2011 at 08:25, Michael Welsh Duggan <md5i <at> md5i.com> wrote:

> (run-hook-with-args 'foo-hook 'foo) C-j
>
> In the past, this would return 'test instead of nil.  This breaks some
> code, in particular bbdb, which has the following:

I'm closing this one, as it is not a bug.

The docstring of `run-hook-with-args' contains this bit, since at
least Emacs 22.1:

"It is best not to depend on the value returned by `run-hook-with-args',
as that may change."

    Juanma

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.