unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#31707: [PATCH 1/1] ido: add ido-fallback special variable
@ 2018-06-04  8:39 Christophe Junke
  2018-06-09  7:00 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Christophe Junke @ 2018-06-04  8:39 UTC (permalink / raw)
  To: 31707; +Cc: Christophe Junke

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.

* lisp/ido.el (ido-fallback): add new variable,
(ido-buffer-internal): reset ido-fallback to nil before prompting user,
(ido-buffer-internal): use ido-fallback when ido-exit is 'fallback,
(ido-file-internal): reset ido-fallback to nil before prompting user,
(ido-file-internal): use ido-fallback when ido-exit is 'fallback,
(ido-fallback-command): add optional argument fallback-command
---
 lisp/ido.el | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/ido.el b/lisp/ido.el
index 705e7dd630..00d0f4f446 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -1242,6 +1242,9 @@ Only used if `ido-use-virtual-buffers' is non-nil.")
 ;; Dynamically bound in ido-read-internal.
 (defvar ido-completing-read)
 
+;; If dynamically set when ido-exit is 'fallback, overrides fallback command.
+(defvar ido-fallback nil)
+
 ;;; FUNCTIONS
 
 (defun ido-active (&optional merge)
@@ -2220,6 +2223,7 @@ If cursor is not at the end of the user input, move to end of input."
 	(run-hook-with-args 'ido-before-fallback-functions
 			    (or fallback 'switch-to-buffer))
 	(call-interactively (or fallback 'switch-to-buffer)))
+    (setq ido-fallback nil)
     (let* ((ido-context-switch-command switch-cmd)
 	   (ido-current-directory nil)
 	   (ido-directory-nonreadable nil)
@@ -2245,7 +2249,7 @@ If cursor is not at the end of the user input, move to end of input."
 
        ((eq ido-exit 'fallback)
 	(let ((read-buffer-function nil))
-	  (setq this-command (or fallback 'switch-to-buffer))
+	  (setq this-command (or ido-fallback fallback 'switch-to-buffer))
 	  (run-hook-with-args 'ido-before-fallback-functions this-command)
 	  (call-interactively this-command)))
 
@@ -2341,6 +2345,7 @@ If cursor is not at the end of the user input, move to end of input."
   ;; Internal function for ido-find-file and friends
   (unless item
     (setq item 'file))
+  (setq ido-fallback nil)
   (let ((ido-current-directory (ido-expand-directory default))
 	(ido-context-switch-command switch-cmd)
 	ido-directory-nonreadable ido-directory-too-big
@@ -2412,7 +2417,7 @@ If cursor is not at the end of the user input, move to end of input."
 	;; we don't want to change directory of current buffer.
 	(let ((default-directory ido-current-directory)
 	      (read-file-name-function nil))
-	  (setq this-command (or fallback 'find-file))
+	  (setq this-command (or ido-fallback fallback 'find-file))
 	  (run-hook-with-args 'ido-before-fallback-functions this-command)
 	  (call-interactively this-command)))
 
@@ -2821,13 +2826,15 @@ If no buffer or file exactly matching the prompt exists, maybe create a new one.
   (setq ido-exit 'takeprompt)
   (exit-minibuffer))
 
-(defun ido-fallback-command ()
-  "Fallback to non-Ido version of current command."
+(defun ido-fallback-command (&optional fallback-command)
+  "Fallback to non-Ido version of current command.
+The optional fallback-command argument indicates which command to run."
   (interactive)
   (let ((i (length ido-text)))
     (while (> i 0)
       (push (aref ido-text (setq i (1- i))) unread-command-events)))
   (setq ido-exit 'fallback)
+  (setq ido-fallback fallback-command)
   (exit-minibuffer))
 
 (defun ido-enter-find-file ()
-- 
2.14.1






^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#31707: [PATCH 1/1] ido: add ido-fallback special variable
  2018-06-04  8:39 bug#31707: [PATCH 1/1] ido: add ido-fallback special variable Christophe Junke
@ 2018-06-09  7:00 ` Eli Zaretskii
  2018-06-11  8:23   ` bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable Christophe Junke
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-09  7:00 UTC (permalink / raw)
  To: Christophe Junke; +Cc: 31707

> From: Christophe Junke <junke.christophe@gmail.com>
> Date: Mon,  4 Jun 2018 10:39:43 +0200
> Cc: Christophe Junke <junke.christophe@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?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-09  7:00 ` Eli Zaretskii
@ 2018-06-11  8:23   ` Christophe Junke
  2018-06-11 12:19     ` Noam Postavsky
  0 siblings, 1 reply; 17+ messages in thread
From: Christophe Junke @ 2018-06-11  8:23 UTC (permalink / raw)
  To: 31783; +Cc: Christophe Junke

- Introduce a global, special ido-fallback variable
- Rename existing occurrences of fallback to ido-fallback

This allows to retain the behaviour that was available prior to
introducing per-file lexical-scope, namely that other modules could
change what fallback command to call when ido-exit is set to
'fallback.
---

Hi,

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.

Thank you for your review and your time.

 lisp/ido.el | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lisp/ido.el b/lisp/ido.el
index 705e7dd630..358e856d4a 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -1242,6 +1242,9 @@ Only used if `ido-use-virtual-buffers' is non-nil.")
 ;; Dynamically bound in ido-read-internal.
 (defvar ido-completing-read)
 
+;; Indicates which fallback command to call when ido-exit is 'fallback.
+(defvar ido-fallback nil)
+
 ;;; FUNCTIONS
 
 (defun ido-active (&optional merge)
@@ -2213,13 +2216,13 @@ If cursor is not at the end of the user input, move to end of input."
     (exit-minibuffer)))
 
 ;;; MAIN FUNCTIONS
-(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)
   ;; Internal function for ido-switch-buffer and friends
   (if (not ido-mode)
       (progn
 	(run-hook-with-args 'ido-before-fallback-functions
-			    (or fallback 'switch-to-buffer))
-	(call-interactively (or fallback 'switch-to-buffer)))
+			    (or ido-fallback 'switch-to-buffer))
+	(call-interactively (or ido-fallback 'switch-to-buffer)))
     (let* ((ido-context-switch-command switch-cmd)
 	   (ido-current-directory nil)
 	   (ido-directory-nonreadable nil)
@@ -2245,7 +2248,7 @@ If cursor is not at the end of the user input, move to end of input."
 
        ((eq ido-exit 'fallback)
 	(let ((read-buffer-function nil))
-	  (setq this-command (or fallback 'switch-to-buffer))
+	  (setq this-command (or ido-fallback 'switch-to-buffer))
 	  (run-hook-with-args 'ido-before-fallback-functions this-command)
 	  (call-interactively this-command)))
 
@@ -2337,7 +2340,7 @@ If cursor is not at the end of the user input, move to end of input."
   ;; Add final slash to result in case it was missing from DEFAULT-DIRECTORY.
   (ido-final-slash (expand-file-name (or dir default-directory)) t))
 
-(defun ido-file-internal (method &optional fallback default prompt item initial switch-cmd)
+(defun ido-file-internal (method &optional ido-fallback default prompt item initial switch-cmd)
   ;; Internal function for ido-find-file and friends
   (unless item
     (setq item 'file))
@@ -2412,7 +2415,7 @@ If cursor is not at the end of the user input, move to end of input."
 	;; we don't want to change directory of current buffer.
 	(let ((default-directory ido-current-directory)
 	      (read-file-name-function nil))
-	  (setq this-command (or fallback 'find-file))
+	  (setq this-command (or ido-fallback 'find-file))
 	  (run-hook-with-args 'ido-before-fallback-functions this-command)
 	  (call-interactively this-command)))
 
@@ -2496,10 +2499,10 @@ If cursor is not at the end of the user input, move to end of input."
        ((eq method 'read-only)
 	(ido-record-work-file filename)
 	(setq filename (concat ido-current-directory filename))
-	(ido-record-command fallback filename)
+	(ido-record-command ido-fallback filename)
 	(ido-record-work-directory)
-	(run-hook-with-args 'ido-before-fallback-functions fallback)
-	(funcall fallback filename))
+	(run-hook-with-args 'ido-before-fallback-functions ido-fallback)
+	(funcall ido-fallback filename))
 
        ((eq method 'insert)
 	(ido-record-work-file filename)
-- 
2.14.1






^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-11  8:23   ` bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable Christophe Junke
@ 2018-06-11 12:19     ` Noam Postavsky
  2018-06-11 12:54       ` Christophe Junke
  2018-06-11 15:28       ` Eli Zaretskii
  0 siblings, 2 replies; 17+ messages in thread
From: Noam Postavsky @ 2018-06-11 12:19 UTC (permalink / raw)
  To: Christophe Junke; +Cc: 31783

merge 31783 31707
quit

Christophe Junke <junke.christophe@gmail.com> 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.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-11 12:19     ` Noam Postavsky
@ 2018-06-11 12:54       ` Christophe Junke
  2018-06-11 15:28       ` Eli Zaretskii
  1 sibling, 0 replies; 17+ messages in thread
