#27238 closed Cleanup/optimization (fixed)
Disable check_pattern_startswith_slash if settings.APPEND_SLASH=False
Reported by: | Mathieu Comandon | Owned by: | Alasdair Nicol |
---|---|---|---|
Component: | Core (System checks) | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Alasdair Nicol | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
I would like to propose the removal of the check_pattern_startswith_slash system check as there are legitimate reasons to have url patterns starting with a slash and this warning can be misleading.
The Django framework has a strong bias in favor of trailing slashes in URLs but not everyone wishes to set up their urls that way. If this warning was to be respected, it's impossible to have urls without a trailing slash in some situations:
# myapp. urls urlpatterns = [ url(r'^dashboard', include('dashboard.urls')), ] # dashboard.urls urlpatterns = [ url(r'^$', views.dashboard_home, name='dashboard_home'), url(r'^/users$', views.users, name='dashboard_users'), ]
The above URLconfs allow to have the URLs /dashboard and /dashboard/users but it will produce the warning.
When trying to make the slash optional in the root URLconf with r'^dashboard/?'
, the 2nd url will reverse to /dashboardusers.
I know that some system checks can be silenced but I'd be in favor of the complete removal of the check since it can be misleading for users who do not wish trailing slashes in their URLs.
Change History (6)
comment:2 by , 8 years ago
We could change the check_resolver
method so that it passes the regex when called recursively.
- If we call
check_resolver(resolver, r'^dashboard')
then we shouldn't runcheck_pattern_startswith_slash
. This would prevent the false positives for strycore's example. - If we call
check_resolver(resolver, r'^dashboard/')
orcheck_resolver(resolver, None)
(initial call) then it is ok to runcheck_pattern_startswith_slash
.
I haven't tried writing a patch for this yet. Modifying the check_resolver method like this might be overly complex. I like the simplicity of checking settings.APPEND_SLASH
as Tim suggested.
comment:3 by , 8 years ago
Easy pickings: | set |
---|---|
Summary: | Removal of check_pattern_startswith_slash → Disable check_pattern_startswith_slash if settings.APPEND_SLASH=False |
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 8 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
The original ticket is #23813. Maybe it's enough to disable the check if
setting.APPEND_SLASH = False
?