-
Notifications
You must be signed in to change notification settings - Fork 5
Add read_dir
to mod_util::Mod
(#232)
#234
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
base: main
Are you sure you want to change the base?
Conversation
initial implementation
* changed name to `read_dir` to mimic fs * Now returns a `Vec<ModEntry>` structs, too dumb atm to figure out an iterator and the difference should be minimal for normal sized dirs * ModEntry has `is_file()` and `is_dir()` helpers, and most other convenient stuff can be grabbed from `path()` * ModEntry can be easily expanded with additional metadata if desired
Notably, the function here returns a let mod: Mod = ...;
let cfg_files = mod.read_dir("locale/en")?.iter()
.filter(|&x| x.is_file() && x.path().extension() == Some("cfg"));
for cfg_file in cfg_files {
let data = mod.read_file(&cfg_file.path())?;
// ...
} The returned value should probably be an iterator instead of a Vec, but I'm too smoothbrained with Rust to be able to do that at the moment. |
pub fn path(&self) -> &PathBuf { | ||
&self.path | ||
} |
Check warning
Code scanning / clippy
this could be a const fn Warning
} | ||
|
||
impl ModEntry { | ||
pub fn path(&self) -> &PathBuf { |
Check warning
Code scanning / clippy
this method could have a #[must_use] attribute Warning
pub fn is_file(&self) -> bool { | ||
self.is_file | ||
} |
Check warning
Code scanning / clippy
this could be a const fn Warning
pub fn path(&self) -> &PathBuf { | ||
&self.path | ||
} | ||
pub fn is_file(&self) -> bool { |
Check warning
Code scanning / clippy
this method could have a #[must_use] attribute Warning
pub fn is_dir(&self) -> bool { | ||
!self.is_file | ||
} |
Check warning
Code scanning / clippy
this could be a const fn Warning
} | ||
|
||
return std::fs::read_dir(&path)? | ||
.filter_map(|x| x.ok()) |
Check warning
Code scanning / clippy
redundant closure Warning
|
||
return std::fs::read_dir(&path)? | ||
.filter_map(|x| x.ok()) | ||
.map(|x| ModEntry::try_from(x)) |
Check warning
Code scanning / clippy
redundant closure Warning
let path = internal_prefix.clone() + dir; | ||
let mut zip = zip.try_borrow_mut()?; | ||
|
||
if let Err(_) = &zip.by_name(&path) { |
Check warning
Code scanning / clippy
redundant pattern matching, consider using is_err() Warning
let entries: Vec<String> = zip | ||
.file_names() | ||
.filter(|&x| x.starts_with(&path) && x != path) | ||
.map(|x| x.into()) |
Check warning
Code scanning / clippy
redundant closure Warning
return entries | ||
.iter() | ||
.map(|x| ModEntry::try_from(&zip.by_name(x)?)) | ||
.collect(); |
Check warning
Code scanning / clippy
unneeded return statement Warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks nice for a start. One thought I have is that instead of returning a Vec<ModEntry>
we could have a new struct like ModDir
that then also implements Iterator
so its usage would be very similar to the regular fs::read_dir
.
The CI test report failing is weird but not a blocker, the clippy lints would be nice to solve tho. |
oh I just read this now, if you're fine with it I'll clean the PR up and change it into the iterator like I proposed. |
Implements the proposed function. Needs testing, however.