[Django] #27238: Removal of check_pattern_startswith_slash
Django
--------------------------------------+--------------------
Reporter: strycore | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (System checks) | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27238>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: strycore | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (System | Version: 1.10
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: alasdairnicol (added)
* needs_docs: => 0
* needs_tests: => 0
* needs_better_patch: => 0
Comment:
The original ticket is #23813. Maybe it's enough to disable the check if
`setting.APPEND_SLASH = False`?
--
Ticket URL: <https://code.djangoproject.com/ticket/27238#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Core (System | Version: 1.10
checks) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by alasdairnicol):
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
run `check_pattern_startswith_slash`. This would prevent the false
positives for strycore's example.
* If we call `check_resolver(resolver, r'^dashboard/')` or
`check_resolver(resolver, None)` (initial call) then it is ok to run
`check_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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27238#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: strycore | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (System checks) | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by timgraham):
* stage: Unreviewed => Accepted
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27238#comment:3>
Django
Reporter: strycore | Owner:
Type: | alasdairnicol
Cleanup/optimization | Status: assigned
Component: Core (System | Version: 1.10
checks) |
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by alasdairnicol):
* status: new => assigned
* owner: nobody => alasdairnicol
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27238#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: strycore | Owner:
Type: | alasdairnicol
Component: Core (System | Version: 1.10
checks) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"911d9f4ed1a39f945769b7198a419850378f9824" 911d9f4e]:
{{{
#!CommitTicketReference repository=""
revision="911d9f4ed1a39f945769b7198a419850378f9824"
Fixed #27238 -- Disabled check_pattern_startswith_slash if
settings.APPEND_SLASH=False.
Thanks strycore for the report and timgraham for suggesting the
solution.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27238#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: strycore | Owner:
Type: | alasdairnicol
Cleanup/optimization | Status: closed
Component: Core (System | Version: 1.10
checks) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"190cd0e49f967d2ec0b6c50a63f0d58d13319611" 190cd0e4]:
{{{
#!CommitTicketReference repository=""
revision="190cd0e49f967d2ec0b6c50a63f0d58d13319611"
[1.10.x] Fixed #27238 -- Disabled check_pattern_startswith_slash if
settings.APPEND_SLASH=False.
Thanks strycore for the report and timgraham for suggesting the
solution.
Backport of 911d9f4ed1a39f945769b7198a419850378f9824 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27238#comment:6>