isDir always false, so code has no effect

[brownrb]

in FSBrowser.ino

Line 195 sets isDir false
bool isDir = false;

then a test in line 197 tests the state of isDir which will always be false
output += (isDir) ? "dir" : "file"

Nowhere is the value of isDir changed, so this code has an error.

[Jeroen88]

Maybe bool isDir = dir.isDirectory() will work

[francescolavra]

Indeed the line:

bool isDir = false;

should be changed to:

bool isDir = entry.isDirectory();

(The OP is talking about the ESP8266WebServer example sketch at https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WebServer/examples/FSBrowser/FSBrowser.ino)

[brownrb]

This fails for ESP8266 because entry.isDirectory() is invalid for ESP8266

[francescolavra]

entry.isDirectory() returns always false if you use SPIFFS (because of a limitation in SPIFFS), but should work if you use LittleFS.

[earlephilhower]

@brownrb would you like to do a PR with this small change, since you found it?

[brownrb]

What is a PR

[d-a-v]

Pull Request on github
Merge Request elsewhere
It's a patch proposal

[earlephilhower]

@brownrb , could you generate a pull request with the fix? You've found the bug and solution, and it would be a good way to get your name in the commit logs for posterity. :)

[brownrb]

What is a pull request? On Monday, February 10, 2020, 06:19:56 AM GMT+13, Earle F. Philhower, III <notifications@github.com> wrote: @brownrb , could you generate a pull request with the fix? You've found the bug and solution, and it would be a good way to get your name in the commit logs for posterity. :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[earlephilhower]

A quick test with LittleFS and directories shows the fix to be more complicated than it looks.

Replacing with isDir = entry.isDirectory() does result in the specific results working (i.e. the JSON this pseudi-cgi generates will contain 'dir's), but the actual web page JavaScript (minified so it's not human readable) doesn't know what to do with an entry of type 'dir' so simply drops directories from the list.

So, the fix needs to also incorporate a new .js file that can handle subdirs. It's a can of worms. :(

[vdeconinck]

If nobody is working on this, I can try to fix it because this FSBrowser is a little gem I include in all my projects and I'm also facing issues migrating to LittleFS due to it.
So unless there's already effort going on this topic, I can try to unminify the code and take a look at it (don't hold your breath though)
@me-no-dev , seems you were the original author of this sample. Would you by chance still have the original JS source ?

[earlephilhower]

@vdeconinck that would be great!

I'm curious, though, if it is worth the effort. We might want to drop this and just have the SDFSBrowser instead. I think I've hacked it personally with just a few changes to run on LittleFS (since SD and LittleFS and SPIFFS now have the exact same API, it's almost just a matter of changing the startup code to call LittleFS.begin())...

[vdeconinck]

@earlephilhower Ahem I can't seem to find SDFSBrowser anywhere (searched my Arduino install folder, searched Github, even asked Google to no avail). Care to point me to that file ?
In the meantime, I checked me-no-dev's alternate version included in the ESPAsyncWebServer project which is a bit more recent, but it is even more obfuscated as the edit.htm.gz file is encoded as an array in the C source, https://github.com/me-no-dev/ESPAsyncWebServer/blob/master/src/SPIFFSEditor.cpp - Yay ;-)

[earlephilhower]

Sorry, had the wrong name. It's this: https://github.com/esp8266/Arduino/tree/master/libraries/ESP8266WebServer/examples/SDWebServer

If you change the SD.begin() with LittleFS.begin() it almost works (I think I had 1 more change somewhere), and has subdir support.

[vdeconinck]

Thanks. I played a bit with the SDWebServer example I had completely overlooked till now, and modifications to make it work with LittleFS are minor indeed, so I would advise the OP to switch to that version:
@brownrb , starting from SDWebServer, replace all occurrences of "SD." by "LittleFS.", remove the pin parameter to begin(), add a "r" parameter to single-parameter versions of open(), and replace FILE_WRITE by "w", and you should be good)
@earlephilhower Do you think it would be welcome to try and make that code a "FS-agnostic" file manager, with a #define logic to select the filesystem type ? Probably the naming of the example should be changed too... Just tell me.