From: Christophe Junke @ 2018-06-11 12:54 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 31783

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

 > 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 <npostavs@gmail.com> wrote:

> merge 31783 31707
> quit
>
> Christophe Junke <junke.christophe@gmail.com> 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.
>

[-- Attachment #2: Type: text/html, Size: 2426 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-11 12:19     ` Noam Postavsky
  2018-06-11 12:54       ` Christophe Junke
@ 2018-06-11 15:28       ` Eli Zaretskii
       [not found]         ` <CAFDFyRiHzxOB7Q6uV1hPYmuC3KfiqJRCmk=nrQ5wTPWUue_W4Q@mail.gmail.com>
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-11 15:28 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: junke.christophe, 31783

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Mon, 11 Jun 2018 08:19:03 -0400
> Cc: 31783@debbugs.gnu.org
> 
> Christophe Junke <junke.christophe@gmail.com> 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.

There's some misunderstanding here, most probably mine.  Sorry; please
help me understand what am I missing.

The original report said that the problem was caused by using
lexical-binding in ido.el, so I proposed to defvar the offending
variable to make it dynamically bound, which is the boilerplate
solution for all such problems.  I thought that was all that was
needed, and I definitely didn't suggest to rename anything.

What did I miss?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
       [not found]         ` <CAFDFyRiHzxOB7Q6uV1hPYmuC3KfiqJRCmk=nrQ5wTPWUue_W4Q@mail.gmail.com>
@ 2018-06-11 16:55           ` Eli Zaretskii
  2018-06-22  0:34             ` Noam Postavsky
  2018-06-11 18:52           ` bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable Christophe Junke
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-11 16:55 UTC (permalink / raw)
  To: Christophe Junke; +Cc: 31783, Noam Postavsky

> From: Christophe Junke <junke.christophe@gmail.com>
> Date: Mon, 11 Jun 2018 18:16:07 +0200
> 
> With respect to naming: my possibly wrong understanding of 
> Emacs Lisp coding conventions is that special variables should be 
> prefixed by the package's name, and that's why I (re)named 
> the variable ido-fallback in both patches. 
> 
> Also, here is a summary of the original problem, as I see it.

Ah, okay.  Thanks for explaining this, I agree that your original
change was TRT.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
       [not found]         ` <CAFDFyRiHzxOB7Q6uV1hPYmuC3KfiqJRCmk=nrQ5wTPWUue_W4Q@mail.gmail.com>
  2018-06-11 16:55           ` Eli Zaretskii
@ 2018-06-11 18:52           ` Christophe Junke
  1 sibling, 0 replies; 17+ messages in thread
From: Christophe Junke @ 2018-06-11 18:52 UTC (permalink / raw)
  To: 31783

[-- Attachment #1: Type: text/plain, Size: 3578 bytes --]

(I forgot to cc the list, here is the message I sent previously)

> I thought that was all that was
needed, and I definitely didn't suggest to rename anything.
> What did I miss?

With respect to naming: my possibly wrong understanding of
Emacs Lisp coding conventions is that special variables should be
prefixed by the package's name, and that's why I (re)named
the variable ido-fallback in both patches.

Also, here is a summary of the original problem, as I see it.

Consider an IDO function which accepts a parameter named FALLBACK, and
which calls PROMPT. Inside PROMPT, we set FALLBACK to another
value. For example:

    ;; -*- lexical-binding: nil -*-

    (defun prompt ()
      (setf fallback 10))

    (defun ido (fallback)
      (prompt)
      fallback)

When the above is evaluated with lexical binding being nil, the
fallback variable is set from within "prompt" and the result from
calling ido is always 10, no matter its input argument.

This is not the case if the code is evaluated with lexical-binding set
to T, in which case "ido" returns the value bound to the lexical
fallback variable: (ido 5) returns 5.

The intent of the patches was to allow fallback to be changed again.

As I learnt with the second patch, globally declaring "fallback" as a
special variable with defvar, with lexical-binding set to T, does not
give the same behaviour as with dynamic scoping rules: the "fallback"
parameter is still bound lexically, shadowing the dynamic binding.

The first patch introduces a globally scoped ido-fallback variable,
different from the "fallback" argument.

Yet another possibility would be to get rid of the optional "fallback"
argument
and keep only a global "ido-fallback", like "ido-exit", but that requires
to change
all call sites.

I hope this is clear.

Suggestions are welcome, please tell me if I misunderstood something or
if there are better ways to reach the original goal.








On Mon, Jun 11, 2018 at 5:28 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Noam Postavsky <npostavs@gmail.com>
> > Date: Mon, 11 Jun 2018 08:19:03 -0400
> > Cc: 31783@debbugs.gnu.org
> >
> > Christophe Junke <junke.christophe@gmail.com> 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.
>
> There's some misunderstanding here, most probably mine.  Sorry; please
> help me understand what am I missing.
>
> The original report said that the problem was caused by using
> lexical-binding in ido.el, so I proposed to defvar the offending
> variable to make it dynamically bound, which is the boilerplate
> solution for all such problems.  I thought that was all that was
> needed, and I definitely didn't suggest to rename anything.
>
> What did I miss?
>

[-- Attachment #2: Type: text/html, Size: 5063 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-11 16:55           ` Eli Zaretskii
@ 2018-06-22  0:34             ` Noam Postavsky
  2018-06-22  6:34               ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Postavsky @ 2018-06-22  0:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Christophe Junke, 31783

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Christophe Junke <junke.christophe@gmail.com>
>> Date: Mon, 11 Jun 2018 18:16:07 +0200
>> 
>> With respect to naming: my possibly wrong understanding of 
>> Emacs Lisp coding conventions is that special variables should be 
>> prefixed by the package's name, and that's why I (re)named 
>> the variable ido-fallback in both patches. 
>> 
>> Also, here is a summary of the original problem, as I see it.
>
> Ah, okay.  Thanks for explaining this, I agree that your original
> change was TRT.

So should this go to emacs-26?  It's introducing a new feature, but that
makes up for a semi-accidental feature that was lost.

And what's the copyright situation?






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-22  0:34             ` Noam Postavsky
@ 2018-06-22  6:34               ` Eli Zaretskii
  2018-06-22  8:24                 ` Christophe Junke
  2018-06-22 11:32                 ` Noam Postavsky
  0 siblings, 2 replies; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-22  6:34 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: junke.christophe, 31783

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: Christophe Junke <junke.christophe@gmail.com>,  31783@debbugs.gnu.org
> Date: Thu, 21 Jun 2018 20:34:10 -0400
> 
> > Ah, okay.  Thanks for explaining this, I agree that your original
> > change was TRT.
> 
> So should this go to emacs-26?

Yes, please.

> It's introducing a new feature, but that makes up for a
> semi-accidental feature that was lost.

What new feature is being introduced there?  I couldn't see anything
new, just fixing a breakage.

> And what's the copyright situation?

There's no assignment.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-22  6:34               ` Eli Zaretskii
@ 2018-06-22  8:24                 ` Christophe Junke
  2018-06-22  9:02                   ` Eli Zaretskii
  2018-06-22 11:32                 ` Noam Postavsky
  1 sibling, 1 reply; 17+ messages in thread
From: Christophe Junke @ 2018-06-22  8:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 31783, Noam Postavsky

[-- Attachment #1: Type: text/plain, Size: 175 bytes --]

>
> > And what's the copyright situation?
>
> There's no assignment.
>

I am willing to assign copyright for this change (and future changes).
Can you send me a form?
Thanks.

[-- Attachment #2: Type: text/html, Size: 444 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-22  8:24                 ` Christophe Junke
@ 2018-06-22  9:02                   ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-22  9:02 UTC (permalink / raw)
  To: Christophe Junke; +Cc: 31783, npostavs

> From: Christophe Junke <junke.christophe@gmail.com>
> Date: Fri, 22 Jun 2018 10:24:44 +0200
> Cc: Noam Postavsky <npostavs@gmail.com>, 31783@debbugs.gnu.org
> 
>  > And what's the copyright situation?
> 
>  There's no assignment.
> 
> I am willing to assign copyright for this change (and future changes).
> Can you send me a form?

For sent off-list.  However, this patch is small enough to be accepted
without waiting for the legal paperwork to be completed.

Thanks.






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-22  6:34               ` Eli Zaretskii
  2018-06-22  8:24                 ` Christophe Junke
@ 2018-06-22 11:32                 ` Noam Postavsky
  2018-06-22 12:45                   ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: Noam Postavsky @ 2018-06-22 11:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: junke.christophe, 31783

Eli Zaretskii <eliz@gnu.org> writes:
>
> What new feature is being introduced there?  I couldn't see anything
> new, just fixing a breakage.

The optional parameter to ido-fallback-command is new.

The ido-fallback is sort of a new variable, although it's arguably just
a proper prefixing of the old `fallback' one.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable
  2018-06-22 11:32                 ` Noam Postavsky
@ 2018-06-22 12:45                   ` Eli Zaretskii
  2018-06-24  1:52                     ` bug#31707: [PATCH 1/1] ido: add ido-fallback special variable Noam Postavsky
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-22 12:45 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: junke.christophe, 31783

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: junke.christophe@gmail.com,  31783@debbugs.gnu.org
> Date: Fri, 22 Jun 2018 07:32:14 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >
> > What new feature is being introduced there?  I couldn't see anything
> > new, just fixing a breakage.
> 
> The optional parameter to ido-fallback-command is new.
> 
> The ido-fallback is sort of a new variable, although it's arguably just
> a proper prefixing of the old `fallback' one.

Maybe I'm confused about which patch will eventually go in.  Can you
show it?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31707: [PATCH 1/1] ido: add ido-fallback special variable
  2018-06-22 12:45                   ` Eli Zaretskii
@ 2018-06-24  1:52                     ` Noam Postavsky
  2018-06-24 14:54                       ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Postavsky @ 2018-06-24  1:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: junke.christophe, 31707

[moving discussion in #31783 back to original #31707]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@gmail.com>
>> Cc: junke.christophe@gmail.com,  31783@debbugs.gnu.org
>> Date: Fri, 22 Jun 2018 07:32:14 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> >
>> > What new feature is being introduced there?  I couldn't see anything
>> > new, just fixing a breakage.
>> 
>> The optional parameter to ido-fallback-command is new.
>> 
>> The ido-fallback is sort of a new variable, although it's arguably just
>> a proper prefixing of the old `fallback' one.
>
> Maybe I'm confused about which patch will eventually go in.  Can you
> show it?

The one in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31707#5.

Depending how strict we want to be about changes, we could split off the
ido-fallback-command parameter addition from the ido-fallback variable.







^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31707: [PATCH 1/1] ido: add ido-fallback special variable
  2018-06-24  1:52                     ` bug#31707: [PATCH 1/1] ido: add ido-fallback special variable Noam Postavsky
@ 2018-06-24 14:54                       ` Eli Zaretskii
  2018-06-26  0:40                         ` Noam Postavsky
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-06-24 14:54 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: junke.christophe, 31707

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: junke.christophe@gmail.com,  31707@debbugs.gnu.org
> Date: Sat, 23 Jun 2018 21:52:29 -0400
> 
> >> > What new feature is being introduced there?  I couldn't see anything
> >> > new, just fixing a breakage.
> >> 
> >> The optional parameter to ido-fallback-command is new.
> >> 
> >> The ido-fallback is sort of a new variable, although it's arguably just
> >> a proper prefixing of the old `fallback' one.
> >
> > Maybe I'm confused about which patch will eventually go in.  Can you
> > show it?
> 
> The one in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31707#5.

So I wasn't confused, thanks.

> Depending how strict we want to be about changes, we could split off the
> ido-fallback-command parameter addition from the ido-fallback variable.

No, I don't think this would be necessary.  We are adding a variable
to fix breakage.  I think this can go to emacs-26 as-is.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#31707: [PATCH 1/1] ido: add ido-fallback special variable
  2018-06-24 14:54                       ` Eli Zaretskii
@ 2018-06-26  0:40                         ` Noam Postavsky
  0 siblings, 0 replies; 17+ messages in thread
From: Noam Postavsky @ 2018-06-26  0:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: junke.christophe, 31707

tags 31707 fixed
close 31707 26.2
quit

Eli Zaretskii <eliz@gnu.org> writes:

> No, I don't think this would be necessary.  We are adding a variable
> to fix breakage.  I think this can go to emacs-26 as-is.

Done.

[1: 12c77f6918]: 2018-06-25 20:05:53 -0400
  Add ido-fallback special variable (Bug#31707)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=12c77f6918c4a60dbbae3f716a58300b4026e8da





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-06-26  0:40 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-04  8:39 bug#31707: [PATCH 1/1] ido: add ido-fallback special variable Christophe Junke
2018-06-09  7:00 ` Eli Zaretskii
2018-06-11  8:23   ` bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable Christophe Junke
2018-06-11 12:19     ` Noam Postavsky
2018-06-11 12:54       ` Christophe Junke
2018-06-11 15:28       ` Eli Zaretskii
     [not found]         ` <CAFDFyRiHzxOB7Q6uV1hPYmuC3KfiqJRCmk=nrQ5wTPWUue_W4Q@mail.gmail.com>
2018-06-11 16:55           ` Eli Zaretskii
2018-06-22  0:34             ` Noam Postavsky
2018-06-22  6:34               ` Eli Zaretskii
2018-06-22  8:24                 ` Christophe Junke
2018-06-22  9:02                   ` Eli Zaretskii
2018-06-22 11:32                 ` Noam Postavsky
2018-06-22 12:45                   ` Eli Zaretskii
2018-06-24  1:52                     ` bug#31707: [PATCH 1/1] ido: add ido-fallback special variable Noam Postavsky
2018-06-24 14:54                       ` Eli Zaretskii
2018-06-26  0:40                         ` Noam Postavsky
2018-06-11 18:52           ` bug#31783: [PATCH v2] ido.el: define a special ido-fallback variable Christophe Junke

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).