Adding a template tag to generate query strings

To me, this could be fixed instead by adding support for *args and **kwargs syntax in the template language

and how would one construct those args/kwargs in the template language?

I’m also -1 on the “quoting keys” suggestion because it breaks consistency. Another place we parse plain keys without quotes is custom template tags.

Adding **kwargs support sounds like the best way forward to me, and best done as a follow up feature.

Edit: another option could be supporting {% querystring mydict %}.

1 Like

One wouldn’t :grin:

You’d either pass them to the context from the view, or via a custom template tag.

I am not sure, one can already pass a dict to query_string on which query_string then operates instead of request.GET

To be honest, I don’t have a convincing use case for the following, but supporting multiple dicts as in

{% querystring mydict1 mydict2 %}

(where mydict1 is typically request.GET and mydict2 is some custom “options” dict) seems to be an approach that is both generic and unobtrusive.

Jup, {% querystring mydict1 mydict2 %} would probably be okay and not the end of the world :slight_smile:

I realize the following goes into nitpicking: But do we want to keep the name query_string or do we want to use the chance (given it is alpha) to change to querystring. The latter feels more natural to me as non-english native speaker, but that is about it…

1 Like

I think this change is backwards compatible so not necessarily a release blocker in that case, unless we really wanted to sneak it in.

This comment from @bmispelon corresponds to my view:

I think both having to quote literal keys would be a pain, and that some kind of **kwarg expansion would be a generally useful technique for us to pick up.

(No opinion on the possible name change. :person_shrugging: )

Just passing by as I saw the ticket created on the subject.

I’m -1 on requiring quoting of keys, +1 on adding support for passing Mapping[str, Any] variables as argument to the template tag, and +1 on renaming to querystring.

1 Like

I have no strong opinion on the naming. I weakly prefer the current name because that’s how it tends to get referred across the web, with a space. But I also realise most (though not all) built in tags avoid underscores when they should grammatically have one.

Based off this discussion:

Then there is a potential name change, I have no strong opinion, sounds like there is a slight preference for querystring (but will wait before raising a ticket)

Hello! I’m +1 to agreed path forward, specifically I’m quite keen on the proposed name change. Thanks for raising this!

Just to add another datapoint, the only builtin template tag with an underscore currently is csrf_token where it kinda makes sense (Oh and get_static_prefix/get_media_prefix). But stuff like firstof and resetcycle don’t have an underscore and there I’d argue it would actually make sense. With that in mind I am +1 on the name change while we have the chance.

EDIT:// There are some filters which have underscores as well, so it’s not like we are 100% consistent either way…

I also think I should mention we also have e.g. HttpRequest.META.QUERY_STRING so I’m a bit tense at having different “underscorisations” or however you call that, for what is essentially the same noun in a different place.

But again it’s not an argument I care that much about the result of.

If we are to change the name we should do it sooner rather than later (so raised a draft PR: Refs #10941 – Renamed query_string to querystring. by sarahboyce · Pull Request #18323 · django/django (github.com))

Also, if we are to do this, we should possibly consider adding an item to our coding style docs

I think we are at 3x +1, I think Tom is a -0. If anyone wants to pitch in, you’re welcome to :+1:

I’m +1 for renaming. I think I started this whole discussion by typing the tag without _ back in this comment:

Hi! I assigned myself the #35529 ticket – @sarahboyce thanks for the unit tests there, they are very helpful.

There’s one thing that comes to mind that probably needs discussion.
In the case of having the same keys in both request.GET and kwargs, we currently use the kwargs value (ref). Since now we want to allow passing dicts and also multiple arguments, what do you think the order of precedence should be?

I guess one approach can be to disregard the previous keys, so whatever comes next takes precedence.
Example

having:
>> request.GET: {"foo": "bar"}
>> mydict1: {"foo": "baz", "other": "fine"}
>> mydict2: {"foo": "car"}
{% querystring foo="zoo" mydict1 mydict2 %}

would result in ?foo=car&other=fine.

Note also that as it stands now, the tag raises TemplateSyntaxError for same key kwargs - {% querystring foo="bar" foo="baz" %} - so we could somehow align the two behaviors? :thinking:

thanks!

Hello,

I think this is one of the most important features of this tag.

1 Like

Yes, later dictionaries should have precedence, just like Python’s dictionary merging:

In [1]: d1 = {"a": 1}

In [2]: d2 = {"a": 2}

In [3]: {**d1, **d2}
Out[3]: {'a': 2}

In [4]: d1 | d2
Out[4]: {'a': 2}

(Perhaps this should even be used in the implementation.)

That’s different - two explicit keys means the developer made a mistake. This should remain an error, just like Python’s SyntaxError for duplicate arguments:

In [6]: dict(mapping={}, mapping={})
  Cell In[6], line 1
    dict(mapping={}, mapping={})
                     ^
SyntaxError: keyword argument repeated: mapping
2 Likes

Thanks everyone for your input. I think there is a consensus to rename and not quote keys.

Given that 5.1rc is scheduled for the 24th, I will work on reviewing/pushing the PRs above forward and have them ready before the RC release.