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

Fixed #28628 --Replaced '\d' in regexes with [0-9] for more strict matching. #9185

Closed
wants to merge 1 commit into from

Conversation

JunyiJ
Copy link
Contributor

@JunyiJ JunyiJ commented Oct 1, 2017

In Python2 and Python3 '\d' in regular expression matches to different patterns. To avoid errors, '\d' was replaced with [0-9].

@wkschwartz
Copy link
Contributor

The matching rules for \d are identical in Pythons 2 and 3. In both versions, u'\d' matches Unicode decimal characters and b'\d' matches b'[0-9]'. Unless some test started failing or you have a particular use case that's stopped working, I'm not sure this PR is necessary. Personally, I find \d more legible than [0-9].

@JunyiJ
Copy link
Contributor Author

JunyiJ commented Oct 2, 2017

Based on Python3's document, what '\d' corresponding to is slightly different from Python2:

Matches any Unicode decimal digit (that is, any character in Unicode character category [Nd]). This includes [0-9], and also many other digit characters.

However, I actually kind of agree with you that '\d' seems to be more robust. But if someone intended to use only [0-9], she/he might meet problem with the current version of django.

@timgraham
Copy link
Member

It would be good to add tests to show what benefit each change has.

@timgraham timgraham changed the title Ref#28628--replace '\d' in regex to [0-9] Fixed #28628 --Replaced '\d' in regexes with [0-9] for more strict matching. Oct 20, 2017
@timgraham
Copy link
Member

Junyi, do you want to move forward with this or should we close it? Absent some tests to show some benefit, I'm not sure.

@JunyiJ
Copy link
Contributor Author

JunyiJ commented Mar 20, 2018

Hi, @timgraham. You can close it. Thanks!

@timgraham timgraham closed this Mar 20, 2018
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.

3 participants