feat(pick): show message times on the viewer's clock, not UTC - #55
Merged
Merged
Conversation
The picker sliced HH:MM straight out of the RFC 3339 stamp, so a transcript written in UTC was displayed in UTC with no zone marker. At +05:30 that reads five and a half hours in the past, and since the row shows a clock face and no date, a message from last month looks like one from this evening. It cost me real time twice before I noticed. Times now move to the machine's offset. Only a `Z` stamp is moved; one that already carries an offset is local to whoever wrote it and is shown as written. No date arithmetic is involved, because crossing midnight changes the date, not the clock face. std has no local-time API and this crate deliberately carries no date dependency, so the offset comes from `date +%z`, the same shell-out last::locate already uses, resolved once. Non-unix and anything unparseable stay on UTC, which is the behaviour they have today. The offset is held on App so the drawing test can pin it to 0 and the picker renders identically on any machine.
Contributor
|
Merged. The Z-only conversion guard, the zero-dependency offset with UTC fallback, and pinning the offset in tests made this an easy review. Ships in the next release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The itch
The picker slices
HH:MMstraight out of the RFC 3339 stamp, so a transcript written in UTC is displayed in UTC with no zone marker. I am at +05:30, so every row reads five and a half hours in the past.That is not just cosmetic, because the row shows a clock face and no date. A
19:53from 27 July is indistinguishable from one from this evening. It cost me real time twice while debugging #53: I read a stale session as a live one, and chased the wrong explanation for several minutes before checking the transcript directly.What changed
clocknow moves the time to the machine's UTC offset.Only a
Zstamp is moved. One that already carries an offset (…+05:30) is local to whoever wrote it and is shown as written, so hosts that record local time are unaffected.No date arithmetic is involved: crossing midnight changes the date, not the clock face, so this is
(hh * 60 + mm + offset).rem_euclid(1440)and nothing more.Where the offset comes from
stdhas no local-time API, and this crate deliberately carries no date dependency, so the offset comes fromdate +%z- the same shell-out idiomlast::locatealready uses - resolved once behind aOnceLock. Non-unix returns 0, and anything unparseable returns 0, so both keep exactly today's UTC behaviour rather than guessing.If you would rather own a dependency for this, say so and I will switch it to
jiffortime. I picked the zero-dependency version because of the note inCargo.tomlabout keeping the build pure Rust, but it is your call and the change is small either way.One existing test touched
the_picker_lists_newest_first_and_opens_the_chosen_messageasserts on rendered times, which would otherwise vary by the machine running CI. The offset is held onAppso that test pins it to0in one line and the assertions stay exactly as they were.Tests
Three, in a
testsmodule beside the code inpick.rs:a_utc_stamp_is_shown_on_the_local_clock- including a day rollover,19:53Zat +05:30 is01:23a_stamp_that_already_carries_an_offset_is_shown_as_writtena_zone_string_reads_as_minutes_east_of_utccargo fmt --all --check,cargo clippy --workspace --all-targetsandcargo test --workspaceall clean on 1.97.1.Deliberately not included
Showing a date for messages that are not from today. That needs real civil-date arithmetic, which is where a dependency genuinely starts to earn its place, and it would double the size of this diff. Happy to follow up if you want it.