Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Headers.add to get a list of parameters on *args #1100

Closed
wants to merge 4 commits into from

Conversation

antlarr
Copy link

@antlarr antlarr commented Apr 7, 2017

This ensures the order of parameters, which can be useful for issues like supporting IE8 in pallets/flask#2223.

This ensures the order of parameters, which can be useful for issues
like supporting IE8 in flask's pallets#2223
@@ -1140,7 +1140,7 @@ def __iter__(self):
def __len__(self):
return len(self._list)

def add(self, _key, _value, **kw):
def add(self, _key, _value, *args, **kw):

This comment was marked as off-topic.

@@ -1153,9 +1153,15 @@ def add(self, _key, _value, **kw):
The keyword argument dumping uses :func:`dump_options_header`
behind the scenes.

In some cases, it would be needed to ensure the order of parameters.
For those cases, a list of ``(key, value)`` tuples can be passed in
*args.

This comment was marked as off-topic.

@@ -1153,9 +1153,15 @@ def add(self, _key, _value, **kw):
The keyword argument dumping uses :func:`dump_options_header`
behind the scenes.

In some cases, it would be needed to ensure the order of parameters.

This comment was marked as off-topic.

werkzeug/http.py Outdated
if isinstance(options, tuple):
iter_func = iter
else:
iter_func = iteritems

This comment was marked as off-topic.

.. versionadded:: 0.4.1
keyword arguments were added for :mod:`wsgiref` compatibility.
"""
if args:
_value = dump_options_header(_value, args)

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

pass args to _options_header_vkw
allow positional args to set too
more permissive mapping vs sequence check
@davidism davidism requested a review from untitaker April 8, 2017 01:54
@untitaker
Copy link
Contributor

From pallets/flask#2223:

Changing the Headers API to care about ordering of options would require a refactor. I don't think that is warranted for a workaround for older browsers.

I don't think we should go down this route since by the standard there is no reason this should be necessary. Also we're talking about IE8 here. If the user cares about that browser they can ensure they pass an ASCII filename to send_file and everything will be fine.

@davidism
Copy link
Member

davidism commented Apr 8, 2017

I guess the order shouldn't matter for the options, but it still seemed like a relatively minor addition. Who knows if there's other weird browser behavior that still cares about the order? I wasn't planning to apply this to Flask since that would require pinning the version.

@untitaker
Copy link
Contributor

untitaker commented Apr 8, 2017 via email

@davidism
Copy link
Member

davidism commented Apr 8, 2017

Fair enough. Thanks for making this PR @antlarr. We'll close it for now, but if it comes up in a modern situation later we can merge it then.

@davidism davidism closed this Apr 8, 2017
@antlarr antlarr deleted the ensure-parameters-order branch April 20, 2017 08:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants