prescient.el icon indicating copy to clipboard operation
prescient.el copied to clipboard

ivy-prescient doesn't prioritizes the exact match

Open casouri opened this issue 4 years ago • 10 comments

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

casouri avatar May 31 '20 21:05 casouri

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 avatar Jun 13 '20 12:06 raxod502

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.

raxod502 avatar Jun 13 '20 12:06 raxod502

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.

wyuenho avatar Sep 05 '20 20:09 wyuenho

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

casouri avatar Sep 05 '20 20:09 casouri

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

wyuenho avatar Sep 05 '20 20:09 wyuenho

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 avatar Sep 05 '20 23:09 raxod502

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.

raxod502 avatar Sep 19 '20 15:09 raxod502

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

wyuenho avatar Sep 19 '20 15:09 wyuenho

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.

raxod502 avatar Sep 20 '20 16:09 raxod502

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.

wyuenho avatar Jan 25 '21 17:01 wyuenho