-
Notifications
You must be signed in to change notification settings - Fork 65
file.list works for directories, shows timestamp information as epoch for create,access,modified,changed #2376
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?
Changes from 5 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,15 @@ use nix::unistd::{Gid, Group, Uid, User}; | |
| use std::fs; | ||
| #[cfg(feature = "stdlib")] | ||
| use std::path::Path; | ||
| #[cfg(feature = "stdlib")] | ||
| use std::sync::Arc; | ||
| #[cfg(feature="stdlib")] | ||
| use spin::RwLock; | ||
| #[cfg(feature = "stdlib")] | ||
| use std::time::UNIX_EPOCH; | ||
|
|
||
| #[cfg(feature = "stdlib")] | ||
| pub fn list(path: Option<String>) -> Result<Vec<BTreeMap<String, Value>>, String> { | ||
| pub fn list(path: Option<String>, dir_self: Option<bool>) -> Result<Vec<BTreeMap<String, Value>>, String> { | ||
| let path = path.unwrap_or_else(|| { | ||
| ::std::env::current_dir() | ||
| .map(|p| p.to_string_lossy().to_string()) | ||
|
|
@@ -30,12 +36,13 @@ pub fn list(path: Option<String>) -> Result<Vec<BTreeMap<String, Value>>, String | |
| } | ||
| }) | ||
| }); | ||
| list_impl(path).map_err(|e| e.to_string()) | ||
| list_impl(path, dir_self.unwrap_or(false)).map_err(|e| e.to_string()) | ||
| } | ||
|
|
||
| #[cfg(not(feature = "stdlib"))] | ||
| pub fn list( | ||
| _path: Option<alloc::string::String>, | ||
| dir_self: Option<bool> | ||
| ) -> Result< | ||
| alloc::vec::Vec<alloc::collections::BTreeMap<alloc::string::String, eldritch_core::Value>>, | ||
| alloc::string::String, | ||
|
|
@@ -44,7 +51,7 @@ pub fn list( | |
| } | ||
|
|
||
| #[cfg(feature = "stdlib")] | ||
| fn list_impl(path: String) -> AnyhowResult<Vec<BTreeMap<String, Value>>> { | ||
| fn list_impl(path: String, dir_self: bool) -> AnyhowResult<Vec<BTreeMap<String, Value>>> { | ||
| use glob::glob; | ||
|
|
||
| let mut final_res = Vec::new(); | ||
|
|
@@ -53,22 +60,66 @@ fn list_impl(path: String) -> AnyhowResult<Vec<BTreeMap<String, Value>>> { | |
| for entry in glob(&path)? { | ||
| match entry { | ||
| Ok(path_buf) => { | ||
| // If I implement `handle_list` roughly: | ||
| // if we're not a directory (file), show self | ||
| // OR | ||
| // if we are a directory, only show self if the flag is set to true | ||
| if !path_buf.is_dir() || dir_self { | ||
| final_res.push(create_dict_from_file(&path_buf)?); | ||
| } | ||
|
|
||
| // for dir, show subcontents | ||
| if path_buf.is_dir() { | ||
| for entry in fs::read_dir(&path_buf)? { | ||
| let entry = entry?; | ||
| final_res.push(create_dict_from_file(&entry.path())?); | ||
| } | ||
| } else { | ||
| final_res.push(create_dict_from_file(&path_buf)?); | ||
| } | ||
| } | ||
| Err(e) => eprintln!("Glob error: {e:?}"), | ||
| } | ||
| } | ||
|
|
||
| // sort by absolute_path | ||
| final_res.sort_by_key(|k| k.get("absolute_path").cloned()); | ||
| Ok(final_res) | ||
| } | ||
|
|
||
| // get the timestamps of a metadata object and return it as a dictionary | ||
| #[cfg(feature = "stdlib")] | ||
| fn get_times_dict(metadata: std::fs::Metadata, mtime: std::io::Result<std::time::SystemTime>) -> Value { | ||
| // create dictionary for times data | ||
| let mut times: BTreeMap<Value, Value> = BTreeMap::new(); | ||
|
|
||
| // add changed time (it's already in epoch format) if we're in unix | ||
| #[cfg(unix)] { | ||
| use std::os::unix::fs::MetadataExt; | ||
| times.insert(Value::String("changed".to_string()), Value::Int(metadata.ctime())); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
File:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixes applied, negative epochs supported for mtime/atime/crtime |
||
|
|
||
| // add time information | ||
| let timestamps = [ | ||
| ("modified", mtime), | ||
| ("created", metadata.created()), | ||
| ("accessed", metadata.accessed()) | ||
| ]; | ||
| for timestamp_req in timestamps { | ||
| // if getting the timestamp was successful, add it | ||
| if let Ok(timestamp) = timestamp_req.1 { | ||
| // convert timestamp to epoch | ||
| let secs = match timestamp.duration_since(UNIX_EPOCH) { | ||
| Ok(duration) => duration.as_secs() as i64, | ||
| Err(err) => -(err.duration().as_secs() as i64) | ||
| }; | ||
|
|
||
| // add timestamp to dict | ||
| times.insert(Value::String(timestamp_req.0.to_string()), Value::Int(secs)); | ||
| } | ||
| }; | ||
|
|
||
| // insert times section to the dictionary | ||
| return Value::Dictionary(Arc::new(RwLock::new(times))); | ||
| } | ||
|
|
||
| #[cfg(feature = "stdlib")] | ||
| fn create_dict_from_file(path: &Path) -> AnyhowResult<BTreeMap<String, Value>> { | ||
| use alloc::format; | ||
|
|
@@ -137,13 +188,19 @@ fn create_dict_from_file(path: &Path) -> AnyhowResult<BTreeMap<String, Value>> { | |
| Value::String(abs_path.to_string_lossy().to_string()), | ||
| ); | ||
|
|
||
| // Times | ||
| if let Ok(modified) = metadata.modified() { | ||
| // cache modified time, then add it to both epoch and as a stringified version | ||
| let mtime = metadata.modified(); | ||
|
|
||
| // turn modified time into a stringified version | ||
| if let Ok(modified) = mtime { | ||
| let dt: chrono::DateTime<chrono::Utc> = modified.into(); | ||
| let formatted = dt.format("%Y-%m-%d %H:%M:%S UTC").to_string(); | ||
| dict.insert("modified".to_string(), Value::String(formatted)); | ||
| } | ||
|
|
||
| // Add Time information | ||
| dict.insert("times".to_string(), get_times_dict(metadata, mtime)); | ||
|
|
||
| Ok(dict) | ||
| } | ||
|
|
||
|
|
@@ -154,18 +211,38 @@ mod tests { | |
| use regex::bytes::Regex; | ||
| use tempfile::NamedTempFile; | ||
|
|
||
| #[test] | ||
| fn test_list_includes_self_when_dir() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| std::fs::write(dir.path().join("a.txt"), b"hi").unwrap(); | ||
| let files = list(Some(dir.path().to_string_lossy().to_string()), Some(true)).unwrap(); | ||
| assert!(files.len() >= 2); // self + child | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_list_owner_group() { | ||
| let tmp = NamedTempFile::new().unwrap(); | ||
| let path = tmp.path().to_string_lossy().to_string(); | ||
|
|
||
| let files = list(Some(path)).unwrap(); | ||
| let files = list(Some(path), None).unwrap(); | ||
| assert_eq!(files.len(), 1); | ||
| let f = &files[0]; | ||
|
|
||
| assert!(f.contains_key("owner")); | ||
| assert!(f.contains_key("group")); | ||
| assert!(f.contains_key("absolute_path")); | ||
| assert!(f.contains_key("times")); | ||
|
AmeliaYeah marked this conversation as resolved.
|
||
| // check times sub-dict | ||
| if let Value::Dictionary(d) = &f["times"] { | ||
| let inner = d.read(); | ||
| assert!(inner.contains_key(&Value::String("modified".into()))); | ||
| assert!(inner.contains_key(&Value::String("accessed".into()))); | ||
| assert!(inner.contains_key(&Value::String("created".into()))); | ||
| #[cfg(unix)] | ||
| assert!(inner.contains_key(&Value::String("changed".into()))); | ||
| } | ||
|
|
||
| // check modified string | ||
| assert!(f.contains_key("modified")); | ||
|
|
||
| // Check absolute_path | ||
|
|
@@ -176,7 +253,7 @@ mod tests { | |
| panic!("absolute_path is not a string"); | ||
| } | ||
|
|
||
| // Check modified time format | ||
| //Check modified time format | ||
| if let Value::String(mod_time) = &f["modified"] { | ||
| // Check format YYYY-MM-DD HH:MM:SS UTC | ||
| let re = Regex::new(r"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} UTC$").unwrap(); | ||
|
|
||
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.
Does dir_self only show the local dir or optionally include it?
If only itself - can you name it only_dir_self to be more clear.
If optionally include the local dir - i think we should include that by default similar to
lsThere 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.
Okay seems like it optionally includes the local dir. I think we should always include this.
Uh oh!
There was an error while loading. Please reload this page.
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.
yeah i feel dir_self is good for backwards compatability, it's optional because in tomes prior to this change (where only
pathwas the parameter), they obviously won't include the param and thus it'll function basically the same to how it did prior.i wanted to include it by default but felt it might be better this way to prevent potential annoyances.
Uh oh!
There was an error while loading. Please reload this page.
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.
in hindsight, i'm not really sure what tomes are going to be hurt by it being included. i kind of originally made it so the dir was included by default but only added this after suggestions from @jabbate19 above.
🤷 idk, i suppose it doesn't hurt, i could include it by default/maybe remove the param for the sake of the function looking more polished tho