SPIFFS: name() and fullname() both return full pathname - doc specifies differently

[ewaldc]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: ESP-12
• Core Version: latest git
• Development Env: Platformio
• Operating System: Windows

Problem Description

According to the documentation

name()

should return a short (no-path) file name whereas

fullname()

should return the full path file name

On LittleFS this works properly but on SPIFFS both functions return the full name (including path). Looking at the source code, both functions indeed just return _stat.name in the case of SPIFFS, whereas in LittleFS there is code to lookup the last / character and return the name past this /.

I know that SPIFFS does not truly support directories, but it supports the notion of directories as in traversing all files with a common path name. Hence I was expecting name() to return the short file name.

PS. This is a low priority question/request since it's very easy to get desired result is user code.

[earlephilhower]

The current behavior is backwards compatible with the (IMO) broken original definition. I'm not so sure it can be changed, even though I see your point emphatically.

The 8266 File class never had real directory support, so before LittleFS() there was no such thing as fullname() and name() always gave the full string of chars SPIFFS used as the filename (i.e. it could be \this/is ^DA\nfile and SPIFFS wouldn't care, because there are no special chars in SPIFFS filenames except for NUL \0.

If we make this work as one would expect, matching the new LittleFS and the new SD, then people that rely on name() on SPIFFS may end up generating multiple copies of files by accident in their code.

The problem is, /file.txt and file.txt are two separate files. So if you opened \file.txt but write File.name()(==file.txt) then it would get weird, very fast.

@devyte , @d-a-v , thoughts? Is this a 3.0 or something that we should make sane now, damn backwards (broken) compat?

[devyte]

I'd say it's a matter of definition. SPIFFS doesn't support dirs, so a / is considered part of the filename.
I suppose we could sort of emulate dir behavior by defining e. g. / as a dir separator for name handling (manipulate the filename string) , but it would be a half measure at best, because I'm not sure full dir support is possible on our end (e. g. dir iteration).

Tbh, this whole lack of dir in SPIFFS has been an annoyance for a long while, but I don't see it as a priority for now. At this time, a user is supposed to stick to SPIFFS, unless one of the SPIFFS issues is encountered in his app, at which point he should try switching to LittleFS. Having both filesystems coexist in an app, or have a lib that supports both simultaneously, isn't something that has been considered so far, given how new LittleFS is.

[ewaldc]

Thanks for a fast response. This is wonderful library and 100% agreed this is a (very) low priority thing as it takes just one single line of user code to get the desired behavior. My primary goal for filing the request is to help make this library even better.
My thoughts:

• if the desired SPIFFS behavior is to have name() == fullname() than at some point in time it would be good to reflect that in the documentation, which now suggest the behavior for LittleFS and SPIFFS is the same for these functions
• I can not totally agree with

SPIFFS doesn't support dirs

It's implementation does not have a physical directories, but the API defines logical directories and has several specific directory functions. The code defines a SPIFFS_TYPE_DIR and refers many times to the term directory. It seems to me that in the original implementation there is no notion of a file name, but rather a path name, and thus there are no name/fullname() functions. The ESP implementation in it's great attempt to unify the LttleFS and SPIFFS API's, seems to have inadvertently progressed from a path name to a path + file name where one expects / to have special meaning (but which it does not in the code), hence the provision for separate name()/fullname() functions.

In summary, my humble view is:

• if the goal is to have LittleFS and SPIFFS be as API-compatible as possible, maybe at some time (v3?) it would be good to make the behavior the same. It might break some code, but that would mean the developer did not read the documentation (which clearly says name() != fullname()), but even then, the fix would be very easy: replace name() with fullname()
• if not breaking SPIFFS code is prime, it would be good to update the documentation to say that for SPIFFS name() == fullname()

I am happy to provide the code changes and/or help to modify the doc.
Hope this makes sense

[devyte]

You seem to have a solution, so how about we start with a PR for your code changes, and we discuss further there?

[ewaldc]

Excellent idea. The code change is 2 lines and is leveraged from the LittleFS code.
Happy to post a PR, will reference issue #.

[ewaldc]

I implemented and tested the change (it's a one liner). It does indeed break a lot of code and samples that I found all over the web and Github. Unfortunately there is a lot of use of name() instead of fullname(). I've come to realize that you were right: it's not worth to break a lot of code for something that just takes one line in user code to fix. I guess, the difference between "being right" and "doing the right thing".

[devyte]

@ewaldc this could be targeted for core release 3.0.0. It's ok to implement changes that break things in a major release, if there's good reason.
I still haven't seen your pr, so I can't tell if your solution is worth pursuing.

[earlephilhower]

Thanks for the PR. Let's leave this issue open until it's merged. As @devyte was saying, since it's a breaking change we need to schedule it for a major version bump, so it may be a while before merging.