#31070: Add a check for URLconfs that mix named and unnamed capture groups
-------------------------------------+-------------------------------------
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: assigned
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Baptiste Mispelon):
Replying to [comment:9 Carlton Gibson]:
> In this case, when is the `(\.html/)` bit ever used? [...]
In my example I used `...(\.html|/)$` which has a sneaky `|` (it's easy to
miss, I should have pointed it out). That way, you could access the same
view with either `/entry/123/` or `/entry/123.html`.
I think it's pretty common to use a group when using the `(A|B)` syntax,
especially when `A` and `B` are more complex than single-character
regexes.
It's also common to use a group for optional things (as you pointed out).
For example I could have rewritten the URLConf like so:
`^entry/(?<pk>\d+)(\.html)?`.
> (I guess the question is "Why would I put it in a group at all if it
were not for something?")
My best answer would be "because parentheses create capturing groups by
default and the non-capturing group syntax is somewhat obscure therefore
nobody bothers making non-capturing groups".
> If this kind of usage makes sense (which I guess in the "optional
format" example it maybe does — but even then... surely the view wants
that info!!!) then I can't see that we should raise a warning, again, for
patterns that have been working forever™.
>
> The possible error it would be worth catching is "You do know your non-
named groups won't ever be used".
>
> My concern here is a lot of warnings for perfectly valid patterns.
(Would folks use groups not meaning for them to be passed to the view?)
The potential for spurious warnings is there, I agree. There's a balance
to be found between being annoying with unnecessary warnings and
preventing real issues for our users.
You're making good arguments that this check might err too much on the
"annoying" side but I don't have a technical solution to correct that.
Thinking about this more, don't really know why Django silently discards
unnamed groups in the case of a mixed pattern (if I had to guess, I'd say
it's a historical leftover of a limitation that doesn't exist anymore).
Maybe this warning could be the first step of a deprecation path that
would end in Django passing all captured groups to the view both named and
unnamed? Or is that too ambitious?
--
Ticket URL: <https://code.djangoproject.com/ticket/31070#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.25f428db4516710f7c0aaf3eddf8ce0e%40djangoproject.com.
