From 6a3b15ab20f8fb731bc4a11d966b1fa7dd5998e2 Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Sun, 19 Jan 2025 23:55:32 +0100 Subject: [PATCH 1/5] build(deps): add chrono-tz and iana-time-zone dependencies --- Cargo.lock | 42 ++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 2 ++ 2 files changed, 44 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 504ec94a2..1121c8f85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -212,6 +212,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" @@ -439,10 +460,12 @@ dependencies = [ "ansi-width", "backtrace", "chrono", + "chrono-tz", "criterion", "dirs", "git2", "glob", + "iana-time-zone", "libc", "locale", "log", @@ -1009,6 +1032,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" @@ -1040,6 +1072,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 29c58e330..ea4d64c7e 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", From 5cfcfb225545ff345bb33f9e6d9f54ec2090e33e Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Mon, 20 Jan 2025 00:03:27 +0100 Subject: [PATCH 2/5] fix(output): get timezone from file timestamp 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. --- src/output/render/times.rs | 24 ++++++++++++++++++++---- src/output/table.rs | 7 ------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/output/render/times.rs b/src/output/render/times.rs index 88b7c1efb..a8463c95e 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -8,15 +8,32 @@ use crate::output::cell::TextCell; use crate::output::time::TimeFormat; use chrono::prelude::*; +use chrono_tz::Tz; 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: Tz = timezone_str + .parse() + .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( + 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 +41,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(), ), } From 2a7e7d5aa38223e64ec432d7955e4d10fceb1308 Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Mon, 20 Jan 2025 00:05:33 +0100 Subject: [PATCH 3/5] test(output/time): add tests to check timezone output --- src/output/time.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) 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 adda6e57ca09b8772d3314ebb711db0f23cd1b95 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 4/5] fix(clippy): linter issues - 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/main.rs | 2 +- src/options/parser.rs | 6 ++-- src/output/details.rs | 2 +- src/output/file_name.rs | 4 +-- src/output/grid.rs | 2 +- src/output/render/permissions.rs | 62 +++++++++++++++----------------- src/output/render/times.rs | 2 +- src/theme/lsc.rs | 2 +- 12 files changed, 44 insertions(+), 48 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/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/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..f91bf55b5 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 { @@ -187,7 +187,7 @@ impl<'a, 'dir, C> FileName<'a, 'dir, 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. /// 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/output/render/permissions.rs b/src/output/render/permissions.rs index 71edd574e..23a815ccc 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(), - }, + } } } } 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( 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 2e8ebbf91a92994ae874a57870901ee7938eb7fe Mon Sep 17 00:00:00 2001 From: Erwin van Eijk <235739+erwinvaneijk@users.noreply.github.com> Date: Tue, 21 Jan 2025 17:32:11 +0100 Subject: [PATCH 5/5] perf: improve repeated timezone lookups Perform a timezone lookup only once at initialization, assuming that the timezone doesn't change during exection. --- src/output/render/times.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/output/render/times.rs b/src/output/render/times.rs index 4e3bb640a..5b453e97f 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -1,3 +1,5 @@ +use std::sync::OnceLock; + // SPDX-FileCopyrightText: 2024 Christina Sørensen // SPDX-License-Identifier: EUPL-1.2 // @@ -15,12 +17,23 @@ pub trait Render { fn render(self, style: Style, time_format: TimeFormat) -> TextCell; } +// Assume that the timezone is constant for the duration of the program. +static INITIAL_TIMEZONE: OnceLock> = OnceLock::new(); + +fn initialize_timezone() -> Option { + let timezone_str = iana_time_zone::get_timezone(); + timezone_str.map_or(None, |tz_str| { + Some( + tz_str + .parse() + .unwrap_or_else(|_| panic!("The timezone cannot be parsed: {tz_str}")), + ) + }) +} + 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: Tz = timezone_str - .parse() - .unwrap_or_else(|_| panic!("The timezone cannot be parsed: {timezone_str}")); + let datestamp = if let Some(timezone) = INITIAL_TIMEZONE.get_or_init(initialize_timezone) { if let Some(time) = self { let time_offset = timezone.offset_from_utc_datetime(&time).fix(); time_format.format(&DateTime::::from_naive_utc_and_offset(