GUI: Use locale-aware GUIUtil::dateTimeStr for datetime fields - #336
GUI: Use locale-aware GUIUtil::dateTimeStr for datetime fields#336kwsantiago wants to merge 2 commits into
Conversation
|
I have negative feelings about this one. The original format gave a date and time that made it obvious "when" it was, so you can ignore the country the machine was running in. A user could "screenshot" a page and post it, but the date/time isn't clear unless you know where the user is posting from (or even when they are posting). I would prefer to see something more like this (in most significant to least significant notation, and the month converted to text): I found 11 instances of this function across the codebase. Should there be a single utility function to format the date/time, with the formatting applied there, so that every date/time is always perfectly consistent? Your thoughts? |
|
Unfortunately, locales are a real thing. Displaying the datetime in the wrong locale doesn't seem like the right move IMO |
There was a problem hiding this comment.
ACK 68685bd
CONCEPT NAK 68685bd
ACK
I am giving this an ACK because I have reviewed the change and the migration from QDateTime::currentDateTime() to GUIUtil::dateTimeStr() is an improvement on the current solution and directly addresses issue #335. I have reviewed the code change and tested it to verify it works as expected.
CONCEPT NAK
I am giving it a concept NAK because I am opposed to the concept of localisation that provides a date/time that is ambiguous to a global audience, which Bitcoin has. For example, given the date "1/2/2022", it is not possible to know whether it refers to the first of February or the 2nd of January without knowing the origin of the person providing the information. I would prefer something based on ISO 8601, where date information is displayed from most to least significant. For example:
String customTime = GUIUtil::dateTimeStr(time_t_value, "yyyy-MM-dd HH:mm:ss");
This is globally consistent, works in every language and time zone, and is easily understood by users in every country. I would go so far as to say that basing a date/time on ISO 8601 is the Gold standard for a project with an international audience.
For example, BIP110 could activate on the 1st of September 2026. Users could post screenshots referencing both 1/9/2026 and 9/1/2026. Now, roll forward 10 years and consider a historian trying to piece this story together.
Review
I have reviewed the code change and tested it to verify it works as expected.
I also reviewed the change with CodeRabbit, and it found no issues.
I built the system using Ubuntu 24.04 running on ARMv8-A (64-bit) with gcc and used guix to create a Windows binary.
git clone https://github.com/bitcoinknots/bitcoin.git pr336
cd pr336
git fetch origin pull/336/head:pr336
git switch pr336
cmake -B build -DCMAKE_BUILD_TYPE=Release -DRDTS_CONSENT=IMPLICIT
nice cmake --build build -j$(nproc)
I then built using guix.
env HOSTS='x86_64-w64-mingw32' ./contrib/guix/guix-build
And tested with:
bitcoin-qt.exe -regtest
Everything worked as expected.
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent/non-localized datetime formatting in the Qt GUI by replacing bare QDateTime::toString() calls with the project’s existing locale-aware GUIUtil::dateTimeStr helper, aligning these fields with other datetime displays in the application.
Changes:
- Use
GUIUtil::dateTimeStr(qint64)for the “Software expiry” datetime in the Information tab. - Use
GUIUtil::dateTimeStr(const QDateTime&)for “Last block time” in the Information tab and the sync overlay. - Use
GUIUtil::dateTimeStr(const QDateTime&)for the client startup time formatting inClientModel.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/qt/rpcconsole.cpp | Switches expiry and last-block timestamp labels to GUIUtil::dateTimeStr for locale-consistent formatting. |
| src/qt/modaloverlay.cpp | Switches sync overlay newest-block timestamp to GUIUtil::dateTimeStr. |
| src/qt/clientmodel.cpp | Switches formatted startup time to GUIUtil::dateTimeStr for consistent GUI datetime formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Locale-aware display is exactly what @luke-jr is pointing at above: a fixed format shows the datetime in the wrong locale for most users. Respecting the user's system locale is the right default, and it's what the rest of the GUI already does for transaction dates, so this just makes these fields consistent with everything else. On the "single utility" question, that's what this PR does: all four fields now go through |
68685bd to
09b7314
Compare
09b7314 to
2f53648
Compare
Fixes #335
Startup time, Expiry time, and Last block time (in the Information tab and the sync overlay) were formatted with bare
QDateTime::toString(). On Qt 5.15, which the release binaries are built against, that produces asctime-style ordering (Thu Jul 16 20:43:39 2026) with the time wedged between the day and the year, and it ignores the user's locale. Qt 6.7+ changed the default ordering, which is why this only shows up on Qt5 builds.Switch these fields to
GUIUtil::dateTimeStr, the same locale-aware formatting used for every other datetime in the GUI. Seconds are no longer shown, consistent with datetimes elsewhere.Before / After