-
Notifications
You must be signed in to change notification settings - Fork 149
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
Add regex name filter #619
base: master
Are you sure you want to change the base?
Conversation
data/config/variety.conf
Outdated
# name_regex_enabled = <True or False> | ||
# name_regex = <regex> | ||
name_regex_enabled = False | ||
name_regex = * |
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.
nit: the default regex should probably be .*
instead of *
, as *
alone isn't a valid regex
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.
Indeed, I updated the defaults to be a valid .*
.
Thanks for spotting it.
Regexes are mostly a programming concept and most users would not be familiar with them. IMHO, when we are talking about filtering by filename, globs are probably a more suitable, popular and easy-to-use concept - https://en.wikipedia.org/wiki/Glob_(programming). In both cases, but especially for regexes, it will be good if the preferences UI has a link to an external documentation on globs/regexes (for globs, a description with 1-2 examples for the use of ? and *, plus a link to read more may be better). Additionally (but it would make the implementation more complex), since this mostly makes sense for local files, it may be better if this is not an option in Filtering, but rather an option that can be specified separately on all "local folder" sources. E.g. I may be interested in all |
I don't have a strong opinion here. If we only focus on matching filenames, regexps are more expressive with little extra cost (Python's glob matching uses regex under the hood anyways), and the simplest usecase with wildcards aren't that much harder to create compared to globs IMO.
I agree on this.
If we go with full path search it'd make more sense to use globs, yeah. Matching directories of arbitrary depth isn't nearly as straightforward with regex. But maybe glob and regex filters don't need to be mutually exclusive more generally 😄 |
Hello,
I implemented a simple name regex check to allow filtering what images are shown.
It is integrated in the preferences dialog under the Filtering category.
This is useful mostly when using a local folder source.
I ran the tests prior to this pull request, all pass except TestNationalGeographicDownloader.TestNationalGeographicDownloader, which is not related to my modifications.
I tested a local build on my own machine (Manjaro Gnome), all works as expected.
Please let me know if I should modify anything for this to be merged in the master branch, I would be happy to help.
Best regards,
Francois