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

Ivy compares :action against the literal symbol 'identity #1632

Closed
raxod502 opened this issue Jun 17, 2018 · 9 comments
Closed

Ivy compares :action against the literal symbol 'identity #1632

raxod502 opened this issue Jun 17, 2018 · 9 comments

Comments

@raxod502
Copy link
Contributor

The following code emulates the operation of find-file with ivy-mode enabled. It works fine (see screenshot below):

(ivy-read "Find file: " 'read-file-name-internal
          :predicate 'file-exists-p
          :require-match 'confirm-after-completion
          :initial-input "~/.emacs.d/init.el"
          :preselect "~/.emacs.d/init.el"
          :def "~/.emacs.d/init.el"
          :history file-name-history
          :keymap nil
          :sort t
          :dynamic-collection nil
          :caller 'read-file-name-internal
          :action 'identity)

image

The following code does the same but substituting the function (lambda (x) x) instead of 'identity. This should make no difference at all, but actually it breaks Ivy:

(ivy-read "Find file: " 'read-file-name-internal
          :predicate 'file-exists-p
          :require-match 'confirm-after-completion
          :initial-input "~/.emacs.d/init.el"
          :preselect "~/.emacs.d/init.el"
          :def "~/.emacs.d/init.el"
          :history file-name-history
          :keymap nil
          :sort t
          :dynamic-collection nil
          :caller 'read-file-name-internal
          :action (lambda (x) x))

image

This is because there is a check against the literal symbol 'identity in ivy--reset-state:

swiper/ivy.el

Lines 1917 to 1918 in 6f29394

(unless (and (ivy-state-action ivy-last)
(not (equal (ivy--get-action ivy-last) 'identity)))

raxod502 added a commit to raxod502/swiper that referenced this issue Jun 17, 2018
This should be replaced with something less gross once
abo-abo#1632 is solved.
raxod502 added a commit to radian-software/radian that referenced this issue Jun 17, 2018
@abo-abo
Copy link
Owner

abo-abo commented Jun 18, 2018

This should make no difference at all, but actually it breaks Ivy:

Your code doesn't break if you fix the missing quote:

- :history file-name-history
+ :history 'file-name-history

This is because there is a check against the literal symbol 'identity in ivy--reset-state:

Checking for 'identity is this is part of the API: the user passes 'identity if they only care about the return result of ivy-read.
Now for find-alternate-file, nil is passed for :action and this should be handled fine as well. Passing (lambda (x) x) isn't the way to have ivy-read not call the action - the action will still be called as expected.

@raxod502
Copy link
Contributor Author

Your code doesn't break if you fix the missing quote:

Actually, it does still break. Exactly the same thing happens with or without that quote.

Checking for 'identity is this is part of the API

It is? Where is that documented?

the user passes 'identity if they only care about the return result of ivy-read.

OK, fine, but why would that affect the operation of :initial-input?

Now for find-alternate-file, nil is passed for :action

Not in my setup. I use ivy-prescient.el, which adds an advice to ivy-read causing the :action to always be non-nil.

Passing (lambda (x) x) isn't the way to have ivy-read not call the action - the action will still be called as expected.

Yes, of course. But I must be misunderstanding something here. Firstly, if the action is (lambda (x) x), then why would it make a difference whether the action is called or not? It can't possibly do anything. And secondly, how could whether or not the action is called affect the operation of :initial-input?

@raxod502
Copy link
Contributor Author

FWIW, the actual problem in my setup is that find-alternate-file is broken. I only factored out the above example for reproducibility.

@abo-abo
Copy link
Owner

abo-abo commented Jun 18, 2018

Actually, it does still break. Exactly the same thing happens with or without that quote.

Can you give me a reproducible scenario, since I can't reproduce with make plain?

@raxod502
Copy link
Contributor Author

I'm using revision 80f05a7 of the swiper repository. I ran make plain, pasted the second example from my original post with the missing quote added, i.e.

(ivy-read "Find file: " 'read-file-name-internal
          :predicate 'file-exists-p
          :require-match 'confirm-after-completion
          :initial-input "~/.emacs.d/init.el"
          :preselect "~/.emacs.d/init.el"
          :def "~/.emacs.d/init.el"
          :history 'file-name-history
          :keymap nil
          :sort t
          :dynamic-collection nil
          :caller 'read-file-name-internal
          :action (lambda (x) x))

pressed C-x C-e, and got exactly the result I showed above.

Do you not have a file called ~/.emacs.d/init.el on your machine? That is the only way I can think that our environments could be different in a relevant way, except perhaps Emacs version (I'm running 26.1).

@abo-abo
Copy link
Owner

abo-abo commented Jun 20, 2018

Do you not have a file called ~/.emacs.d/init.el on your machine?

No.

except perhaps Emacs version (I'm running 26.1).

Using 26.1 on Ubuntu 16. No error is reproduced. Instead, I'm getting the input "~/.emacs.d/init.el" in the minibuffer and ~/.emacs.d/init.el as the only candidate. Selecting it makes C-x C-e return the expanded file name.

@basil-conto
Copy link
Collaborator

FWIW, you can just use user-init-file instead of a hard-coded string.

@raxod502
Copy link
Contributor Author

Even if you create ~/.emacs.d/init.el (or replace the path with one that does exist), you still don't get the behavior I described?

I get the same result with ~/.emacs.d/<some-file-that-doesn't-exist> and ~/<some-folder-that-doesn't-exist>/<some-file>.

Also, I tried the same thing (with ~/.emacs.d/init.el existing) on Arch Linux. Same result.

Also, I found another machine running a newer version of macOS, did a fresh brew install emacs --with-cocoa, cloned just the Swiper repository, and tested again. Same result. There's no ~/.emacs.d/init.el on that machine.

Frankly I've yet to find a situation in which the problem isn't reproduced.

@abo-abo
Copy link
Owner

abo-abo commented May 18, 2020

Thanks, please test.

astoff pushed a commit to astoff/swiper that referenced this issue Jan 1, 2021
…ial-input

* ivy-test.el (ivy-read-file-name-initial-input): Add test

Fixes abo-abo#1632
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

No branches or pull requests

3 participants