-
-
Notifications
You must be signed in to change notification settings - Fork 77
Add entry priority for applications plugin #261
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: master
Are you sure you want to change the base?
Add entry priority for applications plugin #261
Conversation
Kirottu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nitpicks, but otherwise looks alright as a feature.
plugins/applications/src/lib.rs
Outdated
| max_entries: 5, | ||
| preprocess_exec_script: None, | ||
| terminal: None, | ||
| entry_priority: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the default behavior resolves to ActionsFirst, it should be reflected here. That should also be returned by a separate function Config::default_entry_priority or by a sufficient implementation of Default on EntryPriority.
460a293 to
3494622
Compare
|
Alright the code looks good now, but at least in my testing the setting didn't actually have any effect (with both the Zen browser and Firefox the main entry appeared first regardless of the priority setting). I think the weights need to be adjusted so that it works like expected. |
|
Sorry for late reply 😢
I have adjusted weights so that in I'm not certain whether should we apply this in the general scoring logic (like below), or only in the let mut score = (name_score * 10 + desc_score * (if entry.is_action { 5 } else { 1 }) + keyword_score) - entry.offset;For now, to maintain original scoring logic, and not to surprise users, I only applied this to the Thanks! |
Kirottu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it does certainly do something now, it seems like there's still some weighting issues somewhere since if I look for "zen" on my machine with desktop actions on and with the applications first priority, it will correctly show Zen as the first entry, but proceed to show Nextcloud and LibreOffice Calc after it before the desktop actions.
Also sorry for the delay, I've been busy with studies and this PR slipped out of my mind.
To fix your case, it seems that boosting description for action should be applied to both priorities. I have applied a5a916c and tested with the |
Summary
Adds new configuration option to sort the entries to the
applicationsplugin.Motivation
#257
Screenshot
With

ActionsFirst:With

ApplicationsFirst: