diff --git a/code-reviews/Driver.Cli.Common/findings.md b/code-reviews/Driver.Cli.Common/findings.md index d84bd3a..8adc23a 100644 --- a/code-reviews/Driver.Cli.Common/findings.md +++ b/code-reviews/Driver.Cli.Common/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 2 | +| Open findings | 0 | ## Checklist coverage @@ -130,7 +130,7 @@ dispose the previous logger if reconfiguration is genuinely intended. | Severity | Low | | Category | Error handling & resilience | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:68-70` | -| Status | Open | +| Status | Resolved | **Description:** `FormatTable` calls `rows.Max(r => r.Tag.Length)` (and the same for the value and status columns) without guarding against empty input. When `tagNames` and @@ -143,7 +143,13 @@ instead of producing an empty (header-only) table. separator, or an explicit "no rows" line), or use `DefaultIfEmpty(0).Max(...)` for the width computations. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — `FormatTable` guards each `rows.Max(...)` width +computation with a `rows.Length == 0 ? "
".Length : Math.Max(...)` ternary, so +an empty batch read returns the header + separator rows (no data rows) instead of +throwing `InvalidOperationException`. The fix was landed in commit `1433a1c` alongside +the -002 work, and the regression test +`SnapshotFormatterTests.FormatTable_with_empty_input_returns_header_only` (added under +-005) exercises it. ### Driver.Cli.Common-005 @@ -178,7 +184,7 @@ empty-input and `DriverCommandBase` level-selection tests. | Severity | Low | | Category | Documentation & comments | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:71`, `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:9` | -| Status | Open | +| Status | Resolved | **Description:** Two minor doc inaccuracies. (1) The comment at `SnapshotFormatter.cs:71` states the "source-time column is fixed-width (ISO-8601 to ms) so no max-measurement @@ -194,4 +200,8 @@ library. The XML doc is stale relative to the shipped driver-CLI set. right-most and intentionally unpadded rather than claiming fixed width. Add FOCAS to the `DriverCommandBase` class-summary driver list. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — (1) `SnapshotFormatter.cs:71` comment reworded +to state the source-time column is the right-most one and intentionally not +measured/padded, calling out the null-timestamp `"-"` case explicitly. (2) FOCAS was +added to the `DriverCommandBase` class-summary driver enumeration in commit `7ff356b` +(landed alongside the -003 work). diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs index 98c205e..1a6c3e3 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs @@ -68,7 +68,10 @@ public static class SnapshotFormatter int tagW = rows.Length == 0 ? "TAG".Length : Math.Max("TAG".Length, rows.Max(r => r.Tag.Length)); int valW = rows.Length == 0 ? "VALUE".Length : Math.Max("VALUE".Length, rows.Max(r => r.Value.Length)); int statW = rows.Length == 0 ? "STATUS".Length : Math.Max("STATUS".Length, rows.Max(r => r.Status.Length)); - // source-time column is fixed-width (ISO-8601 to ms) so no max-measurement needed. + // source-time is the right-most column, so it is intentionally not measured or padded; + // when a snapshot has a non-null SourceTimestampUtc the cell is 24 chars (ISO-8601 to ms), + // and when the timestamp is null FormatTimestamp emits "-" — the resulting unalignment is + // harmless because nothing is appended after this column. var sb = new System.Text.StringBuilder(); sb.Append("TAG".PadRight(tagW)).Append(" ")