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

feat: allow reading from and writing to php resource #71

Closed
darthf1 opened this issue Apr 1, 2022 · 4 comments
Closed

feat: allow reading from and writing to php resource #71

darthf1 opened this issue Apr 1, 2022 · 4 comments

Comments

@darthf1
Copy link

darthf1 commented Apr 1, 2022

Hi!

Thanks for creating this fork. Migration was easy and with the types and additional features its a huge improvement over the original codebase.

I have a feature request; Is it possible to allow reading from and writing to a php resource? At least for CSV, I'm not sure if it's possible for XLSX (and other formats) as well.

I noticed the current CSV Reader already converts the filePath to a resource. Maybe it's as easy as changing

protected function openReader(string $filePath): void

to

protected function openReader(string | resource $filePath): void

and check which parameter type has passed?

As for the CSV Writer, I'm not sure if it is the same case?

final public function openToFile($outputFilePath): void

to

final public function openToFile(string | resource $outputFilePath): void
@Slamdunk
Copy link
Contributor

Slamdunk commented Apr 1, 2022

Hi, may I ask what's your use case?

@darthf1
Copy link
Author

darthf1 commented Apr 1, 2022

Yes ofcourse,

My filesystem is abstracted using Flysystem and I'm making heavy usage of FilesystemReader::readStream and FilesystemWriter::writeStream methods.

If the reader supports streams, I can pass the result of FilesystemReader::readStream directly without additional writes to disk, and vice versa for the writer.

Also, not the least, with Flysystem I can only get the contents of the file and not the path to the file since files do not need to "live" on the current filesystem.

@Slamdunk
Copy link
Contributor

Slamdunk commented Apr 1, 2022

Regarding the Writer, although it should be possible to write to a user-provided stream, it wouldn't work as you expect with Flysystem: FilesystemWriter::writeStream expects an already filled stream, you cannot pass to it a stream that still needs to receive the data. So at least a php://memory intermediate stream would be needed anyway and in my experience that's more of a burden rather than an help.

Regarding the Reader, only the CSV one could achieve your goal as both ODS and XLSX rely on ZipStream::open which doesn't accept a stream.
And I don't like very much to add a feature so specific to a single component.

I have a decent experience with Flysystem, streams and filesystem operations on PHP, and although bypassing the disk entirely seems good in theory I strongly suggest you to rely on physical temporary files for intermediate steps, you'll save yourself a lot of pita.

@darthf1
Copy link
Author

darthf1 commented Apr 1, 2022

And I don't like very much to add a feature so specific to a single component.

I fully agree. It should only be done if possible for all formats.

Thank you for your reply and your advice. I'll close this request for now.

@darthf1 darthf1 closed this as completed Apr 1, 2022
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

No branches or pull requests

2 participants