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

cli: Display datetimes in local timezone #561

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

vimpostor
Copy link
Contributor

@vimpostor vimpostor commented Sep 27, 2023

This makes up for a much nicer UX for users of the cli utility.
Before this patch the start and end datetimes were always displayed with a fixed timezone (the timezone of the calendar creator).
This meant that users had to manually calculate the time in their own timezone.

With this patch the times are printed in the expected format, i.e. in the user's own local timezone.

It seems that in 7a8d584 an attempt was already made to implement this, but unfortunately start.astimezone(start.tzinfo) is a no-op and does absolutely nothing. Instead no arguments have to be passed to the astimezone() function to convert to the user's timezone, see https://docs.python.org/3/library/datetime.html#datetime.datetime.astimezone

The unit test has been adapted to check for the output in the correct timezone.

@vimpostor vimpostor force-pushed the localdatetime branch 3 times, most recently from 0e753c2 to 2abd60d Compare September 27, 2023 20:38
Copy link
Member

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

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

The changes look good to me. I am wondering though: It never mentions the time zone. How is this tool expected to work: should you always see the times in the local time zone? Should you see it in the event's time zone? What if there are many time zones - then it does not make sense to see e.g. 19:00 as it could mean a lot.

CHANGES.rst Show resolved Hide resolved
Previously the start and end datetimes were always printed out in the
timezone that they appear in the calendar entry.

In 7a8d584 duration support was added and an attempt was already made
to display the datetime in the local timezone. Unfortunately in that
specific case the `start.astimezone(start.tzinfo)` is a no-op and does
absolutely nothing. Unlike the name suggests, `astimezone()` adjusts the
date and time data, such that they match the passed tzinfo, but since
that is the same timezone data as before, nothing changes. [0]

In order to properly convert to the user's local timezone, we need to
call the method with no arguments.

With this the timezone is always properly displayed, which makes up for
a much nicer UX for users of the cli.

The test has to be adapted to expect the datetime in the local timezone,
hence we cannot hardcode the entire expected string anymore.

[0] https://docs.python.org/3/library/datetime.html#datetime.datetime.astimezone
@vimpostor
Copy link
Contributor Author

vimpostor commented Sep 28, 2023

I am wondering though: It never mentions the time zone. How is this tool expected to work: should you always see the times in the local time zone?

Yes

Should you see it in the event's time zone?

No, that wouldn't make any sense. If someone shares their calendar in Google Calendar with me, I expect Google Calendar to show me all events with the time of my own timezone, not the timezone of the other person.
Similarly, the CLI tool should show the time always in the timezone of the user.

What if there are many time zones - then it does not make sense to see e.g. 19:00 as it could mean a lot.

I don't know what you mean with many time zones, the time is always displayed in one timezone, the local timezone of the user. Theoretically we could append the timezone description to the time, but that would be pretty unusual. Usually you only add the timezone description, if it is not already in the local timezone of the user.

By the way before this patch the CLI tool also never showed the timezone info, it just displayed no timezone despite the timezone of the displayed time being not the local timezone. That was pretty surprising behaviour.

Just as an example, to make sure we are on the same page:
Imagine you live in the Europe/Berlin timezone and some other person lives in the America/Recife timezone. This other person now shares a calendar with you with an event starting at 9:30 AM in the other person's timezone.
This means that the raw .ics file contains something like: DTSTART;TZID=America/Recife:20231002T093000.

Now before this patch, the CLI tool would show this for the user living in the Europe/Berlin timezone:

Starts     : Mon Oct  2 09:30:00 2023

Note how there is no timezone info, so the user would mistakenly assume that the event starts at 9:30 in their own timezone (since they don't know what the timezone of the other person is). But in reality the event starts 14:30 in their own timezone.

Now after this patch, the CLI tool outputs the expected:

Starts     : Mon Oct  2 14:30:00 2023

Which is the correct time in the user's timezone.

@niccokunzmann
Copy link
Member

This makes sense to me, too. Thanks for providing this!

@niccokunzmann niccokunzmann merged commit 0fdbdfb into collective:master Sep 29, 2023
9 checks passed
@vimpostor
Copy link
Contributor Author

vimpostor commented Sep 29, 2023

And in case you were wondering where the hell people are using the icalendar CLI tool in "production", many people use it in CLI mail clients (e.g. mutt) to display sent calendar events inline directly in the mail without having to leave the terminal.

That's also why the local timezone makes sense.

@niccokunzmann
Copy link
Member

This is live in v5.0.11. Thanks for your contributions!

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

Successfully merging this pull request may close these issues.

None yet

2 participants