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

Issue with url function replacement #121

Closed
qdufrois opened this issue Feb 1, 2022 · 4 comments · Fixed by #133
Closed

Issue with url function replacement #121

qdufrois opened this issue Feb 1, 2022 · 4 comments · Fixed by #133

Comments

@qdufrois
Copy link

qdufrois commented Feb 1, 2022

Python Version

3.7

Django Version

2.2

Package Version

1.4.0

Description

Django-upgrade changes this kind of url functions:
url(r"^foo", my_view)

with these path functions:
path("foo", my_view)

However, the automated change made by the library is not backwards compatible. The initial code allowed all paths such as foos, footest etc., whereas the upgraded code only allows foo (the missing $ in the first example being the problem).

For this use-case, a backwards compatible solution would have been this:
re_path(r"^foo", my_view)

I would be glad to offer any help needed if you would take this issue into consideration.

@matthiask
Copy link

I'm surprised that url(r"^/foo", ...) even matches anything. it doesn't seem to work in my local testing at all.

The Django docs state that there's no need to add a leading slash and I haven't found a single example containing a leading slash. I'm sure it works on your end but it's still strange.

@qdufrois
Copy link
Author

qdufrois commented Feb 2, 2022

Yeah my bad, that was not a good example, let's forget about those leading slashes, I have removed them in the issue to avoid the confusion.

@adamchainz
Copy link
Owner

Fixed in #133

@qdufrois
Copy link
Author

Thanks a lot for this 🙇

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 a pull request may close this issue.

3 participants