Merge pull request certinia#722 from lukecotter/chore-ms-time-display

feat: ms time display

Author: lcottercertinia

log-viewer/src/__tests__/Util.test.ts

@@ -4,18 +4,21 @@
 import { formatDuration } from '../core/utility/Util.js';
 
 describe('Format duration tests', () => {
-  it('Shows µs for very small values', () => {
-    expect(formatDuration(5)).toBe('0.01 µs');
-    expect(formatDuration(50)).toBe('0.05 µs');
-    expect(formatDuration(500)).toBe('0.5 µs');
-    expect(formatDuration(1000)).toBe('1 µs');
-    expect(formatDuration(5000)).toBe('5 µs');
-    expect(formatDuration(9999)).toBe('10 µs');
-    expect(formatDuration(10000)).toBe('10 µs');
+  it('Shows ms with decimals for very small values (sub-millisecond)', () => {
+    expect(formatDuration(5)).toBe('0 ms'); // 0.000005 ms rounds to 0
+    expect(formatDuration(50)).toBe('0 ms'); // 0.00005 ms rounds to 0
+    expect(formatDuration(500)).toBe('0.001 ms');
+    expect(formatDuration(1000)).toBe('0.001 ms');
+    expect(formatDuration(5000)).toBe('0.005 ms');
+    expect(formatDuration(9999)).toBe('0.01 ms');
+    expect(formatDuration(10000)).toBe('0.01 ms');
+    expect(formatDuration(50000)).toBe('0.05 ms');
+    expect(formatDuration(99999)).toBe('0.1 ms');
   });
 
   it('handles ms duration', () => {
     expect(formatDuration(100_000)).toBe('0.1 ms');
+    expect(formatDuration(500_000)).toBe('0.5 ms');
     expect(formatDuration(1_000_000)).toBe('1 ms');
     expect(formatDuration(1_234_567)).toBe('1.23 ms');
     expect(formatDuration(9_999_999)).toBe('10 ms');
@@ -41,22 +44,22 @@ describe('Format duration tests', () => {
   });
 
   it('handles remove trailing 0 for all units types', () => {
-    expect(formatDuration(5000)).toBe('5 µs');
+    expect(formatDuration(5000)).toBe('0.005 ms');
     expect(formatDuration(100_000)).toBe('0.1 ms');
     expect(formatDuration(5_000_000_000)).toBe('5 s');
     expect(formatDuration(60_000_000_000)).toBe('1m');
   });
 
-  it('handles rounding to 2dp for µs, ms, s', () => {
-    // microseconds
-    expect(formatDuration(1234)).toBe('1.23 µs');
-    expect(formatDuration(9876)).toBe('9.88 µs');
+  it('handles rounding to appropriate precision', () => {
+    // sub-milliseconds (up to 3 decimal places)
+    expect(formatDuration(1234)).toBe('0.001 ms');
+    expect(formatDuration(9876)).toBe('0.01 ms');
 
-    // milliseconds
+    // milliseconds (up to 2 decimal places)
     expect(formatDuration(1_234_567)).toBe('1.23 ms');
     expect(formatDuration(9_876_543)).toBe('9.88 ms');
 
-    // seconds
+    // seconds (up to 2 decimal places)
     expect(formatDuration(1_234_567_890)).toBe('1.23 s');
     expect(formatDuration(9_876_543_210)).toBe('9.88 s');
   });
@@ -65,4 +68,25 @@ describe('Format duration tests', () => {
     // minutes with fractional seconds
     expect(formatDuration(125_670_000_000)).toBe('2m 5.7s');
   });
+
+  describe('compact option', () => {
+    it('omits spaces for milliseconds', () => {
+      expect(formatDuration(0, { compact: true })).toBe('0ms');
+      expect(formatDuration(50000, { compact: true })).toBe('0.05ms');
+      expect(formatDuration(1_000_000, { compact: true })).toBe('1ms');
+      expect(formatDuration(1_234_567, { compact: true })).toBe('1.23ms');
+      expect(formatDuration(100_000_000, { compact: true })).toBe('100ms');
+    });
+
+    it('omits spaces for seconds', () => {
+      expect(formatDuration(5_000_000_000, { compact: true })).toBe('5s');
+      expect(formatDuration(59_500_000_000, { compact: true })).toBe('59.5s');
+    });
+
+    it('omits spaces for minutes', () => {
+      expect(formatDuration(60_000_000_000, { compact: true })).toBe('1m');
+      expect(formatDuration(125_000_000_000, { compact: true })).toBe('2m5s');
+      expect(formatDuration(125_500_000_000, { compact: true })).toBe('2m5.5s');
+    });
+  });
 });

log-viewer/src/core/utility/Util.ts

@@ -2,50 +2,58 @@
  * Copyright (c) 2020 Certinia Inc. All rights reserved.
  */
 
+/**
+ * Options for formatting durations.
+ */
+export interface FormatDurationOptions {
+  /**
+   * If true, omits spaces between values and units (e.g., "5ms" instead of "5 ms").
+   * Useful for compact displays like the minimap axis.
+   */
+  compact?: boolean;
+}
+
 /**
  * Formats a duration in nanoseconds into a human-readable string.
  *
- * Automatically selects the most appropriate unit (microseconds, milliseconds, seconds, or minutes)
+ * Automatically selects the most appropriate unit (milliseconds, seconds, or minutes)
  * based on the magnitude of the duration. Applies appropriate precision for each unit.
  *
  * @param ns - The duration in nanoseconds to format
+ * @param options - Optional formatting options
  * @returns A formatted string representing the duration with appropriate units:
- * - Microseconds (µs) for durations < 0.1ms
- * - Milliseconds (ms) for durations < 1000ms
+ * - Milliseconds (ms) for durations < 1000ms (with up to 3 decimal places for sub-ms)
  * - Seconds (s) for durations < 60s
  * - Minutes and seconds (e.g., "2m 30s") for durations ≥ 60s
  *
  * @example
  * ```typescript
- * formatDuration(5000);        // "5 µs"
- * formatDuration(1500000);     // "1.5 ms"
- * formatDuration(2500000000);  // "2.5 s"
- * formatDuration(90000000000); // "1m 30s"
+ * formatDuration(50000);                      // "0.05 ms"
+ * formatDuration(1500000);                    // "1.5 ms"
+ * formatDuration(2500000000);                 // "2.5 s"
+ * formatDuration(90000000000);                // "1m 30s"
+ * formatDuration(50000, { compact: true });   // "0.05ms"
  * ```
  */
-export function formatDuration(ns: number) {
+export function formatDuration(ns: number, options?: FormatDurationOptions): string {
+  const space = options?.compact ? '' : ' ';
+
   if (!ns) {
-    return '0 ms';
+    return `0${space}ms`;
   }
 
   const ms = ns / 1e6;
 
-  // microseconds (< 0.01 ms)
-  if (ms < 0.1) {
-    const us = ns / 1e3;
-    const precision = us < 10 ? 100 : us < 100 ? 10 : 1;
-    return `${round(us, precision)} µs`;
-  }
-
   if (ms < 1000) {
-    const precision = ms < 10 ? 100 : ms < 100 ? 10 : 1;
-    return `${round(ms, precision)} ms`;
+    // Precision: 3 decimals for sub-ms, 2 for <10ms, 1 for <100ms, 0 for >=100ms
+    const precision = ms < 1 ? 1000 : ms < 10 ? 100 : ms < 100 ? 10 : 1;
+    return `${round(ms, precision)}${space}ms`;
   }
 
   const s = ms / 1000;
   if (s < 60) {
     const precision = s < 10 ? 100 : s < 100 ? 10 : 1;
-    return `${round(s, precision)} s`;
+    return `${round(s, precision)}${space}s`;
   }
 
   const m = Math.floor(s / 60);
@@ -56,7 +64,7 @@ export function formatDuration(ns: number) {
   }
 
   const secStr = sec === Math.floor(sec) ? `${sec}s` : `${round(sec, 10)}s`;
-  return `${m}m ${secStr}`;
+  return `${m}m${space}${secStr}`;
 }
 
 function round(value: number, precision: number): number {

log-viewer/src/features/timeline/__tests__/tooltip.test.ts

@@ -272,7 +272,7 @@ describe('TimelineTooltipManager', () => {
       const tooltip = container.querySelector('#timeline-tooltip') as HTMLElement;
       expect(tooltip.textContent).toContain('total');
       // Check for some duration value (format may vary)
-      expect(tooltip.textContent).toContain('µs');
+      expect(tooltip.textContent).toContain('ms');
 
       tooltipManager.hide();
     });

log-viewer/src/features/timeline/optimised/minimap/MinimapAxisRenderer.ts

@@ -22,14 +22,20 @@
  */
 
 import { BitmapText, Container, Graphics } from 'pixi.js';
-import { TEXT_LABEL_CONSTANTS } from '../../types/flamechart.types.js';
+
+import { formatDuration, TEXT_LABEL_CONSTANTS } from '../../types/flamechart.types.js';
 import type { MinimapManager } from './MinimapManager.js';
 
 /**
  * Nanoseconds per millisecond conversion constant.
  */
 const NS_PER_MS = 1_000_000;
 
+/**
+ * Pre-allocated options for compact duration formatting (avoids allocation in render loop).
+ */
+const COMPACT_FORMAT = { compact: true } as const;
+
 /**
  * Axis configuration.
  */
@@ -67,6 +73,9 @@ export class MinimapAxisRenderer {
   private labelPool: BitmapText[] = [];
   private activeLabelCount = 0;
 
+  /** Pre-allocated stroke options (avoids allocation in render loop) */
+  private strokeOptions: { color: number; width: number };
+
   /**
    * Creates the axis renderer without adding to any parent container.
    * Caller is responsible for adding getTickGraphics() and getLabelsContainer()
@@ -82,6 +91,9 @@ export class MinimapAxisRenderer {
 
     // Create container for labels
     this.labelsContainer = new Container();
+
+    // Initialize stroke options (updated in refreshColors)
+    this.strokeOptions = { color: this.config.tickColor, width: 1 };
   }
 
   /**
@@ -158,12 +170,12 @@ export class MinimapAxisRenderer {
       // Draw tick line full height of minimap
       this.tickGraphics.moveTo(x, axisY);
       this.tickGraphics.lineTo(x, minimapHeight);
-      this.tickGraphics.stroke({ color: this.config.tickColor, width: 1 });
+      this.tickGraphics.stroke(this.strokeOptions);
 
       // Add label if needed (positioned to the left of the line)
       if (shouldShowLabel && timeNs > 0) {
         const label = this.getOrCreateLabel();
-        label.text = this.formatTime(timeNs);
+        label.text = formatDuration(timeNs, COMPACT_FORMAT);
         label.x = x - 3; // 3px to the left of line
         label.y = axisY + 2; // Near top
         label.anchor.set(1, 0); // Right-align to line
@@ -194,6 +206,7 @@ export class MinimapAxisRenderer {
     const tickColorStr =
       computedStyle.getPropertyValue('--vscode-editorLineNumber-foreground').trim() || '#808080';
     this.config.tickColor = this.parseColorToHex(tickColorStr);
+    this.strokeOptions.color = this.config.tickColor;
 
     // Update label tint color
     this.config.labelTint = this.parseColorToHex(tickColorStr);
@@ -370,36 +383,4 @@ export class MinimapAxisRenderer {
       label.visible = false;
     }
   }
-
-  // ============================================================================
-  // PRIVATE: FORMATTING
-  // ============================================================================
-
-  /**
-   * Format time with appropriate units.
-   * Similar to main axis but more compact for minimap.
-   */
-  private formatTime(timeNs: number): string {
-    const timeMs = timeNs / NS_PER_MS;
-
-    // Convert to seconds if >= 1000ms and whole seconds
-    if (timeMs >= 1000 && timeMs % 1000 === 0) {
-      const seconds = timeMs / 1000;
-      return `${seconds}s`;
-    }
-
-    // Format as milliseconds
-    if (timeMs >= 1) {
-      // Whole milliseconds: no decimals
-      if (timeMs === Math.floor(timeMs)) {
-        return `${Math.floor(timeMs)}ms`;
-      }
-      // Fractional: up to 2 decimal places
-      return `${timeMs.toFixed(2).replace(/\.?0+$/, '')}ms`;
-    }
-
-    // Sub-millisecond: show as microseconds
-    const timeUs = timeMs * 1000;
-    return `${Math.round(timeUs)}µs`;
-  }
 }

log-viewer/src/features/timeline/types/flamechart.types.ts

@@ -12,13 +12,18 @@
 //TODO: Remove deps outside timeline
 
 import type { LogEvent, LogSubCategory } from 'apex-log-parser';
+import { formatDuration } from '../../../core/utility/Util.js';
 import type { PrecomputedRect } from '../optimised/RectangleManager.js';
 
 // Re-export LogEvent for internal use within timeline/ folder
 // Note: FlameChart's PUBLIC API (callbacks) should use EventNode, not LogEvent
 // LogEvent is only used internally by data structures like RectangleManager, HitTestManager
 export type { LogEvent };
 
+// Re-export formatDuration for use within timeline/optimised folder
+// This keeps the Util.ts dependency at the boundary (types file)
+export { formatDuration };
+
 // ============================================================================
 // VIEWPORT STATE
 // ============================================================================