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

Document path converter regexes and limitation #229

Merged
merged 2 commits into from Sep 24, 2022

Conversation

UnknownPlatypus
Copy link
Contributor

I noticed while testing on a personal project that the url slug rewriter was matching the [-a-zA-Z0-9_]+ but not the equivalent [\w-] or [-\w].

I thought maybe we could support that too ?

@adamchainz
Copy link
Owner

Django’s SlugConverter uses that regex:

https://github.com/django/django/blob/f88fc72da4eb76f2d464edb4874ef6046f8a8658/django/urls/converters.py#L35-L36

\w matches more characters, since it matches all Unicode letter chars:

In [1]: import re

In [2]: re.match(r'\w', 'α')
Out[2]: <re.Match object; span=(0, 1), match='α'>

So unfortunately we can't automatically convert here. But I understand this might be quite common. Perhaps we could add some flag or extensibility here to allow saying that "in this project \w really only means to match latin letters" ?

@UnknownPlatypus
Copy link
Contributor Author

Oh indeed, the greek letters.

Maybe a flag is a great idea if this case seems to be quite common ?
I'm not sure it actually is thought. Maybe we should keep the current behaviour after-all ?

@adamchainz
Copy link
Owner

I think at least we could document the regexs, and maybe add a sentence that \w is sometimes used for slugs but it's not converted due to potential incompatibility. Would you like to take a crack at that?

@UnknownPlatypus
Copy link
Contributor Author

Looks like a good idea, I'll redo this PR tomorrow to document the slug regexes and why we don't support \w

@UnknownPlatypus
Copy link
Contributor Author

Took me more time than expected to get back to this, I'm in the middle of moving to a new place.
Anyway, let me know if it's looking good for you!

@UnknownPlatypus UnknownPlatypus changed the title Add support for alternative slug regexes Document path converter regexes and pitfalls Sep 22, 2022
@adamchainz adamchainz changed the title Document path converter regexes and pitfalls Document path converter regexes and limitation Sep 24, 2022
@adamchainz
Copy link
Owner

Looking good, I just made a few tweaks to improve readability.

First commit merged here at the Djangocon Europe sprints 👍👍👍

@adamchainz adamchainz merged commit f0d9aa7 into adamchainz:main Sep 24, 2022
@UnknownPlatypus UnknownPlatypus deleted the support_other_slug_regex branch September 25, 2022 09:10
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.

None yet

2 participants