Skip to content
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

Bot won't match in middle of text, re.match() vs re.search() #289

Closed
benbart opened this issue Feb 24, 2022 · 3 comments
Closed

Bot won't match in middle of text, re.match() vs re.search() #289

benbart opened this issue Feb 24, 2022 · 3 comments

Comments

@benbart
Copy link
Contributor

benbart commented Feb 24, 2022

I've recently updated my bot to 2.0.2 and noticed a change in behavior that broke several functions.

Given the following plugin:

class ThisPlugin(Plugin):
    @listen_to(r'foo')
    async def foo(self, message: Message):
        self.driver.reply_to(message, 'foo found')

    @listen_to(r'.*bar')
    async def bar(self, message: Message):
        self.driver.reply_to(message, 'bar found')

If a mattermost user enters the text:

Some text foo. Some text bar. Some more text baz.

I would expect the bot to invoke both foo() and bar(), but will only respond with "bar found". So I dug around. I see that mmpy_bot uses re.match(), which only matches from the beginning of a string, rather than re.search(). mmpy_bot 1.x used re.search().

I'm not completely expert at regex, but preceding all my patterns with a .* seems hacky. Is it possible to have a mode where re.search() is used instead? Or refactor so the matching can be more easily addressed in a subclass? Or I'd be happy to make a PR to change to search() if the devs aren't opposed to that change.

@ttuffin
Copy link
Collaborator

ttuffin commented Feb 24, 2022

Thanks for reporting this. I don't see any issue switching to re.search, works fine in my tests. @attzonko, do you have any thoughts on this?

@benbart
Copy link
Contributor Author

benbart commented Feb 24, 2022

I meant to include this in my original post, but re.match() is faster than re.search(). For my use case, that's not an issue at all. I thought the change might have been to address speed needs for some users/.

benbart added a commit to benbart/mmpy_bot that referenced this issue Feb 24, 2022
...in _handle_post() and _handle_webhook()
attzonko pushed a commit that referenced this issue Feb 27, 2022
...in _handle_post() and _handle_webhook()
@attzonko
Copy link
Owner

Since this was how it worked before the 2.x re-write I am fine with reverting it to the old behavior, despite the small reduction in performance.

unode added a commit to unode/mmpy_bot that referenced this issue Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants