GNOME Bugzilla – Bug 722756
Include matching folder names when doing text searches
Last modified: 2021-05-19 14:00:22 UTC
Created attachment 266948 [details] [review] adds keywords from folder path to valid search terms I have my photos organized in directories with names meaningful to me. Now in Shotwell, when I click "Find" and enter some text, I want the results to include pictures in directories which match that text. If the photo is in the library directory, the match should work anywhere along the path from the library root. So if a photo is at /path/to/library/root/Vacations/Summer/Boston/ocean.jpg, the text search should include this image on inputs like "vacation", "summer", or "boston". If the photo is not in the library directory, the match should only work on the image's parent directory.
Thanks! We'll review shortly.
Pushed to master, commit 51f04e
ritchiew, I've had to revert this commit. See bug #723835 for additional details. I attempted to fix the bug when Sebastien first reported it. My theory was that the keyword generation code was too far down the inheritance stack. MediaSource is a generic object and, I thought, also the subclass for the camera objects when you go to import photos. I wondered if the problem was due to your code traversing the *camera's* directory structure, which is obviously bound to be very slow. So, I moved it up the stack to LibraryPhoto, which means only photos imported into the library would generate folder keywords, which seemed appropriate to me. However, as Sebastien reported, my changes didn't fix the problem. In my haste to try and fix the problem I overlooked that MediaSource is not a subclass of ImportSource, so in fact I believe your commit was properly placed in the class hierarchy. However, the hang problem remains. While looking at Sebastien's bug, I realized there were some other problems with the original commit. Since the photo's filepath can change (if the user moves or renames it), LibraryPhoto needs to detect that situation and re-generate keywords. I coded up that work as well. That code needs to be moved to MediaSource for this to land. My changes are on the wip/723835-camera-hang branch. If you'd like to get this in, I need for you to (a) attempt to repro Sebastien's issue in bug #723835 and (b) produce a patch that fixes those problems as well as incorporates the work I did in the wip branch into your changes in MediaSource. I know this is a bit confusing, but the problem is confusing as well. I don't understand how your changes could have introduced this problem, but Sebastien was able to git bisect back to your patch. Thanks!
Hello, If this could help you. Only when there is one or more photo at the root library (for example : /path/to/library/root/ocean.jpg), I have the following three messages indefinitely : (shotwell:13228): GLib-GIO-CRITICAL **: g_file_get_basename: assertion 'G_IS_FILE (file)' failed (shotwell:13228): GLib-GIO-CRITICAL **: g_file_get_parent: assertion 'G_IS_FILE (file)' failed (shotwell:13228): GLib-GIO-CRITICAL **: g_file_equal: assertion 'G_IS_FILE (file1)' failed And there is the backtrace : (gdb) bt
+ Trace 233166
beguam, this appears to be a separate bug. Thanks for pointing it out, that too needs to be addressed.
Thanks, all. I pushed some changes to a branch on github: https://github.com/ritchiewilson/shotwell/tree/keywords What is there fixes beguam's bug. I'm still having trouble understanding or reproducing bug #723835, however this change should sidestep it. Instead of traversing the file system, this now just gets the filepath from the database. So the file system isn't touched. Last is regenerating keywords when files are moved or renamed. Jim, I believe what I have already works. The method update_indexable_keywords() already gets called when files move (in case the basename changes) and any search gets updated automatically. Can you give me an example for when this fails?
ritchiew, thanks for sticking with this. To be honest, I don't believe your original patch had any significant blocking I/O. If you look at File.get_basename and File.get_parent, they are specifically documented as not performing I/O. They are parsing methods, nothing more. The only call that is slightly worrisome is AppDirs.get_import_dir which calls into GSettings, however, GSettings is supposed to do caching and not hit the disk each time. (I admit, that's not a rock-solid assurance.) Still, what no one has really explained is how any of this, including GSettings, would trigger a problem deep inside GVFS. It looks to me that this patch merely replaces the calls to get_parent and get_basename with array-slicing calls. I'd need to verify that these really do solve the camera problem. Also, you're correct, update_indexable_keywords() will get called when the backing file is changed; I missed the call in notify_altered(). In any event, we've passed feature freeze for Shotwell. This is fairly small, and if the branch doesn't repro the reported camera problem, I'll consider it for 0.18.
*** Bug 718943 has been marked as a duplicate of this bug. ***
> I'll consider it for 0.18. It's now 0.22. Was this feature forgotten? It's so essential for anyone migrating from Picasa or using a job/project-based folder organization. Which describes just about all my photographer clients...
Considering for 0.26, and depending on how I get through with the 0.24 bugs one of the candidates to handle in 0.24 if there's time
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/shotwell/-/issues/4451.