Fixed #10941 -- Added {% query_string %} template tag. #17368
Conversation
knyghty
force-pushed
the
query-string
branch
3 times, most recently
from
4906870
to
b69f44b
Compare
felixxm
changed the title
| @@ -10,6 +10,7 @@ | |||
| from django.conf import settings | |||
| from django.utils import timezone | |||
| from django.utils.html import conditional_escape, escape, format_html | |||
| from django.utils.itercompat import is_iterable | |||
There was a problem hiding this comment.
I'm curious as to why we still use this over isinstance(…, collections.abc.Iterable)? 🤔
Looking at Python's documentation -- see 3) in the first section -- it seems as though it should work perfectly fine. Is there something I'm missing?
It looks as though this was introduced in ad077cc for compatibility with Jython which we no longer support… Perhaps it's time to deprecate is_iterable()?
There was a problem hiding this comment.
is_iterable would presumably be True for objects using the old __getitem__ method, which I don't think is equivalent for isinstance(..., Iterable):
Checking
isinstance(obj, Iterable)detects classes that are registered asIterableor that have an__iter__()method, but it does not detect classes that iterate with the__getitem__()method. The only reliable way to determine whether an object is iterable is to calliter(obj).
That may or may not matter these days (or my reading my be wrong 🤷)
There was a problem hiding this comment.
It looks as though this was introduced in ad077cc for compatibility with Jython which we no longer support… Perhaps it's time to deprecate
is_iterable()?
Maybe it's time, but we should discuss it and do it separately. I've changed to is_iterable() to keep the code consistent for now.
There was a problem hiding this comment.
I opened ticket-34983 for this.
|
That's a great one, thanks a ton! ❤️ |
https://code.djangoproject.com/ticket/10941
https://forum.djangoproject.com/t/adding-a-template-tag-to-generate-query-strings/24521/4
I was unsure about
query_stringvsquerystringas most template tags don't have underscores, but I do prefer with the underscore, personally.We could also simplify the logic a bit if we say we only allow lists (or nothing at all) when setting something to a list, but it seems nicer to allow any iterable to work.
Credit to Carsten Fuchs for the initial implementation.