fix: use re.search instead of re.match in RegexFilter#5368
fix: use re.search instead of re.match in RegexFilter#5368KBVsent wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello @KBVsent, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a subtle but significant issue in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - 我在这里给出了一些总体反馈:
- 从
match切换到search会改变所有现有RegexFilter用法的行为;请考虑是否需要提供向后兼容的路径(例如通过一个标志位或单独的过滤器),以支持那些隐式依赖“从字符串开头匹配”的插件。 - 在应用正则之前调用
.strip()的做法,在你希望对齐标准正则语义的情况下,现在可能会更让人意外;请考虑首尾空白是否应该保留为被搜索字符串的一部分,或者是否应该做成可配置的。
面向 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- Switching from `match` to `search` changes the behavior for all existing `RegexFilter` usages; consider whether you need a backward-compatibility path (e.g., a flag or separate filter) for plugins that implicitly relied on start-of-string matching.
- The use of `.strip()` before applying the regex may now be more surprising given the intent to align with standard regex semantics; consider whether leading/trailing whitespace should remain part of the string being searched or be made configurable.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的评审质量。
Original comment in English
Hey - I've left some high level feedback:
- Switching from
matchtosearchchanges the behavior for all existingRegexFilterusages; consider whether you need a backward-compatibility path (e.g., a flag or separate filter) for plugins that implicitly relied on start-of-string matching. - The use of
.strip()before applying the regex may now be more surprising given the intent to align with standard regex semantics; consider whether leading/trailing whitespace should remain part of the string being searched or be made configurable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Switching from `match` to `search` changes the behavior for all existing `RegexFilter` usages; consider whether you need a backward-compatibility path (e.g., a flag or separate filter) for plugins that implicitly relied on start-of-string matching.
- The use of `.strip()` before applying the regex may now be more surprising given the intent to align with standard regex semantics; consider whether leading/trailing whitespace should remain part of the string being searched or be made configurable.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug in RegexFilter where re.match was used instead of re.search, causing regex patterns to be incorrectly anchored to the start of the string. The change to re.search is the right solution. My review includes a suggestion to add unit tests for this filter to verify the fix and prevent future regressions.
re.match 来自 Python 早期,当时的设计参考了其他语言的正则 API。它作为一个独立函数保留,也是为了向后兼容。re.match 是一个设计上的冗余,现代项目不应该依赖这个函数。 |
RegexFilterwas usingre.match()to evaluate@filter.regexpatterns, butre.match()implicitly anchors the match to the start of the string — equivalent to prepending^to every pattern. This means patterns without an explicit^would silently behave as if one were present, causing messages where the pattern appears mid-sentence to not trigger the handler. This violates standard regex semantics and surprises plugin developers who expect unanchored patterns to match anywhere in the message.Modifications / 改动点
astrbot/core/star/filter/regex.py: Replaceself.regex.match(...)withself.regex.search(...)so that unanchored patterns match anywhere in the message. Plugins that require start-of-string matching can still use an explicit^anchor.Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
Bug Fixes:
Original summary in English
Summary by Sourcery
Bug Fixes: