refactor: use std::chrono for formatDurationStr() helper #549

1. 
   jonatack commented at 0:08 am on February 15, 2022: contributor

   Updates formatDurationStr() to use the chrono standard lib. No change in behavior.
2. gui, refactor: use std::chrono for formatDurationStr() helper 6f2593dc23
3. jonatack renamed this:
   gui, refactor: use std::chrono for formatDurationStr() helper
   refactor: use std::chrono for formatDurationStr() helper
   on Feb 15, 2022
4. shaavan approved
5. 
   shaavan commented at 11:50 am on February 16, 2022: contributor

   ACK 6f2593dc23565abaa3d176595cba6e07883f512e

   The updated code looks clean and is straightforward to understand. I was able to compile the PR branch successfully and test that updated formatDurationStr is providing correct results for the ui->peerConnTime.

   Here is a screenshot of the correct working of the PR.

   

   As a small nit suggestion, I think it would be a good idea to space out the code a little bit into logical sections. But this is a non-critical suggestion, and you may take it up in case you are going to update the PR for some other reasons.

6. w0xlt approved
7. 
   w0xlt commented at 3:01 pm on February 18, 2022: contributor

   tACK 6f2593d on Ubuntu 21.10 Qt 5.15.2
8. hebasto added the label Refactoring on Feb 20, 2022
9. in src/qt/guiutil.cpp:722 in 6f2593dc23

   734+    const auto d{std::chrono::duration_cast<days>(dur)};
   735+    const auto h{std::chrono::duration_cast<std::chrono::hours>(dur - d)};
   736+    const auto m{std::chrono::duration_cast<std::chrono::minutes>(dur - d - h)};
   737+    const auto s{std::chrono::duration_cast<std::chrono::seconds>(dur - d - h - m)};
   738+    QStringList str_list;
   739+    if (auto d2{d.count()}) str_list.append(QObject::tr("%1 d").arg(d2));
   

   ---

   ---

   

   hebasto commented at 8:58 pm on February 20, 2022:

   Maybe add translator comments, here and after?

   ---

   

   jonatack commented at 9:13 pm on February 20, 2022:

   Yes, doing for several of these (FormatPeerAge, formatDurationStr, TimeDurationField, etc.) as a follow-up to this and to #543 (see #543 (review)). That way this remains a straight refactor and current review on both pulls isn’t invalidated.
10. in src/qt/guiutil.cpp:726 in 6f2593dc23

    738+    QStringList str_list;
    739+    if (auto d2{d.count()}) str_list.append(QObject::tr("%1 d").arg(d2));
    740+    if (auto h2{h.count()}) str_list.append(QObject::tr("%1 h").arg(h2));
    741+    if (auto m2{m.count()}) str_list.append(QObject::tr("%1 m").arg(m2));
    742+    const auto s2{s.count()};
    743+    if (s2 || str_list.empty()) str_list.append(QObject::tr("%1 s").arg(s2));
    

    ---

    ---

    

    promag commented at 10:42 pm on February 22, 2022:

    6f2593dc23565abaa3d176595cba6e07883f512e

    nit, suggestion:

    0if (auto s2{s.count()}) str_list.append(QObject::tr("%1 s").arg(s2));
    1return str_list.empty() ? "0 s" : str_list.join(" ");
    

    ---

    

    jonatack commented at 7:59 pm on February 23, 2022:

    Sure, will do if have to retouch or will sneak it into the translator comments follow-up.

    ---

    

    RandyMcMillan commented at 8:30 pm on March 1, 2022:

    Note: When following up - “Connection Age” will bring the naming convention together - in the gui as well as CL

    tr(“Age”)

11. promag approved
12. 
    promag commented at 10:44 pm on February 22, 2022: contributor

    Code review ACK 6f2593dc23565abaa3d176595cba6e07883f512e.

    Maybe leave a comment that <format> can be used once we jump to C++20.

13. 
    RandyMcMillan commented at 8:31 pm on March 1, 2022: contributor

    tACK 6f2593dc23565abaa3d176595cba6e07883f512e

    tested on macOS x86_64, Arm64

    

    

14. hebasto merged this on Mar 5, 2022
15. hebasto closed this on Mar 5, 2022

    ---

16. jonatack deleted the branch on Mar 5, 2022
17. sidhujag referenced this in commit 7d8e3e480d on Mar 6, 2022
18. bitcoin-core locked this on Mar 5, 2023

Contributors
jonatack shaavan w0xlt hebasto promag RandyMcMillan

Labels
Refactoring