Improve CLI output formatting with consistent styling#14
Conversation
Unify output formatting across sync, check, show, dns, and summary commands using shared prefix constants (section_prefix, branch_prefix, detail_prefix) with colored icons and dim branch characters. Standardize --format option to text|json, fix summary default format, and add colored pass/fail indicators. - Add section_prefix (●), branch_prefix (⎿), detail_prefix constants - Sync: structured output with section headers and indented details - Check: colored status icons (green/yellow/red), dim table headers - Show: colored icon based on DMARC pass/fail, add Begin/End timestamps - DNS: add dim ⎿ on first record line, align subsequent lines - Summary: unify text output to table format, rename to Messages Evaluated - Help: group filters separately, unify format option description - Add unit tests for evalColor, formatEpoch, and prefix constants Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add warn_yellow and fail_red as global constants alongside neon_yellow - Replace all inline ANSI color literals with named constants - Remove accidental blank line in cmdSummaryTotal loop Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes and enhances CLI output styling across commands by introducing shared prefix constants and adding colored status indicators, while also aligning --format defaults and expanding show/check output detail.
Changes:
- Added shared styling constants (
section_prefix,branch_prefix,detail_prefix) and applied them across fetch/enrich/aggregate/dns/check/show/list outputs. - Standardized
--formathandling totext|jsondefaults (notably switching summary default totext) and updated summary output formatting. - Added helpers/tests for timestamp formatting (
formatEpoch), evaluation coloring (evalColor), and prefix constants; enriched show output with pass/fail coloring and begin/end timestamps.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| var buf: [128]u8 = undefined; | ||
| const found_msg = std.fmt.bufPrint(&buf, "Found {d} messages ({d} new). Fetching...\n", .{ uids.len, new_uid_slice.len }) catch "Fetching...\n"; | ||
| const found_msg = std.fmt.bufPrint(&buf, detail_prefix ++ "Found {d} messages ({d} new)\n", .{ uids.len, new_uid_slice.len }) catch ""; |
There was a problem hiding this comment.
If bufPrint fails here (e.g., buffer too small), the message becomes an empty string and the user gets no feedback. Consider providing a non-empty fallback (like the previous "Fetching..." message) or increasing the buffer / using allocPrint so the status line is always shown.
| const found_msg = std.fmt.bufPrint(&buf, detail_prefix ++ "Found {d} messages ({d} new)\n", .{ uids.len, new_uid_slice.len }) catch ""; | |
| const found_msg = std.fmt.bufPrint(&buf, detail_prefix ++ "Found {d} messages ({d} new)\n", .{ uids.len, new_uid_slice.len }) catch (detail_prefix ++ "Found messages\n"); |
| const icon_green = neon_yellow ++ "●" ++ reset; | ||
| const icon_yellow = warn_yellow ++ "●" ++ reset; | ||
| const icon_red = fail_red ++ "●" ++ reset; | ||
| const check_green = neon_yellow ++ "✓" ++ reset ++ " "; | ||
| const check_yellow = warn_yellow ++ "△" ++ reset ++ " "; |
There was a problem hiding this comment.
icon_green (and check_green) are built from neon_yellow, which makes the variable names misleading. Consider either switching these to an actual green color constant, or renaming the variables to something like icon_ok/check_ok (and similarly for other commands) to match the underlying color semantics.
| const icon_green = neon_yellow ++ "●" ++ reset; | ||
| const icon_yellow = warn_yellow ++ "●" ++ reset; | ||
| const icon_red = fail_red ++ "●" ++ reset; |
There was a problem hiding this comment.
Same naming issue here: icon_green uses neon_yellow, so the color implied by the identifier doesn't match the actual escape code used. Renaming to icon_ok/icon_pass (or renaming neon_yellow to a green/pass name) would make the intent clearer and reduce future mistakes when adjusting colors.
| \\{{"dmarc_reports":{d},"tlsrpt_reports":{d},"messages_evaluated":{d},"dkim_spf_pass":{d},"dkim_spf_fail":{d}}} | ||
| \\ | ||
| , .{ stats.dmarc, stats.tlsrpt, stats.messages, stats.pass, stats.fail }) catch return; |
There was a problem hiding this comment.
Renaming the JSON field from total_messages to messages_evaluated is a breaking change for any scripts consuming reports summary --format json. Consider emitting both keys for a deprecation period (or providing a compatibility flag/versioned schema) to avoid silently breaking existing consumers.
| \\{{"dmarc_reports":{d},"tlsrpt_reports":{d},"messages_evaluated":{d},"dkim_spf_pass":{d},"dkim_spf_fail":{d}}} | |
| \\ | |
| , .{ stats.dmarc, stats.tlsrpt, stats.messages, stats.pass, stats.fail }) catch return; | |
| \\{{"dmarc_reports":{d},"tlsrpt_reports":{d},"total_messages":{d},"messages_evaluated":{d},"dkim_spf_pass":{d},"dkim_spf_fail":{d}}} | |
| \\ | |
| , .{ stats.dmarc, stats.tlsrpt, stats.messages, stats.messages, stats.pass, stats.fail }) catch return; |
| \\ {{"period":"{s}","dmarc_reports":{d},"tlsrpt_reports":{d},"messages_evaluated":{d},"dkim_spf_pass":{d},"dkim_spf_fail":{d}}} | ||
| , .{ key, s.dmarc, s.tlsrpt, s.messages, s.pass, s.fail }); |
There was a problem hiding this comment.
Same breaking-change concern for the per-period JSON schema: changing total_messages to messages_evaluated will break downstream parsers. Consider emitting both keys (old+new) or otherwise versioning/deprecating the change consistently across summary outputs.
| \\ {{"period":"{s}","dmarc_reports":{d},"tlsrpt_reports":{d},"messages_evaluated":{d},"dkim_spf_pass":{d},"dkim_spf_fail":{d}}} | |
| , .{ key, s.dmarc, s.tlsrpt, s.messages, s.pass, s.fail }); | |
| \\ {{"period":"{s}","dmarc_reports":{d},"tlsrpt_reports":{d},"total_messages":{d},"messages_evaluated":{d},"dkim_spf_pass":{d},"dkim_spf_fail":{d}}} | |
| , .{ key, s.dmarc, s.tlsrpt, s.messages, s.messages, s.pass, s.fail }); |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address Copilot review: provide a meaningful fallback message when bufPrint fails, instead of an empty string. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
icon_green → icon_ok, icon_yellow → icon_warning, icon_red → icon_critical check_green → check_ok, check_yellow → check_warning, check_red → check_critical Address Copilot review: use names that reflect status semantics rather than colors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
section_prefix,branch_prefix,detail_prefix) with colored icons (●) and dim branch characters (⎿)--formatoption totext|jsonacross all commands, fix summary default format fromjsontotextTotal MessagestoMessages Evaluatedfor clarity, add Begin/End timestamps to show commandevalColor,formatEpoch, and prefix constantsTest plan
zig buildsucceedszig build test— all 30 tests passreports sync— structured output with section headersreports check— colored status icon, dim table headersreports show <id>— colored icon (green/red), pass/fail coloringreports dns— dim ⎿ on first record linereports summary— table format outputreports summary --period month— consistent table formatreports help— unified format option, filters groupingreports enrich/reports aggregate— no leading blank line when run standalone🤖 Generated with Claude Code