-
Notifications
You must be signed in to change notification settings - Fork 876
Require a local-part to be a valid email address in AUTOMAIL_RE. #1165
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
Conversation
Make it makes sense to also replace >>> AUTOMAIL_RE = r'<([^<> !]+@[^@<> ]*)>'
>>> re.match(AUTOMAIL_RE, "<bob@>")
<re.Match object; span=(0, 6), match='<bob@>'> |
I've updated the regex to require both |
I've added tests for the missing local-part and domain. I noticed that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All new tests should use the new testing framework. It appears that no changes have been made to the autolinks since introducing the new framework so we don't have a file for that yet. One should be added at tests/test_syntax/inline/autolinks.py
. You only need to include the tests for the behaviors relevant to this PR. However, you may add additional tests if you would like.
Regarding the differing behaviors, I see that as a problem. If any part of the autolink does not match, then the behavior should be the same.
80eee41
to
c44c06e
Compare
Thanks for the pointers, I've updated the PR with the recommended changes. |
Valid email addresses require a local-part before the
@
followed by domain. This PR updates theAUTOMAIL_RE
to use the+
quantifier to require 1 or more characters in the local-part instead of the*
quantifier which allows 0 characters.The current regex allows the invalid addresses in the form
<@domain>
. My particular use case is formatting messages in Slack, where mentioned users take the form<@Uxxxxx>
. markdown is incorrectly processing the mention string form as an email address. I still want markdown's automail feature for processing email addressing in the text.The below code demonstrates the current and new behaviour: