feat: added export functionality#172
Conversation
📝 WalkthroughWalkthroughThis change adds export format support (JSON and CSV) to the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as cmd/root.go
participant RenderReport
participant collectReportData
participant renderReportExport
CLI->>RenderReport: call with format
RenderReport->>RenderReport: check interactive vs format
RenderReport->>collectReportData: fetch entries
RenderReport->>renderReportExport: write JSON/CSV
renderReportExport-->>CLI: output written
Estimated code review effort: 3.5/5 (High for report.go and log.go due to control-flow refactoring; Medium elsewhere) Related issues: Not specified in provided context. Related PRs: Not specified in provided context. Suggested labels: enhancement, cli Suggested reviewers: dhth 🥕 A poem for the burrow:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/ui/export.go (1)
90-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
toExportAggEntryandtoExportStatsEntryare duplicate implementations.Both functions have identical bodies converting
types.TaskReportEntryinto structurally identical structs (exportAggEntryandexportStatsEntry, both withTaskID/Task/NumEntries/SecsSpent/TimeSpent). Consider consolidating into a single struct/function and reusing it for both report-agg and stats exports.♻️ Proposed consolidation
-type exportAggEntry struct { - TaskID int `json:"taskId"` - Task string `json:"task"` - NumEntries int `json:"numEntries"` - SecsSpent int `json:"secsSpent"` - TimeSpent string `json:"timeSpent"` -} - -type exportStatsEntry struct { - TaskID int `json:"taskId"` - Task string `json:"task"` - NumEntries int `json:"numEntries"` - SecsSpent int `json:"secsSpent"` - TimeSpent string `json:"timeSpent"` -} +type exportAggEntry struct { + TaskID int `json:"taskId"` + Task string `json:"task"` + NumEntries int `json:"numEntries"` + SecsSpent int `json:"secsSpent"` + TimeSpent string `json:"timeSpent"` +} + +// exportStatsEntry reuses exportAggEntry's shape +type exportStatsEntry = exportAggEntryAnd drop
toExportStatsEntryin favor oftoExportAggEntry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/ui/export.go` around lines 90 - 108, `toExportAggEntry` and `toExportStatsEntry` in export.go are duplicate converters with identical output shapes, so consolidate them into one shared conversion path. Reuse `toExportAggEntry` for both report-agg and stats export flows, and remove `toExportStatsEntry` if it is no longer needed; update the callers in the export code to use the remaining helper so both `exportAggEntry` and `exportStatsEntry` are populated through the same logic.tests/cli/export_test.go (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract repeated CLI flag literals into shared constants.
Static analysis flags
--format(12 occurrences) and--interactive(3 occurrences) as candidates forgoconst. Low priority for a test file but easy to address.Also applies to: 135-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cli/export_test.go` at line 24, The test cases in the CLI export suite are repeating literal flag strings, so extract the shared CLI flag values into constants and reuse them across the affected tests. Update the table-driven cases in the export tests, including the “json today” case and the other occurrences referenced by the review, to use the shared constants for --format and --interactive instead of inline string literals.Source: Linters/SAST tools
cmd/root.go (1)
656-676: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
--plain/-pis silently ignored when--formatis json/csv.Downstream
RenderReport/RenderTaskLog/RenderStatsbranch onformat != types.OutputFormatPlainbefore theplainbool is ever consulted, so passing both-pand--format json|csvsilently drops the-pflag with no error or warning. The updated help text doesn't call out this interaction either.Consider using
cmd.MarkFlagsMutuallyExclusive("plain", "format")(or an explicit check in RunE) to fail fast instead of silently ignoring one of the flags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/root.go` around lines 656 - 676, The report/log/stats commands currently ignore --plain/-p whenever --format is json or csv, because the rendering path checks outputFormat before the plain flag is used. Update the command setup in cmd/root.go for reportCmd, logCmd, and statsCmd so this conflict is handled explicitly, either by marking plain and format as mutually exclusive or by validating the combination in the RunE handlers and returning an error. Make sure the behavior is clear in the CLI contract and that the relevant flag definitions and command handlers stay aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/ui/export.go`:
- Around line 178-213: The CSV writer functions are checking w.Error() before
the final buffered data is flushed, so last-write failures can be missed. Update
writeReportCSV, writeReportAggCSV, writeLogCSV, and writeStatsCSV to call
w.Flush() explicitly before returning, then return w.Error() after that flush.
Remove reliance on defer w.Flush() in these functions so the flush happens
synchronously at the end of each writer.
---
Nitpick comments:
In `@cmd/root.go`:
- Around line 656-676: The report/log/stats commands currently ignore --plain/-p
whenever --format is json or csv, because the rendering path checks outputFormat
before the plain flag is used. Update the command setup in cmd/root.go for
reportCmd, logCmd, and statsCmd so this conflict is handled explicitly, either
by marking plain and format as mutually exclusive or by validating the
combination in the RunE handlers and returning an error. Make sure the behavior
is clear in the CLI contract and that the relevant flag definitions and command
handlers stay aligned.
In `@internal/ui/export.go`:
- Around line 90-108: `toExportAggEntry` and `toExportStatsEntry` in export.go
are duplicate converters with identical output shapes, so consolidate them into
one shared conversion path. Reuse `toExportAggEntry` for both report-agg and
stats export flows, and remove `toExportStatsEntry` if it is no longer needed;
update the callers in the export code to use the remaining helper so both
`exportAggEntry` and `exportStatsEntry` are populated through the same logic.
In `@tests/cli/export_test.go`:
- Line 24: The test cases in the CLI export suite are repeating literal flag
strings, so extract the shared CLI flag values into constants and reuse them
across the affected tests. Update the table-driven cases in the export tests,
including the “json today” case and the other occurrences referenced by the
review, to use the shared constants for --format and --interactive instead of
inline string literals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f59d18dd-c85d-4161-863a-7515e8918254
⛔ Files ignored due to path filters (20)
tests/cli/__snapshots__/TestExportFormatErrors_log_interactive_json_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestExportFormatErrors_report_incorrect_format_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestExportFormatErrors_report_interactive_json_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestExportFormatErrors_stats_interactive_json_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestLogExport_csv_3d_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestLogExport_csv_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestLogExport_json_3d_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestLogExport_json_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestReportExport_csv_3d_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestReportExport_csv_agg_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestReportExport_csv_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestReportExport_json_3d_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestReportExport_json_agg_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestReportExport_json_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestStatsExport_csv_3d_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestStatsExport_csv_all_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestStatsExport_csv_today_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestStatsExport_json_3d_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestStatsExport_json_all_1.snapis excluded by!**/*.snaptests/cli/__snapshots__/TestStatsExport_json_today_1.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
cmd/root.gointernal/types/types.gointernal/ui/export.gointernal/ui/log.gointernal/ui/report.gointernal/ui/stats.gotests/cli/export_test.go
|
Thanks for sending in the PR :) |
- Flush CSV writers before checking errors to avoid missing write failures - Consolidate duplicate exportAggEntry/exportStatsEntry converters - Reject conflicting --plain and --format json/csv flag combinations - Extract repeated CLI flag literals in export tests Co-authored-by: Fabricio Reche <fabricioereche@users.noreply.github.com>
3 participants
Summary
Adds JSON and CSV export support to the
report,log, andstatscommands via a new--format/-fflag. The default behavior (plain) is unchanged and still renders the existing ASCII table output.CLI changes
--format/-fflag toreport,log, andstatsplain(default),json,csvtypes.ParseOutputFormat()and passes it into the corresponding render functionExamples