From efe1577e285e7850ce838ccecec02369202d35eb Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:37:48 +0100 Subject: [PATCH 1/9] fix: make time use timezone information When a file timestamp is displayed, it should use the timezone info from the time of the timestamp, not from when it is displayed. This occurs when a file timestamp is updated under daylight saving time, and displayed at a time when not under daylight saving time. This bug is quite subtle, as it depends on the time of construction of the timestamp, not on the moment it is read. --- Cargo.lock | 42 ++++++++++++++++++++++++++++++++++++++ Cargo.toml | 2 ++ src/output/render/times.rs | 21 +++++++++++++++---- src/output/table.rs | 7 ------- src/output/time.rs | 30 +++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1334ec3d4..dcf841d61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -211,6 +211,27 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "chrono-tz" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd6dd8046d00723a59a2f8c5f295c515b9bb9a331ee4f8f3d4dd49e428acd3b6" +dependencies = [ + "chrono", + "chrono-tz-build", + "phf", +] + +[[package]] +name = "chrono-tz-build" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e94fea34d77a245229e7746bd2beb786cd2a896f306ff491fb8cecb3074b10a7" +dependencies = [ + "parse-zoneinfo", + "phf_codegen", +] + [[package]] name = "ciborium" version = "0.2.2" @@ -438,10 +459,12 @@ dependencies = [ "ansi-width", "backtrace", "chrono", + "chrono-tz", "criterion", "dirs", "git2", "glob", + "iana-time-zone", "libc", "locale", "log", @@ -1008,6 +1031,15 @@ dependencies = [ "syn", ] +[[package]] +name = "parse-zoneinfo" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f2a05b18d44e2957b88f96ba460715e295bc1d7510468a2f3d3b44535d26c24" +dependencies = [ + "regex", +] + [[package]] name = "partition-identity" version = "0.3.0" @@ -1039,6 +1071,16 @@ dependencies = [ "phf_shared", ] +[[package]] +name = "phf_codegen" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aef8048c789fa5e851558d709946d6d79a8ff88c0440c587967f8e94bfb1216a" +dependencies = [ + "phf_generator", + "phf_shared", +] + [[package]] name = "phf_generator" version = "0.11.3" diff --git a/Cargo.toml b/Cargo.toml index 68bf0414a..ff15999b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,6 +86,8 @@ name = "eza" [dependencies] rayon = "1.10.0" chrono = { version = "0.4.34", default-features = false, features = ["clock"] } +chrono-tz = "0.10.0" +iana-time-zone = "0.1.58" nu-ansi-term = { version = "0.50.1", features = [ "serde", "derive_serde_style", diff --git a/src/output/render/times.rs b/src/output/render/times.rs index 88b7c1efb..814a5a3ce 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -11,12 +11,26 @@ use chrono::prelude::*; use nu_ansi_term::Style; pub trait Render { - fn render(self, style: Style, time_offset: FixedOffset, time_format: TimeFormat) -> TextCell; + fn render(self, style: Style, time_format: TimeFormat) -> TextCell; } impl Render for Option { - fn render(self, style: Style, time_offset: FixedOffset, time_format: TimeFormat) -> TextCell { - let datestamp = if let Some(time) = self { + fn render(self, style: Style, time_format: TimeFormat) -> TextCell { + let datestamp = if let Ok(timezone_str) = iana_time_zone::get_timezone() { + let timezone: chrono_tz::Tz = timezone_str.parse().unwrap(); + if let Some(time) = self { + let time_offset = timezone.offset_from_utc_datetime(&time).fix(); + time_format.format(&DateTime::::from_naive_utc_and_offset( + time, + time_offset, + )) + } else { + String::from("-") + } + } else if let Some(time) = self { + // This is the next best thing, use the timezone now, instead of at the time of the + // timestamp. + let time_offset: FixedOffset = *Local::now().offset(); time_format.format(&DateTime::::from_naive_utc_and_offset( time, time_offset, @@ -24,7 +38,6 @@ impl Render for Option { } else { String::from("-") }; - TextCell::paint(style, datestamp) } } diff --git a/src/output/table.rs b/src/output/table.rs index 83082c5db..d6709bb5c 100644 --- a/src/output/table.rs +++ b/src/output/table.rs @@ -363,9 +363,6 @@ impl Default for TimeTypes { /// /// Any environment field should be able to be mocked up for test runs. pub struct Environment { - /// The computer’s current time offset, determined from time zone. - time_offset: FixedOffset, - /// Localisation rules for formatting numbers. numeric: locale::Numeric, @@ -381,8 +378,6 @@ impl Environment { } fn load_all() -> Self { - let time_offset = *Local::now().offset(); - let numeric = locale::Numeric::load_user_locale().unwrap_or_else(|_| locale::Numeric::english()); @@ -390,7 +385,6 @@ impl Environment { let users = Mutex::new(UsersCache::new()); Self { - time_offset, numeric, #[cfg(unix)] users, @@ -568,7 +562,6 @@ impl<'a> Table<'a> { } else { self.theme.ui.date.unwrap_or_default() }, - self.env.time_offset, self.time_format.clone(), ), } diff --git a/src/output/time.rs b/src/output/time.rs index d1bc6ea5f..a0ad113ac 100644 --- a/src/output/time.rs +++ b/src/output/time.rs @@ -196,4 +196,34 @@ mod test { .all(|string| UnicodeWidthStr::width(string.as_str()) == max_month_width) ); } + + #[test] + fn display_timestamp_correctly_in_timezone_with_dst() { + let timezone_str = "Europe/Berlin"; + let timezone: chrono_tz::Tz = timezone_str.parse().unwrap(); + let time = DateTime::::from_timestamp(1685867700, 0) + .unwrap() + .naive_utc(); + + let fixed_offset = timezone.offset_from_utc_datetime(&time).fix(); + let real_converted = DateTime::::from_naive_utc_and_offset(time, fixed_offset); + let formatted = full(&real_converted); + let expected = "2023-06-04 10:35:00.000000000 +0200"; + assert_eq!(expected, formatted); + } + + #[test] + fn display_timestamp_correctly_in_timezone_without_dst() { + let timezone_str = "Europe/Berlin"; + let timezone: chrono_tz::Tz = timezone_str.parse().unwrap(); + let time = DateTime::::from_timestamp(1699090500, 0) + .unwrap() + .naive_utc(); + + let fixed_offset = timezone.offset_from_utc_datetime(&time).fix(); + let real_converted = DateTime::::from_naive_utc_and_offset(time, fixed_offset); + let formatted = full(&real_converted); + let expected = "2023-11-04 10:35:00.000000000 +0100"; + assert_eq!(expected, formatted); + } } From eb39e2257dcb291184789d9fc262b5e119562e1e Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:28:23 +0100 Subject: [PATCH 2/9] chore: fix FreeBSD induced errors - lifetimes that can be elided - not use return as last statement in function --- src/fs/dir.rs | 4 ++-- src/fs/feature/xattr.rs | 2 +- src/fs/file.rs | 2 +- src/info/sources.rs | 2 +- src/options/parser.rs | 6 +++--- src/output/details.rs | 2 +- src/output/file_name.rs | 2 +- src/output/grid.rs | 2 +- src/theme/lsc.rs | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/fs/dir.rs b/src/fs/dir.rs index 98dafa33e..0f76f648b 100644 --- a/src/fs/dir.rs +++ b/src/fs/dir.rs @@ -108,7 +108,7 @@ pub struct Files<'dir, 'ig> { total_size: bool, } -impl<'dir, 'ig> Files<'dir, 'ig> { +impl<'dir> Files<'dir, '_> { fn parent(&self) -> PathBuf { // We can’t use `Path#parent` here because all it does is remove the // last path component, which is no good for us if the path is @@ -180,7 +180,7 @@ enum DotsNext { Files, } -impl<'dir, 'ig> Iterator for Files<'dir, 'ig> { +impl<'dir> Iterator for Files<'dir, '_> { type Item = File<'dir>; fn next(&mut self) -> Option { diff --git a/src/fs/feature/xattr.rs b/src/fs/feature/xattr.rs index f05bfaf72..d064d67d1 100644 --- a/src/fs/feature/xattr.rs +++ b/src/fs/feature/xattr.rs @@ -672,7 +672,7 @@ struct BorrowedWriter<'a> { pub buffer: &'a mut Vec, } -impl<'a> io::Write for BorrowedWriter<'a> { +impl io::Write for BorrowedWriter<'_> { fn write(&mut self, buf: &[u8]) -> io::Result { self.buffer.write(buf) } diff --git a/src/fs/file.rs b/src/fs/file.rs index bd181de8b..09c60bdac 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -1007,7 +1007,7 @@ pub enum FileTarget<'dir> { // error — we just display the error message and move on. } -impl<'dir> FileTarget<'dir> { +impl FileTarget<'_> { /// Whether this link doesn’t lead to a file, for whatever reason. This /// gets used to determine how to highlight the link in grid views. pub fn is_broken(&self) -> bool { diff --git a/src/info/sources.rs b/src/info/sources.rs index 4e93a8db2..25f138295 100644 --- a/src/info/sources.rs +++ b/src/info/sources.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; use crate::fs::File; -impl<'a> File<'a> { +impl File<'_> { /// For this file, return a vector of alternate file paths that, if any of /// them exist, mean that *this* file should be coloured as “compiled”. /// diff --git a/src/options/parser.rs b/src/options/parser.rs index 8dbd7edd9..2e694a56d 100644 --- a/src/options/parser.rs +++ b/src/options/parser.rs @@ -399,7 +399,7 @@ pub struct MatchedFlags<'args> { strictness: Strictness, } -impl<'a> MatchedFlags<'a> { +impl MatchedFlags<'_> { /// Whether the given argument was specified. /// Returns `true` if it was, `false` if it wasn’t, and an error in /// strict mode if it was specified more than once. @@ -554,14 +554,14 @@ impl fmt::Display for ParseError { fn os_str_to_bytes(s: &OsStr) -> &[u8] { use std::os::unix::ffi::OsStrExt; - return s.as_bytes(); + s.as_bytes() } #[cfg(unix)] fn bytes_to_os_str(b: &[u8]) -> &OsStr { use std::os::unix::ffi::OsStrExt; - return OsStr::from_bytes(b); + OsStr::from_bytes(b) } #[cfg(windows)] diff --git a/src/output/details.rs b/src/output/details.rs index 343656f45..107991426 100644 --- a/src/output/details.rs +++ b/src/output/details.rs @@ -488,7 +488,7 @@ pub struct TableIter<'a> { tree_trunk: TreeTrunk, } -impl<'a> Iterator for TableIter<'a> { +impl Iterator for TableIter<'_> { type Item = TextCell; fn next(&mut self) -> Option { diff --git a/src/output/file_name.rs b/src/output/file_name.rs index ae232a3d9..6113afd6a 100644 --- a/src/output/file_name.rs +++ b/src/output/file_name.rs @@ -165,7 +165,7 @@ pub struct FileName<'a, 'dir, C> { mount_style: MountStyle, } -impl<'a, 'dir, C> FileName<'a, 'dir, C> { +impl FileName<'_, '_, C> { /// Sets the flag on this file name to display link targets with an /// arrow followed by their path. pub fn with_link_paths(mut self) -> Self { diff --git a/src/output/grid.rs b/src/output/grid.rs index a52df4acc..500d42a95 100644 --- a/src/output/grid.rs +++ b/src/output/grid.rs @@ -37,7 +37,7 @@ pub struct Render<'a> { pub filter: &'a FileFilter, } -impl<'a> Render<'a> { +impl Render<'_> { pub fn render(mut self, w: &mut W) -> io::Result<()> { self.filter.sort_files(&mut self.files); diff --git a/src/theme/lsc.rs b/src/theme/lsc.rs index 5fdd574ad..3fdf58cbf 100644 --- a/src/theme/lsc.rs +++ b/src/theme/lsc.rs @@ -97,7 +97,7 @@ pub struct Pair<'var> { pub value: &'var str, } -impl<'var> Pair<'var> { +impl Pair<'_> { pub fn to_style(&self) -> Style { let mut style = Style::default(); let mut iter = self.value.split(';').peekable(); From 0bace8e8b35953ccf0933fdb6e6f7c8f08607de1 Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:41:38 +0100 Subject: [PATCH 3/9] chore: one more elided error --- src/output/file_name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/file_name.rs b/src/output/file_name.rs index 6113afd6a..f91bf55b5 100644 --- a/src/output/file_name.rs +++ b/src/output/file_name.rs @@ -187,7 +187,7 @@ impl FileName<'_, '_, C> { } } -impl<'a, 'dir, C: Colours> FileName<'a, 'dir, C> { +impl FileName<'_, '_, C> { /// Paints the name of the file using the colours, resulting in a vector /// of coloured cells that can be printed to the terminal. /// From 398255936206f4be421f7c76b4874511f1411ee2 Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:55:15 +0100 Subject: [PATCH 4/9] chore: fix FreeBSD specific compile error --- src/main.rs | 2 +- src/output/render/permissions.rs | 62 +++++++++++++++----------------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/main.rs b/src/main.rs index a235180e7..36dac8112 100644 --- a/src/main.rs +++ b/src/main.rs @@ -248,7 +248,7 @@ fn git_repos(options: &Options, args: &[&OsStr]) -> bool { } } -impl<'args> Exa<'args> { +impl Exa<'_> { /// # Errors /// /// Will return `Err` if printing to stderr fails. diff --git a/src/output/render/permissions.rs b/src/output/render/permissions.rs index 71edd574e..0db9b3b18 100644 --- a/src/output/render/permissions.rs +++ b/src/output/render/permissions.rs @@ -19,50 +19,46 @@ pub trait PermissionsPlusRender { impl PermissionsPlusRender for Option { #[cfg(unix)] fn render(&self, colours: &C) -> TextCell { - match self { - Some(p) => { - let mut chars = vec![p.file_type.render(colours)]; - let permissions = p.permissions; - chars.extend(Some(permissions).render(colours, p.file_type.is_regular_file())); - - if p.xattrs { - chars.push(colours.attribute().paint("@")); - } - - // As these are all ASCII characters, we can guarantee that they’re - // all going to be one character wide, and don’t need to compute the - // cell’s display width. - TextCell { - width: DisplayWidth::from(chars.len()), - contents: chars.into(), - } + if let Some(p) = self { + let mut chars = vec![p.file_type.render(colours)]; + let permissions = p.permissions; + chars.extend(Some(permissions).render(colours, p.file_type.is_regular_file())); + + if p.xattrs { + chars.push(colours.attribute().paint("@")); + } + + // As these are all ASCII characters, we can guarantee that they’re + // all going to be one character wide, and don’t need to compute the + // cell’s display width. + TextCell { + width: DisplayWidth::from(chars.len()), + contents: chars.into(), } - None => { - let chars: Vec<_> = iter::repeat(colours.dash().paint("-")).take(10).collect(); - TextCell { - width: DisplayWidth::from(chars.len()), - contents: chars.into(), - } + } else { + let chars: Vec<_> = iter::repeat(colours.dash().paint("-")).take(10).collect(); + TextCell { + width: DisplayWidth::from(chars.len()), + contents: chars.into(), } } } #[cfg(windows)] fn render(&self, colours: &C) -> TextCell { - match self { - Some(p) => { - let mut chars = vec![p.attributes.render_type(colours)]; - chars.extend(p.attributes.render(colours)); + if let Some(p) = self { + let mut chars = vec![p.attributes.render_type(colours)]; + chars.extend(p.attributes.render(colours)); - TextCell { - width: DisplayWidth::from(chars.len()), - contents: chars.into(), - } + TextCell { + width: DisplayWidth::from(chars.len()), + contents: chars.into(), } - None => TextCell { + } else { + TextCell { width: DisplayWidth::from(0), contents: vec![].into(), - }, + } } } } From c464de3c825f9d40a62d3325b85bb51dd9e7e5bd Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Fri, 10 Jan 2025 19:59:47 +0100 Subject: [PATCH 5/9] chore: fix fmt error --- src/output/render/permissions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/render/permissions.rs b/src/output/render/permissions.rs index 0db9b3b18..23a815ccc 100644 --- a/src/output/render/permissions.rs +++ b/src/output/render/permissions.rs @@ -55,7 +55,7 @@ impl PermissionsPlusRender for Option { contents: chars.into(), } } else { - TextCell { + TextCell { width: DisplayWidth::from(0), contents: vec![].into(), } From 4147572c27b1b9e5a2a1eb6b4a182e3ae24a563d Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Fri, 10 Jan 2025 23:12:59 +0100 Subject: [PATCH 6/9] fix: use UTC if no TZ is found --- src/output/render/times.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/output/render/times.rs b/src/output/render/times.rs index 814a5a3ce..b8440880d 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -8,6 +8,7 @@ use crate::output::cell::TextCell; use crate::output::time::TimeFormat; use chrono::prelude::*; +use chrono_tz::Etc::UTC; use nu_ansi_term::Style; pub trait Render { @@ -17,7 +18,7 @@ pub trait Render { impl Render for Option { fn render(self, style: Style, time_format: TimeFormat) -> TextCell { let datestamp = if let Ok(timezone_str) = iana_time_zone::get_timezone() { - let timezone: chrono_tz::Tz = timezone_str.parse().unwrap(); + let timezone: chrono_tz::Tz = timezone_str.parse().unwrap_or(UTC); if let Some(time) = self { let time_offset = timezone.offset_from_utc_datetime(&time).fix(); time_format.format(&DateTime::::from_naive_utc_and_offset( From a14da5b053c6144c681415aad959a88883460d10 Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Sat, 11 Jan 2025 07:42:35 +0100 Subject: [PATCH 7/9] fix: do not revert to UTC if wrong When an invalid timezone string is obtained, which is really unexpected, we write out a message saying so, instead of using a sensible default. --- src/output/render/times.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/output/render/times.rs b/src/output/render/times.rs index b8440880d..31b4b7778 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -8,7 +8,7 @@ use crate::output::cell::TextCell; use crate::output::time::TimeFormat; use chrono::prelude::*; -use chrono_tz::Etc::UTC; +use chrono_tz::Tz; use nu_ansi_term::Style; pub trait Render { @@ -18,7 +18,9 @@ pub trait Render { impl Render for Option { fn render(self, style: Style, time_format: TimeFormat) -> TextCell { let datestamp = if let Ok(timezone_str) = iana_time_zone::get_timezone() { - let timezone: chrono_tz::Tz = timezone_str.parse().unwrap_or(UTC); + let timezone: Tz = timezone_str + .parse() + .expect(&format!("The timezone cannot be parsed: {}", timezone_str)); if let Some(time) = self { let time_offset = timezone.offset_from_utc_datetime(&time).fix(); time_format.format(&DateTime::::from_naive_utc_and_offset( From dc67d2ba03b69f171388c7f102729d129138f0cf Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Sat, 11 Jan 2025 07:54:18 +0100 Subject: [PATCH 8/9] fix: do not use expect Changed the`expect` for an `unwrap_or_else` to make clippy happy and have descriptive error message. --- src/output/render/times.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/render/times.rs b/src/output/render/times.rs index 31b4b7778..a8463c95e 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -20,7 +20,7 @@ impl Render for Option { let datestamp = if let Ok(timezone_str) = iana_time_zone::get_timezone() { let timezone: Tz = timezone_str .parse() - .expect(&format!("The timezone cannot be parsed: {}", timezone_str)); + .unwrap_or_else(|_| panic!("The timezone cannot be parsed: {}", timezone_str)); if let Some(time) = self { let time_offset = timezone.offset_from_utc_datetime(&time).fix(); time_format.format(&DateTime::::from_naive_utc_and_offset( From 70f333b52cf47f49beb41cecded5df356b6eaa55 Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Sat, 11 Jan 2025 07:58:32 +0100 Subject: [PATCH 9/9] refactor: make clippy happy Clippy wants inline variables in a `format!`. --- src/output/render/times.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/render/times.rs b/src/output/render/times.rs index a8463c95e..4e3bb640a 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -20,7 +20,7 @@ impl Render for Option { let datestamp = if let Ok(timezone_str) = iana_time_zone::get_timezone() { let timezone: Tz = timezone_str .parse() - .unwrap_or_else(|_| panic!("The timezone cannot be parsed: {}", timezone_str)); + .unwrap_or_else(|_| panic!("The timezone cannot be parsed: {timezone_str}")); if let Some(time) = self { let time_offset = timezone.offset_from_utc_datetime(&time).fix(); time_format.format(&DateTime::::from_naive_utc_and_offset(