Skip to content

(exa PR) 867: Replace datetime and zoneinfo_compiled crates with chrono#58

Closed
cafkafk wants to merge 1 commit into
mainfrom
pr-867
Closed

(exa PR) 867: Replace datetime and zoneinfo_compiled crates with chrono#58
cafkafk wants to merge 1 commit into
mainfrom
pr-867

Conversation

@cafkafk

@cafkafk cafkafk commented Jul 29, 2023

Copy link
Copy Markdown
Member

@cafkafk cafkafk changed the title (exa PR) 867 (exa PR) 867: Replace datetime and zoneinfo_compiled crates with chrono Jul 29, 2023
@sbatial sbatial mentioned this pull request Jul 30, 2023
63 tasks
@cafkafk cafkafk added this to the exa pulls done milestone Jul 31, 2023
@cafkafk

cafkafk commented Aug 4, 2023

Copy link
Copy Markdown
Member Author

By now, integrating these changes is probably harder, as we have added relative time and other such features. I'm not (currently) up for this, I think my time is better spend elsewhere, but if anyone wants to tackle this it would be hugely appreciated.

@ariasuni

Copy link
Copy Markdown
Contributor

Yeah, I think #60 should be closed in favor of this PR, which fixes the issues we had with the previous code properly.

I’ve updated the code and fixed the Clippy lints. It not only fixes bug, but also reduce a bit the code: if we ignore the Cargo.lock and look only at the .rs files, we get: 5 files changed, 125 insertions(+), 272 deletions(-).

Before I updated this code for eza (which was pretty trivial), this PR passed the xtests. I’ll try to re-run xtests on it to check that nothing was broken.

@ariasuni ariasuni self-assigned this Aug 31, 2023
@cafkafk

cafkafk commented Sep 1, 2023

Copy link
Copy Markdown
Member Author

Yeah, I think #60 should be closed in favor of this PR, which fixes the issues we had with the previous code properly.

Done. Ping me when this is ready for review.

- Improve compatibility with other OSes, timezone are handled for us
- Reduce significantly the code to render date and time
@ariasuni

ariasuni commented Sep 6, 2023

Copy link
Copy Markdown
Contributor

It looks good to me, however note that timeago considers the future as now, as far as files are concerned, though it’s a weird edge case, the important thing is that the absolute display doesn’t crash eza and is correct.

$ cargo run -- -l test_root/far-dates/
    Finished dev [unoptimized] target(s) in 0.08s
     Running `target/debug/eza -l test_root/far-dates/`
.rw-r--r-- 0 ariasuni  1 janv.  2300 beyond-the-future
.rw-r--r-- 0 ariasuni 13 déc.   1901 the-distant-past

$ ls -l test_root/far-dates/
total 0
-rw-r--r-- 1 ariasuni ariasuni 0  1 janv.  2300 beyond-the-future
-rw-r--r-- 1 ariasuni ariasuni 0 13 déc.   1901 the-distant-past

$ cargo run -- -l --time-style=relative test_root/far-dates/
    Finished dev [unoptimized] target(s) in 0.07s
     Running `target/debug/eza -l --time-style=relative test_root/far-dates/`
.rw-r--r-- 0 ariasuni now       beyond-the-future
.rw-r--r-- 0 ariasuni 121 years the-distant-past

@daviessm

daviessm commented Sep 7, 2023

Copy link
Copy Markdown
Contributor

I don't seem to have access to push to this repo/branch but this patch should fix the Windows build and its timezone issues (tested by @agrmohit and myself):

diff --git a/src/fs/file.rs b/src/fs/file.rs
index e282f65..9ebddee 100644
--- a/src/fs/file.rs
+++ b/src/fs/file.rs
@@ -528,7 +528,7 @@ impl<'dir> File<'dir> {
     }
 
     #[cfg(windows)]
-    pub fn changed_time(&self) -> Option<SystemTime> {
+    pub fn changed_time(&self) -> Option<NaiveDateTime> {
         self.modified_time()
     }
 
diff --git a/src/output/table.rs b/src/output/table.rs
index d7d39ea..95f73b7 100644
--- a/src/output/table.rs
+++ b/src/output/table.rs
@@ -357,36 +357,10 @@ impl Environment {
         #[cfg(unix)]
         let users = Mutex::new(UsersCache::new());
 
-        Self { time_offset, numeric, users }
+        Self { time_offset, numeric, #[cfg(unix)] users }
     }
 }
 
-#[allow(clippy::unnecessary_wraps)] // Needs to match Unix function
-#[cfg(windows)]
-fn determine_time_zone() -> TZResult<TimeZone> {
-    use datetime::zone::{FixedTimespan, FixedTimespanSet, StaticTimeZone, TimeZoneSource};
-    use std::borrow::Cow;
-
-    Ok(TimeZone(TimeZoneSource::Static(&StaticTimeZone {
-        name: "Unsupported",
-        fixed_timespans: FixedTimespanSet {
-            first: FixedTimespan {
-                offset: 0,
-                is_dst: false,
-                name: Cow::Borrowed("ZONE_A"),
-            },
-            rest: &[(
-                1_206_838_800, // Sun Mar 30 2008 01:00:00 GMT+0000
-                FixedTimespan {
-                    offset: 3600,
-                    is_dst: false,
-                    name: Cow::Borrowed("ZONE_B"),
-                },
-            )],
-        },
-    })))
-}
-
 lazy_static! {
     static ref ENVIRONMENT: Environment = Environment::load_all();
 }

@cafkafk

cafkafk commented Sep 8, 2023

Copy link
Copy Markdown
Member Author

I don't seem to have access to push to this repo/branch but this patch should fix the Windows build and its timezone issues (tested by @agrmohit and myself):

Maybe you can create a new PR for this? It makes it hard to review when I'm also the author.

@ariasuni

ariasuni commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

I’m gonna create a new PR and take your changes to fix the build on Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants