Skip to content
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

bugfix: ivy does not support symbolic require-match values to completing-read #2367

Closed
wants to merge 1 commit into from
Closed

bugfix: ivy does not support symbolic require-match values to completing-read #2367

wants to merge 1 commit into from

Conversation

equwal
Copy link
Contributor

@equwal equwal commented Dec 15, 2019

This bug affects sly, which uses 'require-match as the value to the argument named require-match instead of a boolean argument (which is perfectly legal!) to completing-read-function.

joaotavora/sly#296

Here is the relevant portion of the docstring from completing-read

REQUIRE-MATCH can take the following values:
- t means that the user is not allowed to exit unless the input is (or
  completes to) an element of COLLECTION or is null.
- nil means that the user can exit with any input.
- `confirm' means that the user can exit with anyt input, but she needs
  to confirm her choice if the input is not an element of COLLECTION.
- `confirm-after-completion' means that the user can exit with any
  input, but she needs to confirm her choice if she called
  `minibuffer-complete' right before `minibuffer-complete-and-exit'
  and the input is not an element of COLLECTION.
- anything else behaves like t except that typing RET does not exit if it
  does non-null completion.

The fourth one is the expected behaviour.

Do not cast them into boolean T values, since they change behaviour.
@abo-abo
Copy link
Owner

abo-abo commented Dec 17, 2019

Thanks.

astoff pushed a commit to astoff/swiper that referenced this pull request Jan 1, 2021
Do not cast them into boolean T values, since they change behaviour.

Fixes abo-abo#2367
Copy link
Collaborator

@basil-conto basil-conto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a misunderstanding here. In Elisp, and returns either nil or its final argument, without casting non-nil values to t. That means that, when both collection and require-match are non-nil, (and collection require-match) evaluates to the same value as require-match.

So, I'm really curious how this PR could affect anything in either Ivy or SLY.

@basil-conto
Copy link
Collaborator

So, I'm really curious how this PR could affect anything in either Ivy or SLY.

In the meantime, I've reverted the logic change in commit 9180550.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants