zed icon indicating copy to clipboard operation
zed copied to clipboard

Respect user preferences when formatting timestamp

Open bennetbo opened this issue 4 months ago • 1 comments

This is a follow up to #7945. The current behaviour reads the locale and infers from that which type of time format should be used (12 hour/24 hour). However, in macOS you can override this behaviour, e.g. you can use en_US locale but still use the 24 hour clock format (Can be customized under Settings > General > Date & Format > 24-hour time). You can even customize the date format.

This PR uses the macOS specific CFDateFormatter API, which outputs time format strings, that respect those settings.

Partially fixes #7956 (as its not implemented for linux)

Release Notes:

  • Added localization support for all macOS specific date and time configurations in chat

bennetbo avatar Feb 19 '24 00:02 bennetbo

Thanks for pointing me in the right direction @ConradIrwin. I managed to implement your suggestion in the format_time crate (not sure about the name of the crate, maybe something more general like localization would be better?).

I did some tests locally, and all formats (Configuration under macOS Settings > General > Date & Time/Language & Region) seem to get picked up by zed now.

Also formatting was originally quite slow, caused by the call to CFDateFormatterCreate. Formatting 10000 timestamps took around 30 seconds on my machine. However, caching the date formatter in a thread local brings down the time to format 10000 timestamps to 0.2 seconds (which I think is fine for now, as we normally only format up to 50/100 dates per frame, depending how large the messages are in the chat).

I will add some notes to the code as there is some unsafe in there, which I am not that familiar with and want to make sure to not introduce some sort of memory leaks.

Also I noticed there is still some time formatting code which is not localized (see NotificationPanel::format_timestamp and in the journal stuff). However, I would like to keep the changes minimal for now and follow up with another PR to fix those.

Note: Previously the year was omitted if the message was sent in the last year. The year is now always included, because there doesn't seem to be away to omit the date using the native api, without some nasty workarounds (handling different settings manually).

So personally, I would say we just include the year even if the message is less then a year old, that's also what other messaging apps seem to be doing (e.g. Discord).

bennetbo avatar Feb 23 '24 12:02 bennetbo

Thank you!

ConradIrwin avatar Feb 24 '24 02:02 ConradIrwin