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

Add fopen method to ISimpleFile #10122

Merged
merged 2 commits into from
Jul 11, 2018
Merged

Conversation

juliusknorr
Copy link
Member

This allows to also use the AppData for large files where using a FileDisplayResponse will fail, because it is calling getContent and trying to put the whole file in memory.

With this, apps can properly use a StreamResponse.

@icewind1991
Copy link
Member

I would prefer having separate read and write methods instead of a generic fopen

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 9, 2018
@juliusknorr juliusknorr force-pushed the feature/noid/appdata-fopen-stream branch from 370fdfd to 8100a14 Compare July 10, 2018 12:28
@juliusknorr
Copy link
Member Author

@icewind1991 I moved it to two separate methods.

* @since 14.0.0
*/
public function fopen(string $mode) {
}
Copy link
Member

Choose a reason for hiding this comment

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

Empty implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, forgot to remove this. 🙈 Fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm Github doesn't show my rebased commits, but when forcepushing everything is up to date :/

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, another commit --amend fixed it.

@juliusknorr juliusknorr force-pushed the feature/noid/appdata-fopen-stream branch from 2efeed5 to 6da2b7c Compare July 11, 2018 10:08
@MorrisJobke MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 11, 2018
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code makes sense 👍

@MorrisJobke MorrisJobke merged commit 229289d into master Jul 11, 2018
@MorrisJobke MorrisJobke deleted the feature/noid/appdata-fopen-stream branch July 11, 2018 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants