Conversation
- Add ExportDailyProjects and ExportRaw functions with proper TSV formatting - Use encoding/csv with tab separator for correct escaping of special characters - Filter blank entries and exclude running entries from exports - Add export CLI command with --format (daily-projects|raw) and --output flags - Comprehensive test coverage for both formats including special character handling - Update spec 006 to mark all tasks as complete
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Changes RequestedPlease make the following small changes before I can approve:
Summary of Changes
Overall Feedback
@Pertempto — once you address the three small items above I will re-review quickly. Thanks! |
- Change ExportDailyProjects and ExportRaw to return (string, error) - Capture and propagate csv.Write errors with context - Check writer.Error() after Flush to catch encoding errors - Update all tests to handle error returns - Update cmd/export.go to propagate errors to CLI exit code - Reason: Previously encoding errors were silently dropped, potentially causing empty/corrupted output without user notification
- Use fmt.Println for stdout output to ensure trailing newline for shell pipeline compatibility - Change file permissions from 0644 to 0600 (owner read/write only) for privacy of sensitive time tracking data, especially important in Docker/CI environments - Add comments explaining the rationale for each change
- Add defensive nil check for entry.End in AggregateByProjectDate (IsRunning() guard should guarantee non-nil, but defensive check prevents potential panics if function is called from elsewhere) - Remove unused Now variable (tests don't depend on it) - Document timezone behavior: ProjectDateEntry.Date uses UTC from time.Parse, intentional to align with date display in stats - Add comment clarifying that date parsing is safe
|
Pushed some commits to address feedback from code review bot. |
This comment was marked as resolved.
This comment was marked as resolved.
- Replace fmt.Println with os.Stdout.WriteString to avoid appending extra newline - Preserves exact TSV output without blank line for accurate downstream parsing - Add error handling for WriteString call - Reason: fmt.Println adds unwanted blank line that breaks TSV parsing expectations
…gregation - Change time.Parse to time.ParseInLocation with entry.Start.Location() - Ensures ProjectDateEntry.Date respects the entry's timezone - "2025-01-01" in user's timezone becomes midnight in that timezone, not UTC - Update docstring to clarify timezone preservation behavior - Improves alignment with local timezone display expectations in stats and exports
- Revert ParseInLocation change, use time.Parse which returns UTC - Entries are stored in UTC, so aggregation should use UTC for consistency - Clarify that display layer (TUI/export) handles timezone conversion - Ensure consistent date-based aggregation regardless of system timezone
|
@bambam955 no need to do in-depth code review for this one. mostly just looking for a sanity check with the testing |
|
I still don't understand how the Docker part works: But until this is resolved I can't actually test anything with the steps given. |
|
Ah, no I think there is something broke with that recipe. It got fixed in another branch. I'll bring that fix over here. 👍🏽 |
|
Actually it was a problem in |
|
@bambam955 It should work now |
|
@Pertempto code looks fine and it tests well. At first I was wishing there was something like Merge whenever 👍 |
Correct. This command is purely for data export. We have the |
Implemented the export command according to spec 6
Testing
First run
just build-dockerNote
The output of the export command on stdout might look funny (misaligned). That is because tabs are often interpreted as just taking the text to the next "tab stop" (usually the next multiple of 8). The key thing is that the fields are correctly separated by tabs. You can pipe the output of the export command to
od -cto see raw characters, or pipe tocat -Awhich will show tabs as^I.Daily Projects
just run-docker exportoutputs all the days with their projects, durations, and descriptionsRaw
just run-docker export -f rawoutputs all the entries with their project, task, start, end, and durationOutput to File
To test the
-ooption, you will have to install/build the actual app, running it inside the docker container will just write a file inside the docker container