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

[Gregory Heytings]

> > 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.
>
> Indeed, and I think my patch should by and large leave them unaffected 
> (it neither magically fixes them nor breaks them).

Indeed, but... it doesn't help/incite them to move forward in the "right" 
direction, to finally have what has been on wishlist for quite a long 
time: to have buffer-local minibuffer-completion-* elements...

> > I admit that you lost me here.  What are these [SOMETHINGn]'s, and why 
> > are they happening?
>
> Because inevitably code can run in there, e.g. because the debugger gets 
> triggered in there or because the caller of `read-from-minibuffer` was 
> not careful to place the let-bindings of `minibuffer-completion-*` as 
> close as possible to `read-from-minibuffer`.

I see.  But when the let-bindings are in a macro the caller doesn't have 
to care with them, and they are indeed happening as close as possible to 
internal-read-from-minibuffer.  Unless they deliberately use 
internal-read-from-minibuffer and do some weird things, of course.

> [ Side note: you can't define `read-from-minibuffer` as a macro because 
> it is not compatible with the byte-code generated from code which called 
> `read-from-minibuffer` as a function.  So your patch would need to be 
> adjusted to keep `read-from-minibuffer` as a function. That opens up yet 
> more opportuninities for those [SOMETHINGn], such as advice placed on 
> `read-from-minibuffer`, ...  ]

I didn't know that ELC files had to be backward-compatible between major 
releases.  That being said, my preference would be to have the whole 
dancing happening at the C level (inside read_from_minibuffer and/or 
read_minibuf), which would solve that problem/these problems.

> > > 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.
>
> No wonder: I wrote `minibuffer-local-*` when I meant 
> `minibuffer-completion-*`. Sorry 'bout that.

No worries ;-)  Now I see what you mean, and I do not see where you see a 
potential problem there: whether the minibuffer-completion-* elements 
become buffer-local depends on the minibuffer-local-completion variable. 
When it is nil (the default), they do not become buffer-local, and the 
behavior of read-from-minibuffer is the same as earlier.  This gives 
external package plenty of time to adapt their code to the future 
minibuffer-local-completion = t situation.