bug#45474: Icomplete exhibiting in recursive minibuffer when it shouldn’t

[Gregory Heytings]

Thanks for your detailed comments.

> > By 'completing-read-default' _and_ by other completion backends that 
> > set minibuffer-completion-* elements and call read-from-minibuffer. 
> > Or am I misunderstanding something?
>
> AFAICT there are very few other pieces of code which let-bind (or set) 
> minibuffer-completion-*.  And my suggested patch should hopefully not 
> affect them too significantly.  Also I don't think we can fix this 
> problem without introducing corner-case incompatibilities and/or extra 
> code/changes in other frontends or backends.

Well, the fact that there were a few other pieces of code which do that 
was (at least as I understood it) part of the initial problem.  And the 
purpose of the discussion (at least as I understood it) was to solve the 
current problem without breaking these few other pieces of code.

> > If not, it means that your patch will fix the problem for 
> > completing-read-default, but not for other completion backends, who 
> > will have to resort on a similar trick to get the same effect.
>
> I think they'd need to make similar changes to fix the problem under 
> discussion in this longish thread, but they can keep using their old way 
> of working and the consequence will just be that they will keep 
> suffering from the old problem.

With my patch all they have to do is to add (minibuffer-local-completion 
t) before calling read-from-minibuffer.

> > Not with the patch I'm proposing.  What it does is the following (in 
> > abbreviated form):
> >
> > (let ((minibuffer-local-* minibuffer-completion-*))
> >    (let ((minibuffer-completion-* nil))
> >       (internal-read-from-minibuffer ...)))
>
> Not quite.  The actual code is more like:
>
>    (let ((minibuffer-local-* minibuffer-completion-*))
>       [SOMETHING1]
>       (let ((minibuffer-completion-* nil))
>          (internal-read-from-minibuffer ...))
>       [SOMETHING2])
>
> and those two [SOMETHINGn] still leak.

I admit that you lost me here.  What are these [SOMETHINGn]'s, and why are 
they happening?  Wasn't the point to not leak the minibuffer-completion-* 
variables?

> [ Another problem is that this approach breaks when you have 
> simultaneous active minibuffers, and the inner minibuffer is triggered 
> from the outer one (e.g. `C-x C-f` following by `C-h o`) since the 
> let-bindings above will (when run for the `C-h o`) temporarily override 
> the completion information of the outer minibuffer. This is not too 
> serious in the sense that it's no worse than what we have now, tho.  ]

I won't comment on this, because I'm deeply saddened to see this 
happening.  Emacs has had a nice Lisp-like stack of minibuffers forever, 
what the purpose of this new thing is is beyond me.

> You share the main downside of my proposal: `minibuffer-local-*` are now 
> only non-nil buffer-locally in the minibuffers.
>
> I mean it's good because it's what we want in the long term, but it's 
> bad because it's not backward compatible and there's probably some code 
> out there which will need to be adjusted to work with this.

I have to admit again that you lost me here.  What do you mean?  With my 
patch, inside the minibuffer, the minibuffer-local-* variables aren't used 
anymore, the usual minibuffer-completion-* variables are used, and they 
are buffer-local, so the sole and only thing that a piece of code wishing 
to use the new semantics needs to do is to let-bind 
(minibuffer-local-completion t) around the read-from-minibuffer call.