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

[Stefan Monnier]

>>> 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...

It helps by showing how to do it.  I haven't seen any proposal here
which helps further than that (except maybe for some proposals which
might break such code, thus inciting them to use a better approach ;-).

>>> 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.

AFAICT you're talking about the let-bindings of `minibuffer-local-*`
whereas the problematic let-bindings are those of
`minibuffer-completion-*` and those are outside of
`read-from-minibuffer`.

> I didn't know that ELC files had to be backward-compatible between major
> releases.

We don't guarantee such compatibility 100%, but we do our best, and it's
pretty easy to avoid the problem here, so turning `read-from-minibuffer`
into a macro would not qualify as "do our best", I think.

> 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.

The [SOMETHINGn] are still outside of `read-from-minibuffer` so the
implementation of `read-from-minibuffer` doesn't affect the
problem, really.

> 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.

No, I'm talking about external packages for example those working like
`icomplete-mode`: they don't change the code which sets
`minibuffer-completion-table`, they just look for the completion table
in `minibuffer-completion-table`.  Currently such packages can access
`minibuffer-completion-table` from any buffer.
With our patches this is not the case any more: they can only access it
from the minibuffer.

So far I haven't found such code, tho, so maybe the risk of breakage is
actually much less severe than I imagined.  In any case, this is risk is
the same for both patches.


        Stefan