New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preselected value gets added to candidates #1017

Closed
fabacino opened this Issue May 18, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@fabacino
Contributor

fabacino commented May 18, 2017

In ivy--reset-state there is some code which adds the value specified through :preselect to the collection if it is not already in there. I first thought this is a bug but apparently it looks like a feature. Unfortunately I have yet to find out what this is useful for. As the custom input can be something completely different than the candidates, it looks quite weird to me. Not specifing a preselect value for ivy-read is not enough, as the custom input is passed as preselect anyway as soon as ivy-resume is called.

Example:

  1. M-x counsel-apropos RET
  2. Type counsel org goto
  3. C-M-j
  4. C-c C-r (or your equivalent for ivy-resume)
  5. counsel org goto got added to the top of the list and can be selected like a normal candidate despite being custom input. As the action dinstinguishes between custom input and selected candidates it will not do the right thing for the custom entry turned candidate.

The problem with the action can probably be circumvented by adding an additional check whether the input is part of the collection, so no big deal there. But still I think it looks off to have an entry in the list which is completely different than the others., e.g. a regexp among a list of symbols.

I would appreciate if somebody could explain the use cases of this feature to me. Thank you.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented May 19, 2017

I think adding :preselect probably should not happen. completing-read documentation says that :initial-input should be added. Those two are close in purpose though.

@fabacino

This comment has been minimized.

Contributor

fabacino commented May 23, 2017

I would like to suggest that nothing should happen if :preselect is given but its value is not in the collection. This would be a nice thing for consistency reasons, as the output of

ivy-read-non-dynamic

and

ivy-read-dynamic

would become identical which probably makes sense.

@abo-abo abo-abo closed this in 365008f May 23, 2017

@abo-abo

This comment has been minimized.

Owner

abo-abo commented May 23, 2017

I would like to suggest that nothing should happen if :preselect is given but its value is not in the collection

I agree, thanks.

@fabacino

This comment has been minimized.

Contributor

fabacino commented May 23, 2017

Nice, thank you.

@tarsius

This comment has been minimized.

Contributor

tarsius commented May 25, 2017

This does break ivy-completing-read's compatibility with completing-read.

When there is no input, then the latter returns the default even if it is not a member of the completion candidates. However, it does not actually add the default to the list of candidates and therefore won't display it when pressing TAB. Also, when REQUIRE-MATCH is non-nil, then it is not possible to type out the default and press RET to use it; in that case it complains that there is no match.

So I think it is unintentional that for completing-read RET with no input returns the default even if that is not a completion candidate. Whether you want to adjust ivy-completing-read to behave the same way, depends on whether you want to achieve bug-for-bug compatibility, I guess.

Magit will have to be adjusted regardless of what you decide, since here the intention is for the default to be treated as a completion candidate (for all completion frameworks, including the default one).

@abo-abo

This comment has been minimized.

Owner

abo-abo commented May 26, 2017

Magit will have to be adjusted regardless of what you decide, since here the intention is for the default to be treated as a completion candidate (for all completion frameworks, including the default one).

Since this is being done, I'll keep the current behavior in Ivy, since I think it's simpler. I think the weird behavior of completing-read will eventually be phased out. The simplest mathematical phrasing of the completion problem is to pick a string from a set of strings, we should try to stick to that (in place of a set of strings, and possibly one or two additional elements not in the set).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment