-
-
Notifications
You must be signed in to change notification settings - Fork 108
feat: add frecency support #671
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
base: main
Are you sure you want to change the base?
Conversation
|
While this does the job,I think we can implement the feature in a much simpler way (a couple of hundred lines of code) by using the intermediate matcher layer on top of nucleo but I might be mistaken 🤔. Will try to have a more thorough look tomorrow. |
44b70f3 to
2467e7e
Compare
|
In the meantime I've ironed out some performance problems here and there and added some docs as well. Based on some informal benchmarks I don't see any performance degradation in a directory with 80k+ files. That aside; I was thinking that, in a way, the frecency feature is just a special case of sorting. What are your thoughts on instead of pub enum SortMode {
/// Score-based nucleo-matcher strategy
#[default]
Fuzzy,
/// Frecency based (channel specific)
Frecency,
/// Frecency based (global)
FrecencyGlobal,
/// Disable sorting
None,
}And we could expose actions to toggle modes, live, while running the app This could play nicely with @raylu PR #672 disabling sorting |
276929a to
3b87ac5
Compare
5067a20 to
48c941f
Compare
|
cache handling should be significantly better now @alexpasmantier if have some time can you review the latest version 🙏. wdyt? |
|
some benchmarking in a directory with 400k files |
73296df to
cd3b936
Compare
d1f836b to
c9b9908
Compare
|
Sorry I haven't taken any time to review this yet. As I said earlier, I was caught up in the config refactor and wanted to lay that down before doing anything else. A couple of things on this frecency PR though:
I know you've been working quite a lot on this and don't mean to disregard that in any way. We'll come back to this when the time is right. |
|
It looks big by the +- numbers, but in reality it's mostly documentation and tests. In any case, let's proceed with your strategy |
c9b9908 to
6051dea
Compare
da3dfed to
7ff55dc
Compare
now they'll use their natural sorting and only use nucleo for the matching capability
6051dea to
f92c13b
Compare
|
Just for reference, I have a much better frecency scoring system on a fork I made from fzf, original discussion here, it's just a matter of migrating the new scoring part to this PR. The fzf overall implementation is much simpler than the one here because there I had access to the matcher. @alexpasmantier how do you feel about vendoring nucleo? It would made things much easier and then we wouldn't have to deal with the problem of waiting for all items and open the door for any custom sorting alg you would want to implement. |
d1362c0 to
bae2241
Compare
32dc349 to
d2034a8
Compare
📺 PR Description
This PR add frecency support to already selected entries (opt-in), they will be boosted over the nucleo matcher results
Checklist