Skip to content

[HttpFoundation] Allow File instance to be passed to BinaryFileResponse #10797

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

Merged
merged 1 commit into from
Apr 28, 2014
Merged

[HttpFoundation] Allow File instance to be passed to BinaryFileResponse #10797

merged 1 commit into from
Apr 28, 2014

Conversation

anlutro
Copy link
Contributor

@anlutro anlutro commented Apr 28, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #10608
License MIT
Doc PR

See #10608 for discussion and reasoning.

The docblocks get rather long with this change. The File class does extend SplFileObject so technically it could be left out of the docblock if that is more desirable.

@stof
Copy link
Member

stof commented Apr 28, 2014

given that it is a subclass of SplFileInfo, it could indeed be left out. And if we put it explciitly, it should be File, not File\File, as we have a use statement for the class

@anlutro
Copy link
Contributor Author

anlutro commented Apr 28, 2014

Thanks, fixed that.

@fabpot
Copy link
Member

fabpot commented Apr 28, 2014

@anlutro I don't see the changes asked by @stof in your latest commit. Forgot to push?

@anlutro
Copy link
Contributor Author

anlutro commented Apr 28, 2014

Oops, yes. Pushed again now.

* @param null|string $contentDisposition The type of Content-Disposition to set automatically with the filename
* @param bool $autoEtag Whether the ETag header should be automatically set
* @param bool $autoLastModified Whether the Last-Modified header should be automatically set
* @param File|\SplFileInfo|string $file The file to stream
Copy link
Member

Choose a reason for hiding this comment

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

As @stof mentioned in the comments, you can remove the type hint on File here as it extends SplFileInfo (same below.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@fabpot
Copy link
Member

fabpot commented Apr 28, 2014

Thank you @anlutro.

@fabpot fabpot merged commit fc04ad2 into symfony:2.3 Apr 28, 2014
fabpot added a commit that referenced this pull request Apr 28, 2014
…yFileResponse (anlutro)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpFoundation] Allow File instance to be passed to BinaryFileResponse

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #10608
| License       | MIT
| Doc PR        |

See #10608 for discussion and reasoning.

The docblocks get rather long with this change. The File class does extend SplFileObject so technically it could be left out of the docblock if that is more desirable.

Commits
-------

fc04ad2 Allow File instance to be passed to BinaryFileResponse
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants