Skip to content

Add fgets to File Impl in FS.h#791

Open
mjs513 wants to merge 2 commits intoPaulStoffregen:masterfrom
mjs513:master
Open

Add fgets to File Impl in FS.h#791
mjs513 wants to merge 2 commits intoPaulStoffregen:masterfrom
mjs513:master

Conversation

@mjs513
Copy link
Contributor

@mjs513 mjs513 commented Nov 29, 2025

Ran across an issue that fgets was not supported. Looking through the forum seems like a few other people ok maybe one or two. But it also looks like a handy function to have available so added it. You can look here for a quick test: https://forum.pjrc.com/index.php?threads/problems-with-external-library-using-sdfat.77492/post-363551

Will make a similar change to the SD library.

Ran across an issue that fgets was not supported.  Looking through the forum seems like a few other people ok maybe one or two.  But it also looks like a handy function to have available so added it.  You can look here for a quick test:
https://forum.pjrc.com/index.php?threads/problems-with-external-library-using-sdfat.77492/post-363551
Will make a similar change to the SD library.
mjs513 added a commit to mjs513/SD that referenced this pull request Nov 29, 2025
@A-Dunstan
Copy link
Contributor

This is not something that should be implemented in the File/FS classes, the real (libc) fgets function is usable by properly implementing _read() and _write() to use File objects as int file descriptors (fds).

(This is also a bad implementation - doesn't check if f is valid before calling the fgets virtual method on it.)

@mjs513
Copy link
Contributor Author

mjs513 commented Nov 29, 2025

At @KurtE suggestion changed the virtual fgets function to be like date virtual functions.
@mjs513
Copy link
Contributor Author

mjs513 commented Nov 29, 2025

Based on a suggestion by @KurtE changed the virtual fgets function call from

virtual int fgets(char* str, int num, char* delim = nullptr) = 0;

to

virtual int fgets(char* str, int num, char* delim = nullptr){ return -1; }

should

Then if littlefs or my dummy classes should rebuild and run with out changes

@ssilverman
Copy link
Contributor

I’m with @A-Dunstan on this one. I don’t think this belongs in File either.

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.

3 participants