Subject: Re: bug#31707: [PATCH 1/1] ido: add ido-fallback special variable

From: Eli Zaretskii <[email-masked]>
Date: Sat, 09 Jun 2018 10:00:56 +0300

> From: Christophe Junke <[email-masked]>
> Date: Mon,  4 Jun 2018 10:39:43 +0200
> Cc: Christophe Junke <[email-masked]>
> 
> 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?