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

Add decorator for matching regular expressions #87

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

grantbacon
Copy link
Contributor

Introduces a decorator similar to @triggered--named @regex_triggered--which returns a command if any of the provided regular expression patterns match the message text

Copy link
Collaborator

@Era-Dorta Era-Dorta left a comment

Choose a reason for hiding this comment

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

This looks great, thanks a lot. It is a feature that should have been implemented long ago.

Copy link
Collaborator

@Era-Dorta Era-Dorta left a comment

Choose a reason for hiding this comment

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

I missed the comments in the my previous review, sorry.

signalbot/__init__.py Show resolved Hide resolved
signalbot/__init__.py Show resolved Hide resolved
signalbot/command.py Outdated Show resolved Hide resolved
@grantbacon
Copy link
Contributor Author

It would be nice to be able to access re.Match objects returned by re.search() in Command#handle() but I'm not sure the least disruptive way to make that change. Can always call re.search() again to get the matches I suppose

Copy link
Collaborator

@Era-Dorta Era-Dorta left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fixes, if you don't mind, I have a few more minor comments. Regarding the re.Match objects, I agree, I don't think there is a neat way to return them and pattern matching is quite fast, so I'm inclined to leave as it is for now.

signalbot/command.py Outdated Show resolved Hide resolved
example/commands/regex_triggered.py Show resolved Hide resolved
example/commands/regex_triggered.py Outdated Show resolved Hide resolved
@grantbacon
Copy link
Contributor Author

Cool! I think I've addressed all the review-- let me know if any other changes to be made, thanks again for taking a look at my changes

Copy link
Collaborator

@Era-Dorta Era-Dorta left a comment

Choose a reason for hiding this comment

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

Very happy with the last set of changes, thanks!

@Era-Dorta Era-Dorta merged commit 1a58853 into filipre:master Jan 16, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants