From 70becb3050fad11c63c375a44ec315c255b5d981 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= <florian@queze.net>
Date: Wed, 31 May 2023 07:41:29 +0200
Subject: [PATCH] Support the 'day' unit in formatTimestamp, and polish the
 display of minutes and hours.

---
 src/test/unit/marker-schema.test.js | 26 +++++++++-
 src/utils/format-numbers.js         | 75 +++++++++++++++++++----------
 2 files changed, 74 insertions(+), 27 deletions(-)

diff --git a/src/test/unit/marker-schema.test.js b/src/test/unit/marker-schema.test.js
index 96a430abea..59e7a70a9c 100644
--- a/src/test/unit/marker-schema.test.js
+++ b/src/test/unit/marker-schema.test.js
@@ -192,8 +192,19 @@ describe('marker schema formatting', function () {
       ['duration', 10],
       ['duration', 12.3456789],
       ['duration', 123456.789],
+      ['duration', 23456789],
+      ['duration', 50000],
+      ['duration', 2000000],
+      ['duration', 50000000],
       ['duration', 123456789],
       ['duration', 0.000123456],
+      ['duration', 1000 * 60 - 0.01], // slightly less than 1min, should not be shown as '60s'
+      ['duration', 1000 * 60 + 1], // slightly more than 1min, should not be shown as '1min0s'
+      ['duration', 779873.639], // '13min60s' should be rounded to '14min'
+      ['duration', 1000 * 3600 - 0.01], // slightly less than 1h, should not be shown as '0h60min'
+      ['duration', 1000 * 3600 + 1], // slightly more than 1h, should not be shown as '1h0min'
+      ['duration', 1000 * 3600 * 24 - 0.01], // slightly less than 1 day, should not be shown as '0d24h'
+      ['duration', 1000 * 3600 * 24 + 1], // slightly more than 1 day, should not be shown as '1min0s'
       ['time', 12.3456789],
       ['seconds', 0],
       ['seconds', 10],
@@ -251,9 +262,20 @@ describe('marker schema formatting', function () {
         "duration - 0s",
         "duration - 10ms",
         "duration - 12.346ms",
-        "duration - 2.06min",
-        "duration - 34.3h",
+        "duration - 2min3s",
+        "duration - 6h31min",
+        "duration - 50s",
+        "duration - 33min20s",
+        "duration - 13h53min",
+        "duration - 1d10h",
         "duration - 123.46ns",
+        "duration - 1min",
+        "duration - 1min",
+        "duration - 13min",
+        "duration - 1h",
+        "duration - 1h",
+        "duration - 1d",
+        "duration - 1d",
         "time - 12.346ms",
         "seconds - 0.000s",
         "seconds - 0.010s",
diff --git a/src/utils/format-numbers.js b/src/utils/format-numbers.js
index da4e6731ce..c3e69976cc 100644
--- a/src/utils/format-numbers.js
+++ b/src/utils/format-numbers.js
@@ -249,12 +249,16 @@ export function formatMinutes(
   significantDigits: number = 5,
   maxFractionalDigits: number = 2
 ) {
+  const msPerSecond = 1000;
+  time = Math.round(time / msPerSecond) * msPerSecond;
+  const msPerMinute = 60 * msPerSecond;
+  const seconds = time % msPerMinute;
   return (
-    formatNumber(
-      time / (60 * 1000),
-      significantDigits,
-      Math.min(2, maxFractionalDigits)
-    ) + 'min'
+    formatNumber((time - seconds) / msPerMinute, significantDigits, 0) +
+    'min' +
+    (maxFractionalDigits > 0 && seconds > 0
+      ? formatSeconds(seconds, significantDigits, 0)
+      : '')
   );
 }
 
@@ -263,12 +267,34 @@ export function formatHours(
   significantDigits: number = 5,
   maxFractionalDigits: number = 1
 ) {
+  const msPerMinute = 60 * 1000;
+  time = Math.round(time / msPerMinute) * msPerMinute;
+  const msPerHour = 60 * msPerMinute;
+  const minutes = time % msPerHour;
   return (
-    formatNumber(
-      time / (3600 * 1000),
-      significantDigits,
-      Math.min(1, maxFractionalDigits)
-    ) + 'h'
+    formatNumber((time - minutes) / msPerHour, significantDigits, 0) +
+    'h' +
+    (maxFractionalDigits > 0 && minutes > 0
+      ? formatMinutes(minutes, significantDigits, 0)
+      : '')
+  );
+}
+
+export function formatDays(
+  time: Milliseconds,
+  significantDigits: number = 5,
+  maxFractionalDigits: number = 1
+) {
+  const msPerHour = 60 * 60 * 1000;
+  time = Math.round(time / msPerHour) * msPerHour;
+  const msPerDay = 24 * msPerHour;
+  const hours = time % msPerDay;
+  return (
+    formatNumber((time - hours) / msPerDay, significantDigits, 0) +
+    'd' +
+    (maxFractionalDigits > 0 && hours > 0
+      ? formatHours(hours, significantDigits, 0)
+      : '')
   );
 }
 
@@ -277,21 +303,20 @@ export function formatTimestamp(
   significantDigits: number = 5,
   maxFractionalDigits: number = 3
 ) {
-  // Format in the closest base (hours, minutes, seconds, milliseconds, microseconds, or nanoseconds),
-  // to avoid cases where times are displayed with too many leading zeroes to be useful.
-  if (time >= 3600 * 1000) {
-    return formatHours(
-      time,
-      significantDigits,
-      Number.isInteger(time / (3600 * 1000)) ? 0 : maxFractionalDigits
-    );
-  }
-  if (time >= 60 * 1000) {
-    return formatMinutes(
-      time,
-      significantDigits,
-      Number.isInteger(time / (60 * 1000)) ? 0 : maxFractionalDigits
-    );
+  // Format in the closest base (days, hours, minutes, seconds, milliseconds,
+  // microseconds, or nanoseconds), to avoid cases where times are displayed
+  // with too many leading zeroes to be useful.
+  // The Math.round call is needed to avoid showing values like '0min60s'.
+  if (Math.round(time / 1000) / 60 >= 1) {
+    // The if blocks are nested to avoid calling Math.round repeatedly for
+    // the most common case where the value will be less than 1 minute.
+    if (Math.round(time / (60 * 1000)) / 60 >= 1) {
+      if (Math.round(time / (60 * 60 * 1000)) / 24 >= 1) {
+        return formatDays(time, significantDigits, maxFractionalDigits);
+      }
+      return formatHours(time, significantDigits, maxFractionalDigits);
+    }
+    return formatMinutes(time, significantDigits, maxFractionalDigits);
   }
   if (time >= 1000) {
     return formatSeconds(