Fixes issue #495. #502

Closed
wants to merge 2 commits into
from

Conversation

2 participants
@jfinkels
Contributor

jfinkels commented Apr 26, 2012

Bam! Also collapses multiple json.dumps() and response_class() statements into a single one, respectively. This is done because it will come in handy in the future when making changes to the arguments to those function calls.

@rduplain

This comment has been minimized.

Show comment
Hide comment
@rduplain

rduplain Apr 26, 2012

Contributor

Thanks for the pull request. Here's an alternative:
https://gist.github.com/2480050

As it stands right now, we may pull JSONP from the Flask core, and prefer leaving it for an extension. The reasons being:

  1. the API for the padded jsonify is a bit awkward with bool or str as possible values, and the str case being either a request arg or a function name
  2. modern browsers do not need JSONP
  3. JSONP would benefit from CSRF protection, which could use the Flask-SeaSurf extension

JSONP support is currently not released in Flask, only in master.

Contributor

rduplain commented Apr 26, 2012

Thanks for the pull request. Here's an alternative:
https://gist.github.com/2480050

As it stands right now, we may pull JSONP from the Flask core, and prefer leaving it for an extension. The reasons being:

  1. the API for the padded jsonify is a bit awkward with bool or str as possible values, and the str case being either a request arg or a function name
  2. modern browsers do not need JSONP
  3. JSONP would benefit from CSRF protection, which could use the Flask-SeaSurf extension

JSONP support is currently not released in Flask, only in master.

@jfinkels

This comment has been minimized.

Show comment
Hide comment
@jfinkels

jfinkels Apr 26, 2012

Contributor

I agree with you on all three points. Maybe I'll try to put together a simple Flask-JSONP extension...

A minor detail about your Gist: I would change indent=None if request.is_xhr else 2 to indent=None if request.is_xhr or callback else 2, so that the wrapped JSON does not have indentation in it.

Contributor

jfinkels commented Apr 26, 2012

I agree with you on all three points. Maybe I'll try to put together a simple Flask-JSONP extension...

A minor detail about your Gist: I would change indent=None if request.is_xhr else 2 to indent=None if request.is_xhr or callback else 2, so that the wrapped JSON does not have indentation in it.

jfinkels added a commit to jfinkels/flask that referenced this pull request May 2, 2012

@rduplain

This comment has been minimized.

Show comment
Hide comment
@rduplain

rduplain May 8, 2012

Contributor

JSONP removed. Note discussion here on a Flask-JSONP extension.

Contributor

rduplain commented May 8, 2012

JSONP removed. Note discussion here on a Flask-JSONP extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment