Bug: emacs 23.2 doesn't like ido-completing-read

Bug: emacs 23.2 doesn't like ido-completing-read

[Mark Walters]

Hello

I have been experimenting with notmuch-always-prompt-for-sender on my
debian stable setup (emacs 23.2.1) and it doesn't like
ido-completing-read. It goes to the minibuffer and then it seems to be
impossible to exit the minibuffer.

I can find an emacs bug report #3274 and some discussion
http://comments.gmane.org/gmane.emacs.bugs/27856 which indicates that it
is a problem with ido initialisation. Unfortunately I can't get from there to a
solution (except upgrade emacs).

Many thanks

Mark

Re: Bug: emacs 23.2 doesn't like ido-completing-read

[David Edmondson]

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

On Sun, 29 Jan 2012 23:17:55 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> I have been experimenting with notmuch-always-prompt-for-sender on my
> debian stable setup (emacs 23.2.1) and it doesn't like
> ido-completing-read. It goes to the minibuffer and then it seems to be
> impossible to exit the minibuffer.
> 
> I can find an emacs bug report #3274 and some discussion
> http://comments.gmane.org/gmane.emacs.bugs/27856 which indicates that it
> is a problem with ido initialisation. Unfortunately I can't get from there to a
> solution (except upgrade emacs).

I'm not sure that notmuch-mua.el should be using `ido-completing-read',
and certainly not without `require'ing it.

Is there a particular thing required that `completing-read' doesn't do?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Bug: emacs 23.2 doesn't like ido-completing-read

[Mark Walters]

On Mon, 30 Jan 2012 09:39:32 +0000, David Edmondson <dme@dme.org> wrote:
> On Sun, 29 Jan 2012 23:17:55 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> > I have been experimenting with notmuch-always-prompt-for-sender on my
> > debian stable setup (emacs 23.2.1) and it doesn't like
> > ido-completing-read. It goes to the minibuffer and then it seems to be
> > impossible to exit the minibuffer.
> > 
> > I can find an emacs bug report #3274 and some discussion
> > http://comments.gmane.org/gmane.emacs.bugs/27856 which indicates that it
> > is a problem with ido initialisation. Unfortunately I can't get from there to a
> > solution (except upgrade emacs).
> 
> I'm not sure that notmuch-mua.el should be using `ido-completing-read',
> and certainly not without `require'ing it.
> 
> Is there a particular thing required that `completing-read' doesn't do?

Thanks for the suggestion: I can confirm that with ido-completing-read
replaced by completing-read it works nicely. The "ido" version shows the
possibilities for you which is nice, but this works fine.

Many thanks

Mark

Re: Bug: emacs 23.2 doesn't like ido-completing-read

[Mark Walters]

On Mon, 30 Jan 2012 10:21:38 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> On Mon, 30 Jan 2012 09:39:32 +0000, David Edmondson <dme@dme.org> wrote:
> > On Sun, 29 Jan 2012 23:17:55 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> > > I have been experimenting with notmuch-always-prompt-for-sender on my
> > > debian stable setup (emacs 23.2.1) and it doesn't like
> > > ido-completing-read. It goes to the minibuffer and then it seems to be
> > > impossible to exit the minibuffer.
> > > 
> > > I can find an emacs bug report #3274 and some discussion
> > > http://comments.gmane.org/gmane.emacs.bugs/27856 which indicates that it
> > > is a problem with ido initialisation. Unfortunately I can't get from there to a
> > > solution (except upgrade emacs).
> > 
> > I'm not sure that notmuch-mua.el should be using `ido-completing-read',
> > and certainly not without `require'ing it.
> > 
> > Is there a particular thing required that `completing-read' doesn't do?
> 
> Thanks for the suggestion: I can confirm that with ido-completing-read
> replaced by completing-read it works nicely. The "ido" version shows the
> possibilities for you which is nice, but this works fine.

