Review of "GNOME Fuzzy App Search" version 4

Details Page Preview

Fuzzy application search results for Gnome Search

Extension Homepage
https://gitlab.com/czarlie/gnome-fuzzy-app-search

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

Version Status
25 Active
24 Active
23 Active
22 Rejected
21 Rejected
20 Active
19 Active
18 Active
17 Inactive
16 Active
15 Active
14 Inactive
13 Active
12 Active
11 Active
10 Active
9 Rejected
8 Rejected
7 Rejected
6 Rejected
5 Rejected
4 Rejected
3 Active
2 Active
1 Active

Previous Reviews on this Version

charlotte posted a review
I have made a little mistake, I think? I should ensure that add_to_index.py actually has execute permissions. Sorry
charlotte posted a review
Can't figure out how to retract my review request, so please reject this
andyholmes waiting for author
I must have missed this when the extension was first submitted, but is there a good reason you aren't just using `Gio.AppInfoMonitor` to monitor application changes and the `Gio.AppInfo.get_all()` utility to list applications? I'm also a bit confused as to why you are using a python subprocess to do non-blocking work (which could be done in GJS), while doing blocking I/O work in the extension? Unless I'm missing something, it looks like you are re-creating a lot of functionality already implemented in GNOME's APIs. Could you clarify this for me?
charlotte posted a review
Honestly, I don't know either why this uses Gio.AppInfo.get_all (I forked this from https://github.com/fffilo/gnome-fuzzy-search ). I originally skimmed that code and thought “there's probably an easier way to do this” and then didn't bother changing it because that wasn't my top priority. As to why I'm using a python subprocess, I have not found documentation about doing non-blocking work within GJS (though to be fair, that is probably lack of knowledge on my part; I did try using https://gjs-docs.gnome.org/gio20~2.66p/gio.task#method-run_in_thread , but it still blocked everything else). This was the easiest way I could find to create an index of all apps without blocking gnome shell for, like, minutes at a time. I'll definitely try to incorporate Gio.AppInfoMonitor into whatever the author maintaining the predecessor of this extension did, but could you point me in a direction where I'd find how to do non-blocking tasks within GJS? I've only found https://gjs.guide/guides/gio/subprocesses.html and it does this via shell commands (and since i can't be bothered to write those, I chose python instead)
charlotte posted a review
Sorry, I misread (which goes to show how little I'd know of the top of my head how little I actually cared about that portion of code at the time) of course I am NOT using Gio.AppInfo.get_all and Gio.AppInfoMonitor even though i absolutely should.
andyholmes waiting for author
Unfortunately you can't create custom GTask-based operation in GJS because JavaScript can't deal with untyped pointer (eg `void *`) and you can't run JavaScript code in more than one thread. Almost all blocking functions do already have asynchronous variants though, and I can put up a general guide for asynchronous work in GJS shortly. I've just merged a guide for working with GFile in a non-blocking fashion. You can find that here: https://gjs.guide/guides/gio/file-operations.html
charlotte posted a review
Well, the thing is, the file reads and writes themselves aren't a huge deal, but I need to do a few thousand of them to first initialise the index. File operations do take up most of the time that needs, but of the 70 seconds it took on my machine, about three seconds are other, non-file operations (I actually originally implemented that in JS — it took longer than in Python, and I already do not want to block Gnome shell for three whole seconds, even if not at once). So the rationale for putting this in a separate process still (kinda) makes sense. I'll keep it this way for now, if that's fine with you (though I will remove all uses of Gio.File from the Python code and opt for pure-Python file handling instead). As for Gio.AppInfo.get_all and Gio.AppInfoMonitor (as well as switching to async file operations for the stuff I still do in JS), I'll get to that next week. I have a semi-important exam on Monday that I should definitely study for instead.
andyholmes posted a review
If you're intent on offloading to the Python script you've written, I'm afraid it will have to wait for a reviewer experienced in Python. This is part of the reason we strongly discourage offloading to subprocesses in other programming languages, unless absolutely necessary. I simply can't follow a lot of what the script does or why it does (eg. double hash AppInfo keys, creating many tiny JSON files instead of a single large db). I'm sure there is a reason for this, but it will require someone with more experience here to review. For what it's worth, switching to pure python file handling may make that harder.
charlotte posted a review
Well, to be quite honest, the Python file isn't exactly up to any high standards either, and I should definitely fix some major issues both in style and general structure, as well as both in Python and JS. That, however, is going to be a bit of work; I'll have to do it when I have more spare time next week. I hope you aren't too mad that I kinda wasted your time with a product so unfinished.
andyholmes rejected
It's no problem. It's simply that because of the reputation GNOME Shell extensions have in the community, we have had to become more strict during review time. I will mark this one as rejected, so it doesn't sit it in the review queue. Please feel free to drop by the extensions channel on Matrix or ask questions on Discourse; there's a lot of experienced extension developers now to take advantage of :) * https://matrix.to/#/#extensions:gnome.org * https://discourse.gnome.org/tag/extensions