-
-
Notifications
You must be signed in to change notification settings - Fork 79
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 |
now they'll use their natural sorting and only use nucleo for the matching capability
c9b9908
to
6051dea
Compare
da3dfed
to
7ff55dc
Compare
📺 PR Description
This PR add frecency support to already selected entries (opt-in), they will be boosted over the nucleo matcher results
Checklist