Improvements #107
Improvements #107
Conversation
| pub(crate) fn first_cluster(&self) -> Option<u32> { | ||
| /// Return first cluster of a entry. | ||
| #[must_use] | ||
| pub fn first_cluster(&self) -> Option<u32> { |
There was a problem hiding this comment.
why make it public? How the user can use this information?
There was a problem hiding this comment.
I use it as a identifier (while a fs is mounted), since it's much simpler than filenames (due to lfn and only Option<u32> to be stored than a full filename - [u8;255]) in my no_std, no_alloc project.
| /// | ||
| /// `path` is a '/' separated directory path relative to self directory. | ||
| /// | ||
| /// An empty path returns the current directory. |
There was a problem hiding this comment.
I'm not sure this is a good way to handle this situation. Do you know any other API that behaves like this? What problem does this change solve?
There was a problem hiding this comment.
It's a side-effect of how the code is now written.
I can add an extra check to return an error in case of an empty string.
As it turns out, I use it no longer - so is (from my point) not neccessary.
src/dir.rs
Outdated
| return Ok(dir); | ||
| } | ||
| let (name, rest_opt) = split_path(path); | ||
| let e = dir.find_entry(name, Some(true), None)?.to_dir(); |
There was a problem hiding this comment.
Name e no longer make sense here. Please remove it and update dir instead
alexkazik
force-pushed
the
improvements
branch
from
ff91e9a to
e93e767
Compare
alexkazik
force-pushed
the
improvements
branch
from
e93e767 to
49010b9
Compare
If you tell me what is controversial, I'll remove it (and maybe add in another PR). (I guess it's the directory routine.) Added to changelog. (P.S. I'd also appreciate it if PR #78 and #98 can also be merged.) |
alexkazik
force-pushed
the
improvements
branch
from
49010b9 to
e7b6f72
Compare
2 participants
Add some small improvements.