Questions about the `completing-read-function' interface

Questions about the `completing-read-function' interface

[Oleh Krehel]

Hi all,

I'd like to know, if a function calls `completing-read', should the
members of DEF be inside COLLECTION? I haven't found info on this in the
doc, and `completing-read-default' doesn't seem to handle DEF not being
in COLLECTION.

But, for instance, `magit-completing-read' (and some other functions) do
make that assumption. And `helm-mode' has code to deal with that.  I'm
thinking of adding a similar work-around to `ivy-mode' as well, but
maybe it would be better if it was somehow mandated that DEF should be
in COLLECTION. Because, theoretically, DEF can be a list, and it would
be inefficient for the completion engine to go through the whole
collection just to make sure that DEF isn't there, and then add it
there.

As second question is about this interface:

    (setq completing-read-function 'foo)

We've accumulated a bunch of packages that call this: icomplete-mode,
helm-mode, icy-mode, ivy-mode. If these functions are called in
succession, each of them will think that they are on, however, the
resource (`completing-read-function') will belong only to the last one.

Maybe we could have this type of interface:

    (set-completing-read-function 'foo callback)

So that each time `set-completing-read-function' is called, the current
minor mode is properly shut down in the callback.  We could keep the
current interface for single-use completion through a let binding. But
all the minor modes mentioned would use `set-completing-read-function'.

Oleh

RE: Questions about the `completing-read-function' interface

[Drew Adams]

> I'd like to know, if a function calls `completing-read', should the
> members of DEF be inside COLLECTION? I haven't found info on this in the
> doc, and `completing-read-default' doesn't seem to handle DEF not being
> in COLLECTION.

Completion can be lax: user input need not match any of the completion
candidates (null REQUIRE-MATCH argument).  Whether DEF is one of the
candidates is generally irrelevant in this case.

And COLLECTION can be a function, returning any string it wants.
It can even return a value that cannot be known at coding time.
What applies to user input pretty much applies to what DEF can be.

So in the general case no, DEF need not be one of the candidates
(or a subset of them, if it is a list).

The calling function (context) knows whether DEF should be one of
the candidates.  And only the calling function.

> But, for instance, `magit-completing-read' (and some other functions) do
> make that assumption.

Then they need to prepare for that assumption.  They have a particular
requirement for the relation between COLLECTION and DEF, and they need
to manage that use case.

> And `helm-mode' has code to deal with that.  I'm thinking of adding a
> similar work-around to `ivy-mode' as well, but maybe it would be better
> if it was somehow mandated that DEF should be in COLLECTION.  Because,
> theoretically, DEF can be a list, and it would be inefficient for the
> completion engine to go through the whole collection just to make sure
> that DEF isn't there, and then add it there.

Please don't.  Any given *caller* of `completing-read' that needs to
rely on DEF being a member (or a subset) of COLLECTION can just DTRT.

> As second question is about this interface:
>     (setq completing-read-function 'foo)
> 
> We've accumulated a bunch of packages that call this: icomplete-mode,
> helm-mode, icy-mode, ivy-mode.

(Not `icy-mode', FWIW.  But it does use `read-file-name-function'.)

> If these functions are called in succession, each of them will think
> that they are on, however, the resource (`completing-read-function')
> will belong only to the last one.
> Maybe we could have this type of interface:
>     (set-completing-read-function 'foo callback)
> 
> So that each time `set-completing-read-function' is called, the current
> minor mode is properly shut down in the callback.  We could keep the
> current interface for single-use completion through a let binding. But
> all the minor modes mentioned would use `set-completing-read-function'.

Again, please do not try to hard-wire any such behavior.  Leave it
up to the various libraries or other callers of `completing-read' to
handle any such possible interactions, if they see the need.

What I hear so far sounds like solutions in search of problems.
Or perhaps a case of the X-Y problem.  Instead of talking solutions,
perhaps you can present the real problem you are looking at - what
is it that you are trying to do?

Re: Questions about the `completing-read-function' interface

[Stefan Monnier]

> I'd like to know, if a function calls `completing-read', should the
> members of DEF be inside COLLECTION? I haven't found info on this in the
> doc, and `completing-read-default' doesn't seem to handle DEF not being
> in COLLECTION.

So far we haven't tried to enforce this and if require-match is not set,
then I don't think we should require it.

> As second question is about this interface:
>     (setq completing-read-function 'foo)
> We've accumulated a bunch of packages that call this: icomplete-mode,
> helm-mode, icy-mode, ivy-mode.

Actually, icomplete-mode does not touch completing-read-function.

> If these functions are called in succession, each of them will think
> that they are on, however, the resource (`completing-read-function')
> will belong only to the last one.

Indeed.

> Maybe we could have this type of interface:
>     (set-completing-read-function 'foo callback)
> So that each time `set-completing-read-function' is called, the current
> minor mode is properly shut down in the callback.

I don't think it's worth the trouble.  `completing-read-function' should
reflect the user's explicit choice, and if the user enables
several of those modes at the same time, she gets what she deserves.

IOW I think it's OK if ivy-mode thinks it's enabled and yet it is not
active because some other mode has overridden completing-read-function.

I agree that there are potential for actual problems if the
completing-read-function is not properly reverted when the modes are
disabled.  For that reason I recommend you use `add-function' and
`remove-function' rather than `setq'.


        Stefan

Re: Questions about the `completing-read-function' interface

[Oleh Krehel]

Drew Adams <drew.adams@oracle.com> writes:

>> I'd like to know, if a function calls `completing-read', should the
>> members of DEF be inside COLLECTION? I haven't found info on this in the
>> doc, and `completing-read-default' doesn't seem to handle DEF not being
>> in COLLECTION.
>
> Completion can be lax: user input need not match any of the completion
> candidates (null REQUIRE-MATCH argument).  Whether DEF is one of the
> candidates is generally irrelevant in this case.
>
> And COLLECTION can be a function, returning any string it wants.
> It can even return a value that cannot be known at coding time.
> What applies to user input pretty much applies to what DEF can be.
>
> So in the general case no, DEF need not be one of the candidates
> (or a subset of them, if it is a list).
>
> The calling function (context) knows whether DEF should be one of
> the candidates.  And only the calling function.

That's kind of what I'm saying. The calling function knows all the
candidates, excepting the REQUIRE-MATCH case, yet it still sends the
candidates in two batches: COLLECTION and DEF.

>> But, for instance, `magit-completing-read' (and some other functions) do
>> make that assumption.
>
> Then they need to prepare for that assumption.  They have a particular
> requirement for the relation between COLLECTION and DEF, and they need
> to manage that use case.
>
>> And `helm-mode' has code to deal with that.  I'm thinking of adding a
>> similar work-around to `ivy-mode' as well, but maybe it would be better
>> if it was somehow mandated that DEF should be in COLLECTION.  Because,
>> theoretically, DEF can be a list, and it would be inefficient for the
>> completion engine to go through the whole collection just to make sure
>> that DEF isn't there, and then add it there.
>
> Please don't.  Any given *caller* of `completing-read' that needs to
> rely on DEF being a member (or a subset) of COLLECTION can just DTRT.

I don't get this statement.

>> So that each time `set-completing-read-function' is called, the current
>> minor mode is properly shut down in the callback.  We could keep the
>> current interface for single-use completion through a let binding. But
>> all the minor modes mentioned would use `set-completing-read-function'.
>
> Again, please do not try to hard-wire any such behavior.  Leave it
> up to the various libraries or other callers of `completing-read' to
> handle any such possible interactions, if they see the need.

I said nothing of hard-wiring. I just described an interface to which
diverse packages can subscribe in order to get a benefit.
If they don't want to do it, it's it's only an opportunity lost for them.

I've seen the more generic packages playing around so many things that
could be happening on the system. Could the system not give some
facilities to manage the contested resources?

Oleh

Re: Questions about the `completing-read-function' interface

[Oleh Krehel]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> I'd like to know, if a function calls `completing-read', should the
>> members of DEF be inside COLLECTION? I haven't found info on this in the
>> doc, and `completing-read-default' doesn't seem to handle DEF not being
>> in COLLECTION.
>
> So far we haven't tried to enforce this and if require-match is not set,
> then I don't think we should require it.

I agree for the case of

    (memq require-match '(nil confirm confirm-after-completion))

What about the other cases?

>> As second question is about this interface:
>>     (setq completing-read-function 'foo)
>> We've accumulated a bunch of packages that call this: icomplete-mode,
>> helm-mode, icy-mode, ivy-mode.
>
> Actually, icomplete-mode does not touch completing-read-function.

Oops, I forgot. It does some weird thing with the hook instead.

>> If these functions are called in succession, each of them will think
>> that they are on, however, the resource (`completing-read-function')
>> will belong only to the last one.
>
> Indeed.
>
>> Maybe we could have this type of interface:
>>     (set-completing-read-function 'foo callback)
>> So that each time `set-completing-read-function' is called, the current
>> minor mode is properly shut down in the callback.
>
> I don't think it's worth the trouble.  `completing-read-function' should
> reflect the user's explicit choice, and if the user enables
> several of those modes at the same time, she gets what she deserves.

I think it could be handled better without any disadvantage:

1. all minor modes subscribe with `set-completing-read-function'
2. user calls M-x mode-1. mode-1 is active and subscribes.
3. user calls M-x mode-2. Emacs sends a message to mode-1 to shut
down. mode-2 is active and subscribes.
4. and so on.

There's no way to end up in a worse state than is now. And the modes
that don't subscribe simply don't get the benefit of knowing that they
don't own the resource any more. Just like it is now.

> IOW I think it's OK if ivy-mode thinks it's enabled and yet it is not
> active because some other mode has overridden completing-read-function.

It's harmless in this case. It just causes an annoyance for the user to
do M-x ivy-mode twice before it's actually active.  There are other
contested resources besides `completing-read-function', like keymaps or
windows. For example, hydra messes with `input-method-function', a
resource that belongs to `key-chord-mode'.

> I agree that there are potential for actual problems if the
> completing-read-function is not properly reverted when the modes are
> disabled.  For that reason I recommend you use `add-function' and
> `remove-function' rather than `setq'.

I don't understand. Can you give an example?

Oleh

RE: Questions about the `completing-read-function' interface

[Drew Adams]

> > The calling function (context) knows whether DEF should be one of
> > the candidates.  And only the calling function.
> 
> That's kind of what I'm saying. The calling function knows all the
> candidates, excepting the REQUIRE-MATCH case, yet it still sends the
> candidates in two batches: COLLECTION and DEF.

DEF entries are not completion candidates.  The completion candidates
are in one batch: COLLECTION.  Suggested defaults are in another batch:
DEF.  The two batches serve two different purposes.

In some particular use case there might be a particular relation
between the two that must hold, but not in general.  In such a use
case, the code need only enforce whatever relation is needed: call
`completing-read' with appropriate COLLECTION and DEF values.

> > Any given *caller* of `completing-read' that needs to rely on
> > DEF being a member (or a subset) of COLLECTION can just DTRT.
> 
> I don't get this statement.

What part of it do you not get?  Instead of trying to impose a relation
between COLLECTION and DEF for all purposes, leave it up to the caller
to call the function with appropriate COLLECTION and DEF arguments.

If, for a given calling context, DEF should be a member (or a subset)
of COLLECTION, then that call should make sure that it is.  That's all.

> I said nothing of hard-wiring. I just described an interface to which
> diverse packages can subscribe in order to get a benefit.  If they
> don't want to do it, it's it's only an opportunity lost for them.

OK, my misunderstanding then.  So you want to add something optional
for code to be able to use, rather than to change `completing-read'
etc.  (What is that something?)

I have no opinion on that, a priori.  I haven't seen the suggested
feature (e.g. code or spec), so it's hard to guess what you are
proposing.  I don't see the description of the interface to which
diverse packages would subscribe.  Maybe you could specify it more?

And I would still suggest that you might want to point to the real
problem you want to solve, before proposing solutions to it.  I say
that because I think that might help you get your point/question
across.  But that's just a suggestion, in case it helps - if not,
please ignore.

> I've seen the more generic packages playing around so many things
> that could be happening on the system. Could the system not give some
> facilities to manage the contested resources?

Sorry, but "playing around so many things", and "manage the contested
resources", are too vague for me to grok what the problem is that
you are looking at solving.  Maybe others can help - I haven't seen
the problem specified clearly yet.  It may be clear to you (and maybe
to other readers), but it is not clear to me.

If my reply isn't helping you, please ignore.  Hopefully someone
else will respond with something that helps more.  FWIW, I agree
with what Stefan wrote (and I think it agrees with what I wrote).

Re: Questions about the `completing-read-function' interface

[Stefan Monnier]

>> So far we haven't tried to enforce this and if require-match is not set,
>> then I don't think we should require it.
> I agree for the case of
>     (memq require-match '(nil confirm confirm-after-completion))
> What about the other cases?

If require is t I think it'd be OK for the completing-read function to
ignore any DEF argument which is not in COLLECTION.

For the same reason, M-p should arguably skip history members which
aren't in COLLECTION (or which are rejected by PRED).

But there are some issues: even if DEF (or history elements) would be
rejected by require-match when you hit RET, it might still be a useful
string to get via M-n (which the user then completes before hitting RET).

> 1. all minor modes subscribe with `set-completing-read-function'
> 2. user calls M-x mode-1. mode-1 is active and subscribes.
> 3. user calls M-x mode-2. Emacs sends a message to mode-1 to shut
> down. mode-2 is active and subscribes.
> 4. and so on.

But this extra complexity only "solves" the very narrow problem you're
facing, while introducing non-trivial issues: what about minor modes
that override completing-read-function but which only apply to some
cases?  There can be several such minor modes active at the same time
without conflict.

> There's no way to end up in a worse state than is now.

Well yes, there is: increase code complexity for little benefit is
a worse state.

>> I agree that there are potential for actual problems if the
>> completing-read-function is not properly reverted when the modes are
>> disabled.  For that reason I recommend you use `add-function' and
>> `remove-function' rather than `setq'.
> I don't understand. Can you give an example?

M-x ivy-mode RET
M-x helm-mode RET
M-x ivy-mode RET

You should now have helm-mode active and working properly, yet with your
current code, helm-mode will be "enabled by inactive".  If you use
add/remove-function this case will be handled correctly.


        Stefan

Re: Questions about the `completing-read-function' interface

[Oleh Krehel]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> So far we haven't tried to enforce this and if require-match is not set,
>>> then I don't think we should require it.
>> I agree for the case of
>>     (memq require-match '(nil confirm confirm-after-completion))
>> What about the other cases?
>
> If require is t I think it'd be OK for the completing-read function to
> ignore any DEF argument which is not in COLLECTION.
>
> For the same reason, M-p should arguably skip history members which
> aren't in COLLECTION (or which are rejected by PRED).
>
> But there are some issues: even if DEF (or history elements) would be
> rejected by require-match when you hit RET, it might still be a useful
> string to get via M-n (which the user then completes before hitting RET).
>
>> 1. all minor modes subscribe with `set-completing-read-function'
>> 2. user calls M-x mode-1. mode-1 is active and subscribes.
>> 3. user calls M-x mode-2. Emacs sends a message to mode-1 to shut
>> down. mode-2 is active and subscribes.
>> 4. and so on.
>
> But this extra complexity only "solves" the very narrow problem you're
> facing, while introducing non-trivial issues: what about minor modes
> that override completing-read-function but which only apply to some
> cases?  There can be several such minor modes active at the same time
> without conflict.
>
>> There's no way to end up in a worse state than is now.
>
> Well yes, there is: increase code complexity for little benefit is
> a worse state.

It's not that much more complex. Currently, we have this behavior for
major modes. When one is activated, Emacs deactivates the second one.

We could get the same feature for themes, or company-mode/auto-complete,
there are plenty of use cases.

>>> I agree that there are potential for actual problems if the
>>> completing-read-function is not properly reverted when the modes are
>>> disabled.  For that reason I recommend you use `add-function' and
>>> `remove-function' rather than `setq'.
>> I don't understand. Can you give an example?
>
> M-x ivy-mode RET
> M-x helm-mode RET
> M-x ivy-mode RET
>
> You should now have helm-mode active and working properly, yet with your
> current code, helm-mode will be "enabled by inactive".  If you use
> add/remove-function this case will be handled correctly.

I meant to give an example of how `add-function' helps here.  Also, with
your sequence of commands, starting from nothing, we have ivy-mode set
to "t" and having the `completing-read-function' resource,
while `helm-mode' is also "t" and may think that it has the resource,
but it doesn't.

I've also seen this type of code: ido-vertical-mode stores the variable
`ido-decorations' of ido-mode and modifies it.  When you turn off
ido-vertical-mode, it restores `ido-decorations'.  But what if other
code has changed `ido-decorations' in the meantime, while
ido-vertical-mode is still on?  Any way you put it, the result is
incorrect. Unless there's a robust system behind it all that manages the
`ido-decorations' variable.

Oleh

Re: Questions about the `completing-read-function' interface

[Stefan Monnier]

>> M-x ivy-mode RET
>> M-x helm-mode RET
>> M-x ivy-mode RET
>> 
>> You should now have helm-mode active and working properly, yet with your
>> current code, helm-mode will be "enabled by inactive".  If you use
>> add/remove-function this case will be handled correctly.
> I meant to give an example of how `add-function' helps here.

That's exactly what the example does, AFAIK.

> Also, with your sequence of commands, starting from nothing, we have
> ivy-mode set to "t" and having the `completing-read-function'

No, the idea of this example is that after the second line ivy-mode is
still t but completing-read-function is now set to use the helm version.
After the 3rd line, with `setq' completing-read-function would be reset
to the default value, whereas with remove-function
completing-read-function would still continue using the helm version.

> I've also seen this type of code: ido-vertical-mode stores the variable
> `ido-decorations' of ido-mode and modifies it.  When you turn off
> ido-vertical-mode, it restores `ido-decorations'.  But what if other
> code has changed `ido-decorations' in the meantime, while
> ido-vertical-mode is still on?  Any way you put it, the result is
> incorrect. Unless there's a robust system behind it all that manages the
> `ido-decorations' variable.

That's exactly what add-function aims to solve.


        Stefan

Re: Questions about the `completing-read-function' interface

[Oleh Krehel]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> M-x ivy-mode RET
>>> M-x helm-mode RET
>>> M-x ivy-mode RET
>>> 
>>> You should now have helm-mode active and working properly, yet with your
>>> current code, helm-mode will be "enabled by inactive".  If you use
>>> add/remove-function this case will be handled correctly.
>> I meant to give an example of how `add-function' helps here.
>
> That's exactly what the example does, AFAIK.
>
>> Also, with your sequence of commands, starting from nothing, we have
>> ivy-mode set to "t" and having the `completing-read-function'
>
> No, the idea of this example is that after the second line ivy-mode is
> still t but completing-read-function is now set to use the helm version.
> After the 3rd line, with `setq' completing-read-function would be reset
> to the default value, whereas with remove-function
> completing-read-function would still continue using the helm version.
>
>> I've also seen this type of code: ido-vertical-mode stores the variable
>> `ido-decorations' of ido-mode and modifies it.  When you turn off
>> ido-vertical-mode, it restores `ido-decorations'.  But what if other
>> code has changed `ido-decorations' in the meantime, while
>> ido-vertical-mode is still on?  Any way you put it, the result is
>> incorrect. Unless there's a robust system behind it all that manages the
>> `ido-decorations' variable.
>
> That's exactly what add-function aims to solve.

You mean to use code like this:

(if ivy-mode
    (add-function :override completing-read-function 'ivy-completing-read)
  (remove-function completing-read-function 'ivy-completing-read))

This can solve the problem if all packages adhere to this rule
simultaneously. I mean that now this sequence works:

- helm-mode
- ivy-mode
- ivy-mode

But this sequence doesn't:

- ivy-mode
- helm-mode
- helm-mode

Since which Emacs version is `add-function` available?
Would you recommend the above approach, possibly in the manual?

Oleh

Re: Questions about the `completing-read-function' interface

[Stefan Monnier]

> You mean to use code like this:
> (if ivy-mode
>     (add-function :override completing-read-function 'ivy-completing-read)
>   (remove-function completing-read-function 'ivy-completing-read))

Yes.

> This can solve the problem if all packages adhere to this rule
> simultaneously.

It works better if all packages use it, indeed, but even if only
ivy-mode uses it, it handles the

    M-x ivy-mode RET
    M-x helm-mode RET
    M-x ivy-mode RET

case better.

> But this sequence doesn't:
> - ivy-mode
> - helm-mode
> - helm-mode

Indeed, it doesn't fix this sequence, but it doesn't make it worse either.

> Since which Emacs version is `add-function` available?

24.4.

> Would you recommend the above approach, possibly in the manual?

Yes, I recommend it for all modifications of `*-function' variables.


        Stefan