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

fgets() is missing #189

Open
BenMorel opened this issue Jan 22, 2020 · 5 comments
Open

fgets() is missing #189

BenMorel opened this issue Jan 22, 2020 · 5 comments
Labels
missing function question Further information is requested

Comments

@BenMorel
Copy link

Hi, it looks like common file functions such as fread() and fwrite() are present, but not fgets().
Could you please add it? Thanks!

@Kharhamel
Copy link
Collaborator

Kharhamel commented Jan 24, 2020

Hello, I am not sure this will be possible. Looking at the the doc: https://www.php.net/manual/fr/function.fgets.php, The function can also return FALSE once there is nothing left to read.

All our generated functions can do is only checking for the return value to know if an exception can be thrown. We would need to find another to know if an error happens and write a special case for this function.

I will try to look into it once I have some time

@BenMorel
Copy link
Author

Thanks for your reply.

I've been actually surprised to see that your implementation only checks the return value, and does not systematically throw when a PHP error has been caught.

In the case of fgets(), a warning is thrown if an error occurs, that's how you can differentiate from a false value meaning nothing left to read, or a false value meaning an exception.

I've done something like this here:

https://github.com/brick/std/blob/0.3.0/src/Io/FileStream.php#L96

In the case of fgets(), I consider false as a non-exceptional return value. If an error occurs, the warning will be caught and converted to an exception. So the return value is not taken into account at all for exceptions.

In the case of fwrite() however, I consider false as an exceptional return value. As before, if an error occurs, the warning will be caught and converted to an exception. And if no warning is caught but false is returned, an exception is also thrown.

@Kharhamel
Copy link
Collaborator

I've been actually surprised to see that your implementation only checks the return value, and does not systematically throw when a PHP error has been caught.

Yes it is definitely much more limiting, but also easier to implement automatically and it doesn't risks messing with people's error handlers. Generally, the goal of this library isn't to rewrite the whole standard library, only to simplify error gestion a bit.

This subject isn't new, see #77 and #120

@Kharhamel
Copy link
Collaborator

Kharhamel commented Jan 24, 2020

We have a special case file where we can manually write safe versions of functions. However, i don't think we can use the code you linked, because it is kind of a design choice to not use errors handlers in safe.

@BenMorel
Copy link
Author

Hmm this is a matter of opinion I guess. I really expected this library to catch every single PHP error and re-throw it. I would personally expect Safe to bypass any custom error handler, for the duration of the function.

The fact is, in this precise case, it's really a pain to have fgets() fail because of an I/O error, and not get a catchable exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing function question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants