-
Notifications
You must be signed in to change notification settings - Fork 52
desktop_entries: Reload .desktop on search #269
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?
Conversation
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.
It'd be better to reload only on an empty search query
387a8b8
to
debc2f7
Compare
Since pop-launcher has been demonized, the desktop_entries plugin isn't relaunched on every search anymore. In consequence, it is not providing up-to-date results when .desktop files have changed (e.g., on un-/installation of a new application) anymore. Move the app.reload() call into the request loop to trigger a re-index. In a release-build this has been fine performance wise on an 8th Gen i5 laptop device. As a small performance improvement, only trigger this update on a search event and if the search query is empty. Alternatives could include remembering the last time the index was built (initialised to UNIX_EPOCH before the loop) and only conditionally triggering a re-index after a hot-period of 10s since the last run. In our testing this didn't result in any noticeable difference.
debc2f7
to
cfaa866
Compare
Implemented the requested change + rebased ontop of The new behavior seems that the
But it seems that the If I understand this correctly, then this PR can be closed? |
iirc, a plugin will only be "demonized" when they have long_lived in their manifest (ex: cosmic_toplevel, https://github.com/pop-os/launcher/blob/master/plugins/src/cosmic_toplevel/plugin.ron#L7). |
You could also add a watcher on the .desktop dirs. Idk if its better performance wise |
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.
This entirely breaks .desktop
entry searching (none of them show up in the results) on both 22.04 and 24.04.
Since pop-launcher has been demonized, the desktop_entries plugin isn't relaunched on every search anymore. In consequence, it is not providing up-to-date results when .desktop files have changed (e.g., on un-/installation of a new application) anymore.
Move the app.reload() call into the request loop to trigger a re-index. In a release-build this has been fine performance wise on an 8th Gen i5 laptop device.
Alternatives could include remembering the last time the index was built (initialised to UNIX_EPOCH before the loop) and only conditionally triggering a re-index after a hot-period of 10s since the last run. In our testing this didn't result in any noticeable difference.
NOTE: This has been tested against commit fca3b25 but builds against master too. However, master error'ed with unrelated issues regarding the parsing of CLI arguments.