Skip to content

Add readFile method to Filesystem component #4

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

Closed
wants to merge 2 commits into from

Conversation

manie20
Copy link

@manie20 manie20 commented Jul 28, 2014

Apologies if this PullRequest should not be submitted like this.
I'm rather new to GitHub.

I would love to see read file integration since this is a crucial part of the filesystem.
Is there a reason for this not begin implemented in the component?

(e.g. should I include my own fork)

@fabpot
Copy link
Member

fabpot commented Jul 28, 2014

This addition has already been rejected in the past. The reason is that the Filesystem class abstract "complex" operations, it's not an abstraction for a filesystem.

@fabpot fabpot closed this Jul 28, 2014
@jrjohnson
Copy link

@fabpot I'm not clear on exactly what you are saying here - is it that reading a file is an easy operation and therefor doesn't need to be handed by this class? I can agree with that - but I would still very much like to see a read file option here for because it means that if I depend on the filesystem service I can mock it in tests including reading a file. Otherwise I have to jump through the hoop of creating a class which extends filesystem and provides only this one additional method.

If there is any chance of including a read method I would be happy to bring this PR up to date and add some tests.

@TomasVotruba
Copy link

Is this still a mental blocker?

I have to use 2 classes in my filesystem operations, just because this method is missing.

image

Which doesn't make sense, as RUD part of CRUD is already there.

@tacman
Copy link

tacman commented Jul 26, 2020

What is the proper way to read a file? The file_get_contents function seems low-level. Since the dumpFile() method exists, it seems natural that there would be a reverse command.

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.

5 participants