-
Notifications
You must be signed in to change notification settings - Fork 6k
Check caption in Filters.regex #2163
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
Conversation
Added regex matching for message caption in Filters.regex.
|
Hi. Thanks for the PR! I do see the use case for regexing a caption, but the change you propose would be breaking. I.e. My first idea would be to move it to a standalone Maybe @Poolitzer has an opinion on that? In any case, we'd need unittests (see Codecov failure) - maybe have a look at |
Nope that would be fine for me. We could mention it in Filters.photo doc but we dont do with text either so I think that would be the better solution. |
The same as for regex, with only the content of the message changed, that is now inside caption.
Lines too long and an additional blank line
Bibo-Joshi
left a comment
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.
Thanks for the update! I take it that the tests are basically copy-pasted from Filters.regex, so those should be fine. Two points missing from what I see:
- docstring for the new filter (you can keep it shorter than the one of
Filters.regex, just say "works the same" and link toFilters.regex) - please make sure that pre-commit is happy. have a look at the contribution guide for how to set that up locally.
|
Of course, next step is docstring! I had some trouble configuring the pre-commit on my machine: it did succeed locally, yet the result was different. Now it should be fine (hopefully). |
Maybe merge from master. We recently changed some of the config … |
|
Now everything should be fine, however merging from master made test-official fail, despite me not having touched anything that should trigger it. Let me know what you think! |
|
Yeah, TG just released API 5.0 today, so test official will be failing until we incorporate the changes … nothing to worry about. Looks good, I'll merg :) Thanks for the contribution! |
Added regex matching for message caption in Filters.regex.
I noticed that the regex filter only checks inside the message text, so if it's an image or a document it will be ignored, even if inside the caption the content could match.
Since we can already differentiate text messages and other contents using filters, I think the regex filter should work regardless of the type of message.