Skip to content

Conversation

@icekom
Copy link

@icekom icekom commented Jun 25, 2018

Filter that allows exact matches with messages in a list.

Can be used to make sure that user replies with one of the custom keyboard buttons.

Dmitriy Grigoryev added 2 commits June 25, 2018 20:48
@jsmnbom
Copy link
Member

jsmnbom commented Jun 26, 2018

Thank you a lot for taking the time to write this code, and submitting this PR! :D
I am, however, unsure of the benefit of adding this filter, for these 2 reasons:

  • Doesn't provide which keyword triggered the filter to the handler function, which means you need to check it in your handler function afterwards
  • Usually one would just use a RegexHandler for this sort of thing. This has the added benefit of actually passing to the handler function which keyword matched.

Any of the devs feel differently?

@icekom
Copy link
Author

icekom commented Jun 27, 2018

The purpose of this filter is different. It doesn't check if a keyword exists in message, but if the message is an exact match with one of the strings in the list.

For example if you have a keyboard markup generated from a list:

buttons = ['Start', 'Settings', 'Back']
markup = ReplyKeyboardMarkup([[x] for x in buttons])
bot.send_message(chat_id, text, reply_markup=markup)

You can use MessageHandler(Filters.in_list(buttons), callback) in one of the states of ConversationHandler to filter out any replies that don't match one of the buttons and direct them to fallback handler. And you can get the string that triggered that filter from update.message.text, as it is an exact match to the one in the list.

@jsmnbom
Copy link
Member

jsmnbom commented Jul 8, 2018

Alright, you're right. I agree that we need much a filter. I think we should simply name it in instead of in_list since it should work on everything that the python keyword in does.

@JosXa
Copy link
Contributor

JosXa commented Jul 8, 2018

@jsmnbom It doesn't specify what attribute of an update you're actually comparing though. Shouldn't it be text_in(), caption_in(), callback_query_in() perhaps?

Copy link
Member

@tsnoam tsnoam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is a great addition.
I have one small nitpicking comment and obviously we'll also need unitests.

About renaming the class to "in" I think that this is a reserved word, and even if it isn't I'm not a fan of using python syntax as class/variables names.

@icekom
Copy link
Author

icekom commented Jul 9, 2018

Yes, in is a reserved keyword. Maybe msg_in?

@jsmnbom
Copy link
Member

jsmnbom commented Jul 9, 2018

It might be a good idea to go with @JosXa 's suggestion then: text_in ?

I'm definitely not a fan of having list in there, since it works with more things than lists (other sequences and sets and stuff too).

@JosXa
Copy link
Contributor

JosXa commented Jul 9, 2018

@icecom-dg I'd like msg_in with parameters to control whether to check message text or caption

@icekom icekom changed the title List filter Add msg_in filter Jul 23, 2018
@icekom
Copy link
Author

icekom commented Jul 23, 2018

Review requested. Probably not the best docstring.

@tsnoam tsnoam closed this in #1570 Oct 26, 2019
tsnoam pushed a commit that referenced this pull request Oct 26, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2020
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🔌 enhancement pr description: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants