Skip to content

Add content disposition type parameter to FileResponse - #1266

Merged
lovelydinosaur merged 2 commits into
Kludex:masterfrom
qu1ck:disposition
Feb 14, 2022
Merged

lovelydinosaur merged 2 commits into
Kludex:masterfrom
qu1ck:disposition

Conversation

@qu1ck

@qu1ck qu1ck commented Aug 14, 2021

Copy link
Copy Markdown
Contributor

disposition "attachment" causes browsers to download
the file. E.g. "inline" will will be attempted to be
displayed directly.

@qu1ck

qu1ck commented Sep 16, 2021

Copy link
Copy Markdown
Contributor Author

Friendly ping.
I have a larger PR pending for support of 206 partial content responses that I want to send after this is merged.

@Kludex Kludex left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation needs to be updated on this PR as well.

Comment thread starlette/responses.py
filename: str = None,
stat_result: os.stat_result = None,
method: str = None,
content_disposition_type: str = "attachment",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only "inline" and "attachment" are allowed here, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

@Kludex

Kludex commented Sep 16, 2021

Copy link
Copy Markdown
Owner

disposition "attachment" causes browsers to download
the file. E.g. "inline" will will be attempted to be
displayed directly.
@qu1ck

qu1ck commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

I added the parameter description to the docs. There are a couple other parameters missing there but it's outside of the scope of the pull request, just FYI.

@adriangb adriangb added the feature New feature or request label Feb 2, 2022

@lovelydinosaur lovelydinosaur left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty okay with this yup.
It's a nice low-footprint change, so no big objections.

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

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants