Review of "Recent Items" version 28

Details Page Preview

Adds an icon for recently used items at the top panel (for now, there are no updates to extensions.gnome.org. see https://github.com/bananenfisch/RecentItems for details. to get the latest (working) version, you need to install it manually (until there is no better solution))

Extension Homepage
https://github.com/bananenfisch/RecentItems

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Previous Reviews on this Version

bananenfisch posted a review
Hm, i never discussed this on matrix (didn't use matrix in general) :p Ok, then i can not update (i dont want to use a ugly solution for myself or others)... if other users want to use it, they need to install it manually in the future. thx
JustPerfection posted a review
It should be another extension monitoring `~/.local/share/recently-used.xbel` then. In case you want to read the discussion about the recent manager on Matrix: https://matrix.to/#/!hCGqHbXRuMrvWnudnP:gnome.org/$rCHx14A8MNfpyyHNVziBbXB3C6PVZAKirn_kFpjxgms?via=gnome.org&via=matrix.org&via=envs.net
JustPerfection rejected
You cannot import `Gtk` to the GNOME Shell process (line 12 `extension.js`): [EGO Review Guidelines: import](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#do-not-import-gtk-libraries-in-gnome-shell)
bananenfisch posted a review
This extension works with the GTK RecentManager - so to meet the guidelines, it would work with a custom implemented RecentManager. Cons: * it's a pain to sync from recent file, while a RecentManager is already available in GTK * it's a bad memory management to implement a new object for all the recent items, while this object already exists in the memory - and get it via the static method get_default: https://docs.gtk.org/gtk4/type_func.RecentManager.get_default.html IMO it's way better the use the RecentManager from GTK - i will not implement and use an ugly solution. So if this guideline rule affects this extension, i will not upload anymore. thx
JustPerfection posted a review
Sorry for the inconvenience. As we discussed before on Matrix channel, we cannot allow Gtk in the shell process.
bananenfisch posted a review
Thanks for the matrix discussion. Well yes, i understand to have clear and simple rules in the guidelines. For me, there are 2 pro and 1 cons (why i would always prefer the GTK.RecentManager over a custom wrapper): - From the docs: "GtkRecentManager objects are expensive: be sure to create them only when needed. You should use gtk_recent_manager_get_default() instead." the extension uses this static fabric get_default... but a custom wrapper over recently-used.xbel (maybe a large file) would double the memory usage over the already loaded GTK recentmanager itemlist. - To reimplement the same breaks with the KISS principle. vs - from the chat: 'but "don't use gtk" makes for an easier rule than "only use bits of gtk that work without an open display connection"' I respect the guidelines, so this is just my point... and i don't want to start a discussion about it. Anyway, if the GTK import will be accepted just for /this/ usecase, because it's really the better option, then it's up to you ;-) If not, it's also ok for me - i can use my extension with the gtk import anyway... if some users want it on the extension placed here, i can remigrate the custom recentmanager from blankparticle later (i will never use it for myself though ;))... Thanks for clarification!
JustPerfection posted a review
`extension.js` is in the shell process. IIRC it was like 2 years ago, the shell process started the clean up to stay away from the Gtk. 45 was the time we strictly made it a rule. When you import gtk to the shell process, you are not only importing `RecentManager`. The whole Gtk library and its dependencies getting loaded into the shell process. In some cases it can even cause segfault and crash the GNOME Shell process (you can search for `gtk process` in the Matrix room to find out more). Anyway, you can create a gjs app with recent manager and ship it with the extension package since it is running in a separate process. From your extension, that app can be called with a spawn command (async). For recent manager `changed` signal, you can use timeout and spawn command instead. Or you can start that app as a service (run app on enable and kill on disable) and write the changes in a file and your extension can monitor that file. That may be inconvenient but that's the only solution I can give since you don't want monitoring the `.xbel` file.