Subject: Re: bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable

From: Christophe Junke <[email-masked]>
Date: Mon, 11 Jun 2018 14:54:52 +0200

> I believe this doesn't work, function parameters are always lexically
bound.

Indeed, there is even a warning from disassemble about the lexical argument
shadowing the dynamic one.

Thank you for noticing that.

> So I think your first patch was fine.

Fine.

To recap, the first patch also redefines "ido-fallback-command" so that it
accepts an optional parameter (the fallback command).
Is that ok for you? The idea was to let Ido be exited with a custom
fallback command through a function, and without requiring other packages
to set a variable directly.

Thank you.

On Mon, Jun 11, 2018 at 2:19 PM, Noam Postavsky <[email-masked]> wrote:

> merge 31783 31707
> quit
>
> Christophe Junke <[email-masked]> writes:
>
> > I agree that it is simpler to rename the existing variable, and just
> > add a defvar declaration. Here is a different version of the patch
> > which does only this.
>
> > +;; Indicates which fallback command to call when ido-exit is 'fallback.
> > +(defvar ido-fallback nil)
>
> > -(defun ido-buffer-internal (method &optional fallback prompt default
> initial switch-cmd)
> > +(defun ido-buffer-internal (method &optional ido-fallback prompt
> default initial switch-cmd)
>
> I believe this doesn't work, function parameters are always lexically
> bound.  Compare
>
>     ; -*- lexical-binding: t -*-
>     (setq lexical-binding t) ; for use in *scratch*
>
>     (defvar x nil)
>
>     (disassemble (lambda (x y)
>                    (+ x y)))
>
>     (let ((x 1))
>       (disassemble (lambda (y)
>                      (+ x y))))
>
> So I think your first patch was fine.
>