You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Not sure if this is a bug strictly speaking or a feature request, but after running this package with --target-version 4.1 the fixer that converts re_path usage to path misses cases which should result in a direct translation.
An example from our codebase is re_path(r"^accounts/", include("allauth.urls")),
In the current version this line is left unmodified.
Presumably something like this could be automatically converted to path("accounts/", include("allauth.urls")),
The text was updated successfully, but these errors were encountered:
Ah right, currently the fixer only considers regexes that end with $, which prevents it from applying in this situation. I think we could maybe allow non-$-terminated regexes for include() only. It's not safe to convert for views since the lack of $ means the regex matches any characters at the end.
Agree, it's looking safe for the include() case, should be quite a simple update to support this case.
Should I step in for a patch ? I've seen you started some refactoring to fix #250 so maybe we should wait for this before doing more work on this fixer?
PS: @amrishparmar See #121 for reference on why it's not safe for views.
Yeah it needs to wait until #250 is fixed. If you want to make an alternate PR before I get to work on it, feel free.
adamchainz
changed the title
re_path -> path converter missing some cases
Make re_path -> path fixer convert include()s with unterminated regexes
Oct 22, 2022
Python Version
3.10.7
Django Version
4.1.2
Package Version
1.10.0
Description
Not sure if this is a bug strictly speaking or a feature request, but after running this package with
--target-version 4.1
the fixer that convertsre_path
usage topath
misses cases which should result in a direct translation.An example from our codebase is
re_path(r"^accounts/", include("allauth.urls")),
In the current version this line is left unmodified.
Presumably something like this could be automatically converted to
path("accounts/", include("allauth.urls")),
The text was updated successfully, but these errors were encountered: