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-prescient doesn't prioritizes the exact match #62

Open
casouri opened this issue May 31, 2020 · 10 comments
Open

ivy-prescient doesn't prioritizes the exact match #62

casouri opened this issue May 31, 2020 · 10 comments
Labels

Comments

@casouri
Copy link

casouri commented May 31, 2020

It seems that prescient doesn't prioritize the exact match as vanilla ivy. I enabled both ivy-prescient-mode and prescient-persist-mode. When I type C-h f re-search-forward, vanilla ivy selects the exact match, but prescient doesn't. Is this expected?

Ivy (selectiong the second candidate):

Screen Shot 2020-05-31 at 5 49 33 PM

Prescient (selecting the first candidate):
Screen Shot 2020-05-31 at 5 50 02 PM

@raxod502
Copy link
Member

No, that seems like a bug. I suspect it is due to another internal change in Ivy. The best solution is to switch to Selectrum, which does not have any of these issues, but perhaps somebody will contribute a pull request to help ivy-prescient.el work around the problem.

@raxod502
Copy link
Member

I would, by the way, be happy to add somebody as a maintainer for ivy-prescient.el, as I do not feel like I can maintain that particular component well anymore.

@wyuenho
Copy link

wyuenho commented Sep 5, 2020

It's not just ivy, company-prescient has this problem as well. It's entirely possible to prioritize exact match within prescient 's sorting function rather than asking people to switch to Selectrum. Selectrum is nice, but there are still too many subtle idoism such as backspace kills a path segment and C-k deletes a buffer that Selectrum just can't replace yet.

@casouri
Copy link
Author

casouri commented Sep 5, 2020

The author already said he can't maintain ivy-prescient.el anymore.

@wyuenho
Copy link

wyuenho commented Sep 5, 2020

The author didn't say he can't maintain prescient

@raxod502
Copy link
Member

raxod502 commented Sep 5, 2020

If indeed this bug also exists in company-prescient.el, then I will take a look and see if I can fix it there. Thanks for the report. I will not test anything related to ivy-prescient.el, however.

@raxod502
Copy link
Member

Actually, I take it back. This is not a bug, because regular Company does not prioritize exact matches. The reason you see the behavior in Ivy and Selectrum is because those frameworks implement it specifically, irrespective of the sorting function in use. I would be fine with a pull request that adds the option to prioritize the exact match directly in prescient.el, for people who use prescient.el with a framework that doesn't already do it.

@wyuenho
Copy link

wyuenho commented Sep 19, 2020

Do you have a rough idea what needs to be changed in prescient?

@raxod502
Copy link
Member

You'd want to start by adding a new user option to toggle the behavior. You'd probably want to port something like

(defun selectrum--move-to-front-destructive (elt lst)
  "Move all instances of ELT to front of LST, if present.
Make comparisons using `equal'. Modify the input list
destructively and return the modified list."
  (let* ((elts nil)
         ;; All problems in computer science are solved by an
         ;; additional layer of indirection.
         (lst (cons (make-symbol "dummy") lst))
         (link lst))
    (while (cdr link)
      (if (equal elt (cadr link))
          (progn
            (push (cadr link) elts)
            (setcdr link (cddr link)))
        (setq link (cdr link))))
    (nconc (nreverse elts) (cdr lst))))

which is what Selectrum uses to implement the same functionality. Because of the performance optimization in prescient--sort-compare and related functions, I'd suggest inlining the check into prescient-sort-compare and prescient-sort separately. You would also want to run some benchmarks to make sure that you didn't hurt performance too much when the user option is enabled, and not at all when it's disabled.

It might be simpler to implement this at the Company level, to be honest, or perhaps just put it in company-prescient.el.

@wyuenho
Copy link

wyuenho commented Jan 25, 2021

Turns out company already has a company-sort-prefer-same-case-prefix transformer that will do show exact match at the top lol. I just need to make sure it's always the last one in the list.

@raxod502 raxod502 added the ivy label Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants