Skip to content

Conversation

@Bibo-Joshi
Copy link
Member

As discussed offline, this refactors the Filters.msg_in filter to work as __call__ of Filters.text, thus closing #1625
I encountered two (and a half) obstacles:

  • BaseFilters __call__ is used to perform the update checks, i.e. it executes filter(). To allow for Filters.text({'foo', 'bar'}) anyway, Filters.texts __call__ now checks, if the argument is of type Update. If it's not, it returns an instance of a subclass that works like the former msg_in. An alternative way would be to do something like

    def __call__(self, update=None, iterable=None):
        if update:
            self.filter(update)
        else:
            return _TextIterable(iterable)
    

    However calling Filters.text(iterable={'foo', 'bar'}) seemed neither convenient nor intuitive for me. Also, usually you won't have to call Filter.text(update) manually anyway …

  • To recreate the caption filter functionality, I needed to add the base class Filters.caption that allows only messages that have a caption. Including caption filtering into Filters.text like

    Filters.text({'foo', 'bar'}, caption=True)
    

    didn't seem very natural to me …
    If you feel like Filters.caption is unneeded, we can just remove it. The main usecase for this PR is Filters.text(buttons) anyway …

  • I wasn't very original in naming the subclasses (_TextIterable and _CaptionIterable). If you have a better naming idea, please tell :)

@Bibo-Joshi Bibo-Joshi added this to the 12.3 milestone Nov 20, 2019
@Bibo-Joshi Bibo-Joshi requested a review from Eldinnie November 20, 2019 18:39
@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Nov 20, 2019

About failing tests:

  • Py2: Something about pinning messages. Seems unrelated. will restart.
  • Codecov: 🤷‍♂️

@Bibo-Joshi Bibo-Joshi added the 📋 pending-review work status: pending-review label Nov 23, 2019
@Bibo-Joshi
Copy link
Member Author

@Eldinnie Are you okay with the changes? And do you have any clue, what codecov wants?

@Eldinnie
Copy link
Member

Codecov is telling us that we did not test all branches:
image
It's telling you that the test if it's an update_filter is obsolete, because it never is (at least never tested). That's why the diff_target is not 100%

Anyway I'll allow it for now :D

@Eldinnie Eldinnie merged commit 5e8a961 into master Nov 29, 2019
@Eldinnie Eldinnie deleted the refactor_msg_in branch November 29, 2019 12:09
Bibo-Joshi added a commit that referenced this pull request Feb 6, 2020
tsnoam pushed a commit that referenced this pull request Feb 8, 2020
* Make Filters.command only accept MessageEntitie commands

* Add option to filters.command to allow cmds anywhere in the message

* Make codecov happy, also retroactive for #1631
@Bibo-Joshi Bibo-Joshi mentioned this pull request Apr 2, 2020
18 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📋 pending-review work status: pending-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants