Skip to content

bpo-31677: email: Add re.ASCII when re.IGNORECASE is used #3868

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

Merged
merged 2 commits into from
Oct 4, 2017

Conversation

methane
Copy link
Member

@methane methane commented Oct 3, 2017

While there is not real bug in this case, using re.ASCII is
safe when re.IGNORECASE is used.

This commit removes dead copy in email.util too.

https://bugs.python.org/issue31677

While there is not real bug in this case, using re.ASCII is
safe when re.IGNORECASE is used.

This commit removes dead copy in email.util too.
Copy link
Member

@warsaw warsaw left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for removing the dead code too.

\? # literal ?
(?P<atom>.*?) # non-greedy up to the next ?= is the atom
\?= # literal ?=
''', re.VERBOSE | re.IGNORECASE)
Copy link
Member

Choose a reason for hiding this comment

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

This was probably left in place for backward compatibility reasons (just in case someone was using it). Or it may have been overlooked. No way to tell :) But, probably better to not delete it if you backport this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since this change doesn't actual bugfix (B and Q don't have another lower cases),
I won't backport it to Python 3.6.

@methane methane merged commit bf477a9 into python:master Oct 4, 2017
@methane methane deleted the email-re branch October 4, 2017 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants