Skip to content
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

feat(time_display): time should use /etc/timezone #721

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

erwinvaneijk
Copy link
Contributor

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.

@erwinvaneijk
Copy link
Contributor Author

I wasn't able to fix the commit message when it was pushed to github, so created a new PR. Sorry about that.

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.
Signed-off-by: Erwin <235739+erwinvaneijk@users.noreply.github.com>
@cafkafk
Copy link
Member

cafkafk commented Feb 12, 2024

I wasn't able to fix the commit message when it was pushed to github, so created a new PR. Sorry about that.

Does this mean this PR should be closed?

@erwinvaneijk
Copy link
Contributor Author

No, I already closed the broken branch .

Signed-off-by: Erwin <235739+erwinvaneijk@users.noreply.github.com>
Nix doesn't seem to have /etc/localtime (or equivalent), which
iana-time-zone doesn't like, as it depends on it.
Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to test this with libfaketime and it didn't seem to actually change the dates to DST, how did you test this?

@erwinvaneijk
Copy link
Contributor Author

erwinvaneijk commented Mar 3, 2024

I tried to test this with libfaketime and it didn't seem to actually change the dates to DST, how did you test this?

libfaketime fakes the time of the machine, but that's not really what should be changed. It should interpret the UTC time in the timestamp to the (DST or not) timezone for the current timezone at the time of creation of the file. What I found hard is that nix apparently doesn't set the current timezone in a way that chrono_tz knows about.

I've used the following sequence to create a testcase:

% touch -d "2023-02-04 10:35:00" bar
% touch -d "2023-06-04 10:35:00" baz
% touch -d "2023-11-12 10:35:00" foo

When using ls (from GNU coreutils):

% ls -al --full-time ~/tmp/tz
-rw-r--r--  1 user staff   0 2023-02-04 10:35:00.000000000 +0100 bar
-rw-r--r--  1 user staff   0 2023-06-04 10:35:00.000000000 +0200 baz
-rw-r--r--  1 user staff   0 2023-11-12 10:35:00.000000000 +0100 foo

When using eza from main branch:

% ./target/debug/eza -l --time-style full-iso ~/tmp/tz
.rw-r--r-- 0 user 2023-02-04 10:35:00.000000000 +0100 bar
.rw-r--r-- 0 user 2023-06-04 09:35:00.000000000 +0100 baz
.rw-r--r-- 0 user 2023-11-12 10:35:00.000000000 +0100 foo

with the updated eza from PR:

% ./target/debug/eza -l --time-style full-iso ~/tmp/tz
.rw-r--r-- 0 user 2023-02-04 10:35:00.000000000 +0100 bar
.rw-r--r-- 0 user 2023-06-04 10:35:00.000000000 +0200 baz
.rw-r--r-- 0 user 2023-11-12 10:35:00.000000000 +0100 foo

And, without the updated code the test in src/output/time.rs:195 would fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants