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

User-space Stream implementations and TakeResult #19

Closed
mallardduck opened this issue Dec 14, 2020 · 1 comment
Closed

User-space Stream implementations and TakeResult #19

mallardduck opened this issue Dec 14, 2020 · 1 comment

Comments

@mallardduck
Copy link
Contributor

mallardduck commented Dec 14, 2020

The main point of this Issue is to seek understanding of the overall intentions of the Stream interface.
I started toying with the library recently and thought it would be cool to implement a TextFileStream.
After looking over the StringStream it seemed pretty easy to get worked out using fopen and PHP's stream functions.

However I ran into a few odd issues and wanted to understand if I was doing it wrong, or if this was an area of improvment for the library. Those issues were:

  1. When defining my own Stream I found TakeResult was marked as internal.
  2. Using my own defined stream seems to yield issues with EOF.
  3. In my file based stream characters get eaten and lost.

Copy of TextFileParser: https://gist.github.com/mallardduck/dd2dab36d0713e5373583e74f2156381

user defined Streams and TakeResult

While I was able to get it working, I did notice along the way that TakeResult is type hinted as a return types.
However it's also marked as @internal so in PHPStorm it show's as crossed out indicating it shouldn't be used.
See:
Screen Shot 2020-12-14 at 11 30 45 AM

So that got me wondering two things: a) should this just be safely ignored or maybe the @internal tag changed, and b) should a consumer of this library expect to be able to define their own streams at all? Or maybe just not yet?


UPDATE: After some further testing and examining the Stream I wrote I found my flaw.
The root issue were two fold in the end, one caused by me and one by overlooking how the file socket works.
So the one I caused, was that I was overwriting the Position, then setting that on the new one.

So something like:

$this->position = $this->position->advance($chunk);
...
new TextFileStream($this->filePath, $this->position)

That was fixed by leaving the property alone, assigning to the temp variable like StringStream does and then using that.

The second issue was that I was unknowingly letting the file sockets active position drift.
So to fix that I started properly setting the position on the file resource right before taking anything from it.

@mallardduck
Copy link
Contributor Author

Closing this issue for now as it was mostly learn curve issues.
At this point, i'm noodling around to implement a good TextFileStream to PR.

Twitter thread for context: https://twitter.com/parsica_php/status/1339193793552273409

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

1 participant