I hadn't realised before but this also goes wrong if you use the prefix
argument to mail (eg ctrl-u m). The current code seems to mess up the
mini-buffer and even closing the buffer does not get out of it.

Changing ido-completing-read read to completing-read seems to fix the
problem entirely. Since this will affect most debian stable users (*) I
think we should fix it. The trivial patch below works for me but other solutions
may be better as they could maintain the benefits of ido-completing-read
(eg that it shows the possibilities).

Best wishes

Mark

(*) Perhaps only those who have more than one email address.

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 023645e..41f82c2 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -191,7 +191,7 @@ the From: header is already filled in by notmuch."
     ;; Now prompt the user, either for an email address only or for a full identity.
     (if one-name-only
 	(let ((address
-	       (ido-completing-read (concat "Sender address for " name ": ") addresses
+	       (completing-read (concat "Sender address for " name ": ") addresses
 				    nil nil nil 'notmuch-mua-sender-history (car addresses))))
 	  (concat name " <" address ">"))
       (ido-completing-read "Send mail From: " notmuch-identities
-- 
1.7.2.3

Re: Bug: emacs 23.2 doesn't like ido-completing-read

[Tomi Ollila]

On Sat, 04 Feb 2012 12:31:57 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> I hadn't realised before but this also goes wrong if you use the prefix
> argument to mail (eg ctrl-u m). The current code seems to mess up the
> mini-buffer and even closing the buffer does not get out of it.
> 
> Changing ido-completing-read read to completing-read seems to fix the
> problem entirely. Since this will affect most debian stable users (*) I
> think we should fix it. The trivial patch below works for me but other solutions
> may be better as they could maintain the benefits of ido-completing-read
> (eg that it shows the possibilities).
>
> Best wishes
> 
> Mark
> 
> (*) Perhaps only those who have more than one email address.
> 
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 023645e..41f82c2 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -191,7 +191,7 @@ the From: header is already filled in by notmuch."
>      ;; Now prompt the user, either for an email address only or for a full identity.
>      (if one-name-only
>  	(let ((address
> -	       (ido-completing-read (concat "Sender address for " name ": ") addresses
> +	       (completing-read (concat "Sender address for " name ": ") addresses
>  				    nil nil nil 'notmuch-mua-sender-history (car addresses))))
>  	  (concat name " <" address ">"))
>        (ido-completing-read "Send mail From: " notmuch-identities

Should that other ido-completing-read be kept there -- it is the only
one left after this patch...

Tomi


> -- 
> 1.7.2.3

Re: Bug: emacs 23.2 doesn't like ido-completing-read

[Mark Walters]

On Sat, 04 Feb 2012 22:14:25 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sat, 04 Feb 2012 12:31:57 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> > 
> > I hadn't realised before but this also goes wrong if you use the prefix
> > argument to mail (eg ctrl-u m). The current code seems to mess up the
> > mini-buffer and even closing the buffer does not get out of it.
> > 
> > Changing ido-completing-read read to completing-read seems to fix the
> > problem entirely. Since this will affect most debian stable users (*) I
> > think we should fix it. The trivial patch below works for me but other solutions
> > may be better as they could maintain the benefits of ido-completing-read
> > (eg that it shows the possibilities).
> >
> > Best wishes
> > 
> > Mark
> > 
> > (*) Perhaps only those who have more than one email address.
> > 
> > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> > index 023645e..41f82c2 100644
> > --- a/emacs/notmuch-mua.el
> > +++ b/emacs/notmuch-mua.el
> > @@ -191,7 +191,7 @@ the From: header is already filled in by notmuch."
> >      ;; Now prompt the user, either for an email address only or for a full identity.
> >      (if one-name-only
> >  	(let ((address
> > -	       (ido-completing-read (concat "Sender address for " name ": ") addresses
> > +	       (completing-read (concat "Sender address for " name ": ") addresses
> >  				    nil nil nil 'notmuch-mua-sender-history (car addresses))))
> >  	  (concat name " <" address ">"))
> >        (ido-completing-read "Send mail From: " notmuch-identities
> 
> Should that other ido-completing-read be kept there -- it is the only
> one left after this patch...

Whoops: I hadn't even noticed that one. Is there any way of fixing it
so ido-completing-read works (see the link in my first message
id:"871uqijdy4.fsf@qmul.ac.uk")? Not because I want the extra
functionality but perhaps someone else likes it as it is.

Many thanks

Mark

[PATCH] emacs: initialize ido mode in notmuch-mua.el

[Mark Walters]

ido-completing-read is used in notmuch-mua.el without any
initialization.  This should work but a bug (emacs bug #3247) in some
emacs 23 versions (including 23.2.1 from Debian stable) causes emacs
to get stuck in the mini-buffer. This tests to see if ido-mode has
been initialized and if not calls it. This avoids the bug for these
older versions of emacs.

This is the only place that ido-completing-read is used so an alternative 
would be to replace ido-completing-read by completing-read but cworth 
expressed a preference for the ido version in 
id:"87wrhfvk6a.fsf@yoom.home.cworth.org"


---
 emacs/notmuch-mua.el |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 4be7c13..9fe75db 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -194,6 +194,8 @@ the From: header is already filled in by notmuch."
 	    addresses     (cons (notmuch-user-primary-email) (notmuch-user-other-email))
 	    one-name-only t))
     ;; Now prompt the user, either for an email address only or for a full identity.
+    (unless ido-mode
+      (ido-mode t))
     (if one-name-only
 	(let ((address
 	       (ido-completing-read (concat "Sender address for " name ": ") addresses
-- 
1.7.2.3

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> ido-completing-read is used in notmuch-mua.el without any
> initialization.  This should work but a bug (emacs bug #3247) in some
> emacs 23 versions (including 23.2.1 from Debian stable) causes emacs
> to get stuck in the mini-buffer. This tests to see if ido-mode has
> been initialized and if not calls it. This avoids the bug for these
> older versions of emacs.
>
> This is the only place that ido-completing-read is used so an alternative 
> would be to replace ido-completing-read by completing-read but cworth 
> expressed a preference for the ido version in 
> id:"87wrhfvk6a.fsf@yoom.home.cworth.

This patch was tagged wip, but then there was no progress ;).  It still
applies cleanly, and doesn't seem to do any harm in emacs 23.4 nor
emacs24.3. I don't have 23.2 at hand but I guess Tomi or Mark can
confirm it actually fixes the bug there.

I propose we either apply the patch or ::wontfix the bug.

cheers,

d

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[Tomi Ollila]

On Sat, Jan 25 2014, David Bremner <david@tethera.net> wrote:

> Mark Walters <markwalters1009@gmail.com> writes:
>
>> ido-completing-read is used in notmuch-mua.el without any
>> initialization.  This should work but a bug (emacs bug #3247) in some
>> emacs 23 versions (including 23.2.1 from Debian stable) causes emacs
>> to get stuck in the mini-buffer. This tests to see if ido-mode has
>> been initialized and if not calls it. This avoids the bug for these
>> older versions of emacs.
>>
>> This is the only place that ido-completing-read is used so an alternative 
>> would be to replace ido-completing-read by completing-read but cworth 
>> expressed a preference for the ido version in 
>> id:"87wrhfvk6a.fsf@yoom.home.cworth.
>
> This patch was tagged wip, but then there was no progress ;).  It still
> applies cleanly, and doesn't seem to do any harm in emacs 23.4 nor
> emacs24.3. I don't have 23.2 at hand but I guess Tomi or Mark can
> confirm it actually fixes the bug there.
>
> I propose we either apply the patch or ::wontfix the bug.

How about doing it with defadvice with something like:

(from http://comments.gmane.org/gmane.emacs.bugs/27856 one can notice
this happening with emacs 23.3 -- David can check whether 23.4 if
affected)

(if (and (= emacs-major-version 23) (< emacs-minor-version 4))
  (defadvice ido-completing-read (before notmuch-ido-mode-init activate)
      (unless (ido-mode) (ido-mode t))
      (ad-disable-advice 'ido-completing-read 'before 'notmuch-ido-mode-init)))

>
> cheers,
>
> d


Tomi

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> How about doing it with defadvice with something like:
>
> (from http://comments.gmane.org/gmane.emacs.bugs/27856 one can notice
> this happening with emacs 23.3 -- David can check whether 23.4 if
> affected)

It's fixed in 23.4

>
> (if (and (= emacs-major-version 23) (< emacs-minor-version 4))
>   (defadvice ido-completing-read (before notmuch-ido-mode-init activate)
>       (unless (ido-mode) (ido-mode t))
>       (ad-disable-advice 'ido-completing-read 'before 'notmuch-ido-mode-init)))

Advice always seems over complex to me, but maybe I'm just
prejudiced/ignorant.

Can you decode/reverse-engineer the fix in 

    http://permalink.gmane.org/gmane.emacs.bugs/41957

?

Maybe the advice or whatever should call 

   (ido-init-completion-maps)
   (add-hook 'minibuffer-setup-hook 'ido-minibuffer-setup)
   (add-hook 'choose-completion-string-functions
      'ido-choose-completion-string))

Although that looks worryingly permanent.



d

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[Tomi Ollila]

On Sun, Jan 26 2014, David Bremner <david@tethera.net> wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> How about doing it with defadvice with something like:
>>
>> (from http://comments.gmane.org/gmane.emacs.bugs/27856 one can notice
>> this happening with emacs 23.3 -- David can check whether 23.4 if
>> affected)
>
> It's fixed in 23.4
>
>>
>> (if (and (= emacs-major-version 23) (< emacs-minor-version 4))
>>   (defadvice ido-completing-read (before notmuch-ido-mode-init activate)
>>       (unless (ido-mode) (ido-mode t))
>>       (ad-disable-advice 'ido-completing-read 'before 'notmuch-ido-mode-init)))
>
> Advice always seems over complex to me, but maybe I'm just
> prejudiced/ignorant.

With defadvice we can keep "mainline code less messy and less hard to 
maintain" ;) -- and only install the advice on versions that need it.

Also with defadvice the other use of ido-completing-read is covered.

> Can you decode/reverse-engineer the fix in 
>
>     http://permalink.gmane.org/gmane.emacs.bugs/41957
>
> ?
>
> Maybe the advice or whatever should call 
>
>    (ido-init-completion-maps)
>    (add-hook 'minibuffer-setup-hook 'ido-minibuffer-setup)
>    (add-hook 'choose-completion-string-functions
>       'ido-choose-completion-string))
>
> Although that looks worryingly permanent.

(emacs-version) -> 24.3.1

(describe-variable 'minibuffer-setup-hook)

-> (rfn-eshadow-setup-minibuffer minibuffer-history-isearch-setup minibuffer-history-initialize)

(ido-completing-read "test" '("foo" "bar"))


(describe-variable 'minibuffer-setup-hook)

-> (ido-minibuffer-setup rfn-eshadow-setup-minibuffer minibuffer-history-isearch-setup minibuffer-history-initialize)

SO, although permanent, consistent w/ newer emacs versions...

> d


Tomi

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:
>
> (describe-variable 'minibuffer-setup-hook)
>
> -> (ido-minibuffer-setup rfn-eshadow-setup-minibuffer minibuffer-history-isearch-setup minibuffer-history-initialize)
>
> SO, although permanent, consistent w/ newer emacs versions...
>

Yeah, the other hook is set permanently as well.  So, do these three
lines of code (in advice if you must ;) ) fix the problem?

d

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[Mark Walters]

I don't have anything older than 23.4 anymore so I am afraid I can't
test. My recollection is that I upgraded to 23.4 the problem went away
so I forgot about it. 

(I couldn't see easily what version of emacs debian stable has now as
seemed to be called 45.)

Best wishes

Mark


On Sat, 25 Jan 2014, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> ido-completing-read is used in notmuch-mua.el without any
>> initialization.  This should work but a bug (emacs bug #3247) in some
>> emacs 23 versions (including 23.2.1 from Debian stable) causes emacs
>> to get stuck in the mini-buffer. This tests to see if ido-mode has
>> been initialized and if not calls it. This avoids the bug for these
>> older versions of emacs.
>>
>> This is the only place that ido-completing-read is used so an alternative 
>> would be to replace ido-completing-read by completing-read but cworth 
>> expressed a preference for the ido version in 
>> id:"87wrhfvk6a.fsf@yoom.home.cworth.
>
> This patch was tagged wip, but then there was no progress ;).  It still
> applies cleanly, and doesn't seem to do any harm in emacs 23.4 nor
> emacs24.3. I don't have 23.2 at hand but I guess Tomi or Mark can
> confirm it actually fixes the bug there.
>
> I propose we either apply the patch or ::wontfix the bug.
>
> cheers,
>
> d

Re: [PATCH] emacs: initialize ido mode in notmuch-mua.el

[Tomi Ollila]

On Sun, Jan 26 2014, Mark Walters <markwalters1009@gmail.com> wrote:

> I don't have anything older than 23.4 anymore so I am afraid I can't
> test. My recollection is that I upgraded to 23.4 the problem went away
> so I forgot about it. 

Fine, I prepare and test the patch then ;/

Tomi

> (I couldn't see easily what version of emacs debian stable has now as
> seemed to be called 45.)
>
> Best wishes
>
> Mark

[PATCH 1/1] emacs: initialize ido(-completing-read) in emacs 23.[123]

[Tomi Ollila]

Otherwise `ido-completing-read' will freeze after PROMPT is displayed.
---

I tested this on emacs 23.1 & 24.3.

I also tested this by adding (sit-for 2.0) inside the advice -- this
lead to the addition of (ad-activate 'ido-completing-read (*)).

(*) http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-02/msg00087.html

 emacs/notmuch-mua.el | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 00cd980..481abd7 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -287,6 +287,19 @@ the From: header is already filled in by notmuch."
 
 (defvar notmuch-mua-sender-history nil)
 
+;; Workaround: Running `ido-completing-read' in emacs 23.1, 23.2 and 23.3
+;; without some explicit initialization fill freeze the operation.
+;; Hence, we advice `ido-completing-read' to ensure required initialization
+;; is done.
+(if (and (= emacs-major-version 23) (< emacs-minor-version 4))
+    (defadvice ido-completing-read (before notmuch-ido-mode-init activate)
+      (ido-init-completion-maps)
+      (add-hook 'minibuffer-setup-hook 'ido-minibuffer-setup)
+      (add-hook 'choose-completion-string-functions
+		'ido-choose-completion-string)
+      (ad-disable-advice 'ido-completing-read 'before 'notmuch-ido-mode-init)
+      (ad-activate 'ido-completing-read)))
+
 (defun notmuch-mua-prompt-for-sender ()
   (interactive)
   (let (name addresses one-name-only)
-- 
1.8.0

Re: [PATCH 1/1] emacs: initialize ido(-completing-read) in emacs 23.[123]

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:
> I tested this on emacs 23.1 & 24.3.

I tested it on 23.4 and 24.3.1, as expected it didn't break anything.

d

Re: [PATCH 1/1] emacs: initialize ido(-completing-read) in emacs 23.[123]

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> Otherwise `ido-completing-read' will freeze after PROMPT is displayed.
> ---

pushed.

d