GNU bug report logs - #31707
[PATCH 1/1] ido: add ido-fallback special variable

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Christophe Junke <junke.christophe <at> gmail.com>
Cc: 31707 <at> debbugs.gnu.org
Subject: bug#31707: [PATCH 1/1] ido: add ido-fallback special variable
Date: Sat, 09 Jun 2018 10:00:56 +0300

> From: Christophe Junke <junke.christophe <at> gmail.com>
> Date: Mon,  4 Jun 2018 10:39:43 +0200
> Cc: Christophe Junke <junke.christophe <at> gmail.com>
> 
> The Ido module has been compiled with "lexical-binding: t" for some
> time now. Previously, when the bindings were dynamic, it was possible
> for other packages to modify the "fallback" variables declared inside
> "ido-file-internal" and "ido-buffer-internal".
> 
> In particular, that was the case in magit-extras.el, which runs
> magit-status on current path when exiting Ido. This feature is now
> broken since "fallback" is lexical. For reference, the current code
> for "ido-enter-magit-status" does the following:
> 
>     (with-no-warnings ; FIXME these are internal variables
>       (setq ido-exit 'fallback fallback 'magit-status))
>     (exit-minibuffer)
> 
> I think it would be cleaner to have it do:
> 
>     (ido-fallback-command 'magit-status)
> 
> The current patch:
> 
> - Introduces an ido-fallback special variable, which, when set,
>   overrides the local, lexical, "fallback" variable; it does so only
>   when ido-exit is set to 'fallback.
> 
> - Adds an optional parameter to "ido-fallback-command" that is used to
>   specify which fallback command to run on exit.

Thanks.

However, if the problem was caused by using lexical-binding in ido.el,
then I wonder why you need anything except one additional line:

  (defvar ido-fallback)

This should make ido-fallback a dynamically-bound variable, and all
the rest should "just work" as it did before.  Right?

Previous Next

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