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-update-candidates does not work in :action #2775

Open
nbfalcon opened this issue Dec 29, 2020 · 6 comments
Open

ivy-update-candidates does not work in :action #2775

nbfalcon opened this issue Dec 29, 2020 · 6 comments

Comments

@nbfalcon
Copy link
Contributor

This issue came up when I was trying to implement a counsel-kill-buffer command. Calling the action using ivy-call should remove it from the buffer list, since killing a buffer twice is impossible. However, for some strange reason, the old candidates were restored after my action was called. To reproduce, evaluate the following in a *scratch* buffer:

(ivy-read "> " '("1" "2" "q'")
          :action (lambda (a)
                    (message "M")
                    (ivy-update-candidates '("q" "5")))
          :require-match t
          :caller 'q)

Then call ivy-call. Afterwards, the candidate list should be "q" "5", but it was still "1 2 3".

Debugging this further (C-u C-M-x ivy-call), I noticed that the candidates were indeed updated after my action was called and stayed that way until the minibuffer was selected again, after which ivy--exhibit probably reset them again. Adding (setf (ivy-state-collection ivy-last) cands) at the start of ivy-update-candidates didn't help, either.

Even though internal-complete-buffer is handled specially in ivy--update-minibuffer, this bug also shows up if the former is used as CANDIDATES.

I suspect that somewhere around ivy--exhibit the list of candidates is saved and restored somehow, so that ivy--all-candidates and (ivy-state-collection ivy-last) get out of sync.

@basil-conto
Copy link
Collaborator

Would doing something like what ivy--kill-buffer-action and counsel-yank-pop-action-remove do work in your case?

The crux being that, like in #2715, ivy-update-candidates seems very alpha quality. :)

@nbfalcon
Copy link
Contributor Author

Thank you, those functions (especially ivy--kill-current-candidate) are adequate for my use case. Should I close this issue? My problem was solved, but ivy-update-candidates should perhaps work in :action at some point.

@basil-conto
Copy link
Collaborator

Should I close this issue?

It's up to you.

My problem was solved, but ivy-update-candidates should perhaps work in :action at some point.

That depends on what ivy-update-candidates is intended to be used for. As I mention in #2715, it's a completely undocumented function that seems to have been spun up for some ad-hoc use case. Without further work on or documentation for it, I have no idea whom or what it's for. As far as I'm concerned, it's therefore some weird internal function that may or may not be changed in the future. But that's just me. ;)

@zbelial
Copy link

zbelial commented Feb 15, 2021

Hi @nbfalcon @basil-conto
I'm trying to implement ivy support in consult (the issue is here), and I had the same problem.
I'm not familiar with ivy's internal, but it seems that the problem is in ivy--filter, where it checks whether to use cache or not, as following

    (if (and
         ivy--old-re
         ivy--old-cands
         (equal re ivy--old-re))
        ;; quick caching for "C-n", "C-p" etc.
        ivy--old-cands

As you can see, if cache is usable, ivy--filter will just return ivy--old-cands, which is not synced with ivy--all-candidates set in ivy-update-candidates.

@nbfalcon If you make a small change to you code as follows, you'll get the expected result.

(ivy-read "> " '("1" "2" "q'")
          :action (lambda (a)
                    (message "M")
                    (setq ivy--old-cands nil)  ;; this makes ivy--filter not return cached candidates
                    (ivy-update-candidates '("q" "5")))
          :require-match t
          :caller 'q)

So, IMHO in ivy-update-candidates, we can do something to make ivy--old-cands invalid.

@basil-conto
Copy link
Collaborator

So, IMHO in ivy-update-candidates, we can do something to make ivy--old-cands invalid.

Thanks, sounds reasonable. But then like I said I'm not familiar with the intention/design behind ivy-update-candidates :).

@zbelial
Copy link

zbelial commented Feb 15, 2021

Thanks, sounds reasonable. But then like I said I'm not familiar with the intention/design behind ivy-update-candidates :).
I see. But like you said, it seems that it's for some ad-hoc use case(it's only used in counsel-search), so maybe this is a good chance to make it better. :)

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