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

Added an advanced example, returning a different filename. #3

Merged
merged 2 commits into from
Mar 6, 2012
Merged

Added an advanced example, returning a different filename. #3

merged 2 commits into from
Mar 6, 2012

Conversation

clalarco
Copy link
Contributor

@clalarco clalarco commented Mar 6, 2012

I added an advanced example, where a different filename is returned. This worked for me and it's not documented.

$response = $this->get('igorw_file_serve.response_factory')
->create( $filename = $real_path,
$contentType = 'application/octet-stream',
$options = $file_options,
Copy link
Contributor

Choose a reason for hiding this comment

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

why assigning new variables here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify the example code. If you like you can write it in a line, but for me it's a bit confusing.
Now that including the arguments names have the same intention.

Copy link
Contributor

Choose a reason for hiding this comment

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

the issue is not about using several lines but about creating the $filename, $contentType and $options variables (btw, I'm not even sure it is valid)

@clalarco
Copy link
Contributor Author

clalarco commented Mar 6, 2012

However, this worked fine.

@igorw igorw merged commit 5ecd9a8 into igorw:master Mar 6, 2012
@igorw
Copy link
Owner

igorw commented Mar 6, 2012

Merged, thanks.

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.

None yet

3 participants