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

Issue with sorting in helm-fuzzy-matching-default-sort-fn-1 #2617

Closed
1 task done
knilink opened this issue Sep 14, 2023 · 8 comments
Closed
1 task done

Issue with sorting in helm-fuzzy-matching-default-sort-fn-1 #2617

knilink opened this issue Sep 14, 2023 · 8 comments
Labels

Comments

@knilink
Copy link

knilink commented Sep 14, 2023

What happened?

Not exactly a bug.
I came across a behavior in helm-fuzzy-matching-default-sort-fn-1 that IMO might worth some rethinking in terms of user experience. version helm-20230914.903
Sometimes I make typo when running command and the typo command remains prioritized.
This makes accessing the intended item slightly cumbersome.

Have tried helm-fuzzy-matching-sort-fn-preserve-ties-order and doesn't seem to be making any difference probably because (= scr1 scr2) is too strict with the score algorithm (have to be same matching position and the same length)

I started noticing this issue recently probably within a month or two, so I guess could be related to recent changes.

How to reproduce?

(require 'helm)
(setq helm-pattern "foo")
(helm-fuzzy-matching-default-sort-fn-1 '("bar-foo" "bar-foo" "bar-foo" "foo-(。•́︿•̀。)") nil nil t)
;result: ("foo-(。•́︿•̀。)" "bar-foo" "bar-foo" "bar-foo")

as in the example command foo-(。•́︿•̀。) enters the history by typo and stay prioritized, while the intended one is bar-foo

Helm Version

Melpa or NonGnuElpa

Emacs Version

Emacs-28.1

OS

GNU/Linux

Relevant backtrace (if possible)

No response

Minimal configuration

  • I agree using a minimal configuration
@knilink knilink added the bug label Sep 14, 2023
@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Sep 14, 2023 via email

@knilink knilink closed this as completed Sep 18, 2023
@knilink knilink reopened this Sep 21, 2023
@knilink
Copy link
Author

knilink commented Sep 21, 2023

Hi @thierryvolpiatto
could you enlighten me in what scenario would this sorting be making scenes?

(setq helm-pattern "foo")
(helm-fuzzy-matching-default-sort-fn-1 '("bar-foo" "bar-foo" "bar-foo" "funky-0strich-0ctopus-danceathon!") nil nil 1)
("funky-0strich-0ctopus-danceathon!" "bar-foo" "bar-foo" "bar-foo")

many packages like magit has a prefix for all its commands, with current sorting, have to waste extra typing to filter out the intended command.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Sep 21, 2023 via email

@knilink
Copy link
Author

knilink commented Sep 28, 2023

yah, it's expected that bar-foo should have a higher score but it didn't seem to be the case and (helm-fuzzy-flex-style-score "bar-foo" "foo") would actually return 0.

did a bit further investigation and found that (helm-completion--flex-transform-pattern (list "fob")) actually getting ("f" any "o" any "o" any) instead of (prefix "f" any "o" any "b" any point) as the code comment described. Thus the pattern and candidate have to match their initial letter to get score.

helm/helm-core.el

Lines 4526 to 4532 in 31d2dfe

(defun helm-completion--flex-transform-pattern (pattern)
;; "fob" => '(prefix "f" any "o" any "b" any point)
(cl-loop for p in pattern
if (stringp p) nconc
(cl-loop for str across p
nconc (list (string str) 'any))
else nconc (list p)))

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Sep 28, 2023 via email

@knilink
Copy link
Author

knilink commented Jul 21, 2024

closing, as i guess the way how it should be sorted is more a personal thing than being a bug as long as user can customize it

i ended up adding the following snippet to my script to override the default scoring function which drop the "start of string" in the pattern.

(defun my-helm-fuzzy-flex-style-score (candidate pattern)
`helm-flex--style-score'."
  (let ((regexp (helm-aif (gethash pattern helm--fuzzy-flex-regexp-cache)
                    it
                  (clrhash helm--fuzzy-flex-regexp-cache)
                  (puthash pattern (replace-regexp-in-string "\\`\\\\`" "" (helm--fuzzy-flex-pattern-to-regexp pattern))
                           helm--fuzzy-flex-regexp-cache))))
    (helm-flex--style-score candidate regexp t)))

(setq helm-fuzzy-default-score-fn 'my-helm-fuzzy-flex-style-score)

@knilink knilink closed this as completed Jul 21, 2024
@knilink
Copy link
Author

knilink commented Jul 29, 2024

To anyone interested, I found flx provides much better fuzzy scoring.
Below is how i configure it to override the default helm fuzzy score

(declare-function flx-score "ext:flx")
(declare-function flx-make-string-cache "ext:flx")
(defvar my--flx-cache (flx-make-string-cache))

(defun my-flx-helm-fuzzy-flex-style-score (candidate pattern)
  (car (flx-score candidate pattern my--flx-cache)))

(setq helm-fuzzy-default-score-fn 'my-flx-helm-fuzzy-flex-style-score)

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Jul 29, 2024 via email

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

No branches or pull requests

2 participants