From 29e4ea103f8b47561b4e10d62d44e1d54366261a Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Sat, 14 Mar 2026 23:35:55 +0100 Subject: [PATCH 1/7] refactor: replace proportional scaling with targeted segment carving for log deduction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual logs now carve their exact time ranges out of overlapping checkout segments (reusing the idle-gap splitting logic) instead of uniformly scaling all checkout minutes. Duration-only logs are placed at the first available schedule slot via FindAvailableSlot() rather than at now-minutes. - Add deductLogOverlaps() in deduct.go (reuses idle gap splitting) - Add FindAvailableSlot() in placement.go for duration-only log placement - Remove deductScheduleOverrun() — no longer needed - Always use segment-based path in BuildReport/BuildDetailedReport/BuildExportData - Update log.go to use findDurationSlot with fallback Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/cli/log.go | 31 +++- internal/cli/log_test.go | 23 ++- internal/timetrack/deduct.go | 48 ++++++ internal/timetrack/deduct_test.go | 246 +++++++++++++++++++++++++++ internal/timetrack/export.go | 32 +--- internal/timetrack/placement.go | 77 +++++++++ internal/timetrack/placement_test.go | 121 +++++++++++++ internal/timetrack/timetrack.go | 102 ++--------- 8 files changed, 566 insertions(+), 114 deletions(-) create mode 100644 internal/timetrack/deduct.go create mode 100644 internal/timetrack/deduct_test.go create mode 100644 internal/timetrack/placement.go create mode 100644 internal/timetrack/placement_test.go diff --git a/internal/cli/log.go b/internal/cli/log.go index c7102d5..b8b0920 100644 --- a/internal/cli/log.go +++ b/internal/cli/log.go @@ -115,9 +115,14 @@ func runLog( if err != nil { return err } - y, m, d := baseDate.Date() - start = time.Date(y, m, d, now.Hour(), now.Minute(), 0, 0, now.Location()). - Add(-time.Duration(minutes) * time.Minute) + // Try to place at the first available schedule slot + start, err = findDurationSlot(homeDir, proj, baseDate, minutes, now) + if err != nil { + // Fall back to now - minutes if no schedule/slot available + y, m, d := baseDate.Date() + start = time.Date(y, m, d, now.Hour(), now.Minute(), 0, 0, now.Location()). + Add(-time.Duration(minutes) * time.Minute) + } } else { if fromFlag == "" { fromFlag, err = pk.Prompt("From (e.g. 9am, 14:00)") @@ -378,6 +383,26 @@ func checkBudgetWarning( return true, nil } +// findDurationSlot attempts to find the first available schedule slot for a +// duration-only log entry. Returns an error if no schedule is configured or +// no slot is available. +func findDurationSlot(homeDir string, proj *project.ProjectEntry, baseDate time.Time, minutes int, now time.Time) (time.Time, error) { + windows, _, _, err := getDayScheduleWindows(homeDir, proj, baseDate) + if err != nil { + return time.Time{}, err + } + if len(windows) == 0 { + return time.Time{}, fmt.Errorf("no schedule windows") + } + + logs, err := entry.ReadAllEntries(homeDir, proj.Slug) + if err != nil { + return time.Time{}, err + } + + return timetrack.FindAvailableSlot(logs, windows, baseDate, minutes, now.Location()) +} + // formatWindowsSummary formats schedule windows as a comma-separated summary. func formatWindowsSummary(windows []schedule.TimeWindow) string { parts := make([]string, len(windows)) diff --git a/internal/cli/log_test.go b/internal/cli/log_test.go index b066c28..6674d4c 100644 --- a/internal/cli/log_test.go +++ b/internal/cli/log_test.go @@ -444,10 +444,31 @@ func TestLogDurationModeWithDate(t *testing.T) { require.NoError(t, err) assert.Len(t, entries, 1) assert.Equal(t, 180, entries[0].Minutes) - // fixedNow is 2025-06-15 14:00 UTC, so start = 2025-01-10 at 14:00 - 3h = 11:00 + // Duration-only log is placed at the first available schedule slot (9:00) assert.Equal(t, 2025, entries[0].Start.Year()) assert.Equal(t, time.January, entries[0].Start.Month()) assert.Equal(t, 10, entries[0].Start.Day()) + assert.Equal(t, 9, entries[0].Start.Hour()) +} + +func TestLogDurationModeUnscheduledDayFallback(t *testing.T) { + homeDir, repoDir, proj := setupLogTest(t) + + // 2025-01-11 is a Saturday — no schedule, so findDurationSlot fails + // and falls back to now - duration (fixedNow is 14:00, so 14:00 - 3h = 11:00) + stdout, err := execLog(homeDir, repoDir, "", "3h", "", "", "2025-01-11", "", "weekend work") + + require.NoError(t, err) + assert.Contains(t, stdout, "logged") + assert.Contains(t, stdout, "3h") + + entries, err := entry.ReadAllEntries(homeDir, proj.Slug) + require.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, 180, entries[0].Minutes) + assert.Equal(t, 2025, entries[0].Start.Year()) + assert.Equal(t, time.January, entries[0].Start.Month()) + assert.Equal(t, 11, entries[0].Start.Day()) assert.Equal(t, 11, entries[0].Start.Hour()) } diff --git a/internal/timetrack/deduct.go b/internal/timetrack/deduct.go new file mode 100644 index 0000000..da2dd22 --- /dev/null +++ b/internal/timetrack/deduct.go @@ -0,0 +1,48 @@ +package timetrack + +import ( + "time" + + "github.com/Flyrell/hourgit/internal/entry" +) + +// deductLogOverlaps removes manual log time ranges from checkout segments, +// similar to how idle gaps are carved out. Each log's [Start, Start+Minutes) +// range is treated as a gap to remove from overlapping segments. +// Checkout-generated entries are skipped (they are already materialized logs). +func deductLogOverlaps( + segments []sessionSegment, + logs []entry.Entry, + year int, month time.Month, + loc *time.Location, +) []sessionSegment { + // Collect log ranges for the target month + var gaps []idleGap + for _, l := range logs { + logStart := l.Start.In(loc) + if logStart.Year() != year || logStart.Month() != month { + continue + } + // Skip checkout-generated entries — they don't reduce checkout time + if l.Source == "checkout-generated" { + continue + } + logEnd := logStart.Add(time.Duration(l.Minutes) * time.Minute) + gaps = append(gaps, idleGap{ + stop: logStart, + start: logEnd, + }) + } + + if len(gaps) == 0 { + return segments + } + + // Reuse the same gap-splitting logic as idle trimming + var result []sessionSegment + for _, seg := range segments { + trimmed := applyGapsToSegment(seg, gaps) + result = append(result, trimmed...) + } + return result +} diff --git a/internal/timetrack/deduct_test.go b/internal/timetrack/deduct_test.go new file mode 100644 index 0000000..4e67346 --- /dev/null +++ b/internal/timetrack/deduct_test.go @@ -0,0 +1,246 @@ +package timetrack + +import ( + "testing" + "time" + + "github.com/Flyrell/hourgit/internal/entry" + "github.com/Flyrell/hourgit/internal/schedule" + "github.com/stretchr/testify/assert" +) + +// splitWorkday returns a DaySchedule with a split schedule: 09:00-11:00, 12:00-17:00. +func splitWorkday(year int, month time.Month, day int) schedule.DaySchedule { + return schedule.DaySchedule{ + Date: time.Date(year, month, day, 0, 0, 0, 0, time.UTC), + Windows: []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 11, Minute: 0}}, + {From: schedule.TimeOfDay{Hour: 12, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + }, + } +} + +// Test Case 1: Checkout "A" at 02:00, checkout "B" at 11:00, manual log 10:00-11:00 +func TestDeductLogOverlaps_LogBetweenCheckouts_Default(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 2, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Previous: "A", Next: "B"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Minutes: 60, Message: "meeting", Task: "meeting"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + rowB := findRow(report, "B") + rowLog := findRow(report, "meeting") + + assert.NotNil(t, rowA) + assert.NotNil(t, rowB) + assert.NotNil(t, rowLog) + + // A: 09:00-10:00 = 60min (log carves 10:00-11:00) + assert.Equal(t, 60, rowA.Days[2]) + // B: 11:00-17:00 = 360min + assert.Equal(t, 360, rowB.Days[2]) + // Log: 60min + assert.Equal(t, 60, rowLog.Days[2]) +} + +func TestDeductLogOverlaps_LogBetweenCheckouts_Split(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{splitWorkday(year, month, 2)} // 9-11, 12-17 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 2, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Previous: "A", Next: "B"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Minutes: 60, Message: "meeting", Task: "meeting"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + rowB := findRow(report, "B") + + assert.NotNil(t, rowA) + assert.NotNil(t, rowB) + + // A: 09:00-10:00 = 60min (log carves 10:00-11:00, schedule only goes to 11:00) + assert.Equal(t, 60, rowA.Days[2]) + // B: 12:00-17:00 = 300min (split schedule, second window) + assert.Equal(t, 300, rowB.Days[2]) +} + +// Test Case 2: Log spanning a lunch gap +func TestDeductLogOverlaps_LogSpanningGap_Default(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 2, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Previous: "A", Next: "B"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Minutes: 120, Message: "meeting", Task: "meeting"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + rowB := findRow(report, "B") + + assert.NotNil(t, rowA) + assert.NotNil(t, rowB) + + // A: 09:00-10:00 = 60min (log carves 10:00-12:00, which also takes from B's start) + assert.Equal(t, 60, rowA.Days[2]) + // B: checkout at 11:00, but log covers 10:00-12:00, so B effective from 12:00-17:00 = 300min + assert.Equal(t, 300, rowB.Days[2]) +} + +// Test Case 3: Log at start of checkout +func TestDeductLogOverlaps_LogAtCheckoutStart_Default(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 17, 0, 0, 0, time.UTC), Previous: "A", Next: "B"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Minutes: 180, Message: "research", Task: "research"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + assert.NotNil(t, rowA) + + // A: checkout 10:00-17:00, log carves 10:00-13:00, so A gets 13:00-17:00 = 240min + assert.Equal(t, 240, rowA.Days[2]) +} + +// Edge case: Log on different day only affects that day +func TestDeductLogOverlaps_LogOnDifferentDay(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2), workday(year, month, 3)} + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2024, 12, 31, 10, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 3, 10, 0, 0, 0, time.UTC), Minutes: 60, Message: "meeting", Task: "meeting"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + assert.NotNil(t, rowA) + + // Day 2: unaffected by log on day 3 + assert.Equal(t, 480, rowA.Days[2]) + // Day 3: log carves 10:00-11:00, A gets 09:00-10:00 + 11:00-17:00 = 420min + assert.Equal(t, 420, rowA.Days[3]) +} + +// Edge case: Log fully contains a segment +func TestDeductLogOverlaps_LogContainsSegment(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Previous: "A", Next: "B"}, + } + // Log covers entire A segment (10:00-11:00) + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Minutes: 120, Message: "meeting", Task: "meeting"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + // A: checkout 10:00-11:00, log covers 09:00-11:00, so A is fully removed + assert.Nil(t, rowA) +} + +// Edge case: Multiple logs on same day +func TestDeductLogOverlaps_MultipleLogs(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2024, 12, 31, 10, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Minutes: 60, Message: "meeting1", Task: "meeting1"}, + {ID: "l2", Start: time.Date(2025, 1, 2, 14, 0, 0, 0, time.UTC), Minutes: 60, Message: "meeting2", Task: "meeting2"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + assert.NotNil(t, rowA) + + // A: 480 - 60 - 60 = 360min (two logs carved out) + assert.Equal(t, 360, rowA.Days[2]) +} + +// Edge case: checkout-generated logs should NOT deduct +func TestDeductLogOverlaps_CheckoutGeneratedSkipped(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2024, 12, 31, 10, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Minutes: 120, Message: "A", Task: "A", Source: "checkout-generated"}, + } + + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil) + + rowA := findRow(report, "A") + assert.NotNil(t, rowA) + + // checkout-generated log should not reduce checkout time — total = 480 + 120 + assert.Equal(t, 480+120, rowA.TotalMinutes) +} + +// Activity start/stop + log interaction +func TestDeductLogOverlaps_WithIdleGaps(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2024, 12, 31, 10, 0, 0, 0, time.UTC), Previous: "main", Next: "A"}, + } + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 14, 0, 0, 0, time.UTC), Minutes: 60, Message: "meeting", Task: "meeting"}, + } + + // Idle gap from 10:00-11:00 + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC)}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC)}, + } + + activity := ActivityEntries{Stops: stops, Starts: starts} + report := BuildReport(checkouts, logs, nil, days, year, month, afterMonth(year, month), nil, activity) + + rowA := findRow(report, "A") + assert.NotNil(t, rowA) + + // Schedule: 9-17 = 480min + // Idle gap carves 10:00-11:00 = -60min → 420min + // Log carves 14:00-15:00 = -60min → 360min + assert.Equal(t, 360, rowA.Days[2]) +} diff --git a/internal/timetrack/export.go b/internal/timetrack/export.go index e87c058..be7a89d 100644 --- a/internal/timetrack/export.go +++ b/internal/timetrack/export.go @@ -66,20 +66,16 @@ func BuildExportData( } } - scheduleWindows, scheduledMins := buildScheduleLookup(daySchedules, year, month) - - // Use segment-based approach when commits are available - var segments []sessionSegment - var checkoutBucket map[string]map[int]int - if len(commits) > 0 { - segments = buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, now) - if len(activity) > 0 && (len(activity[0].Stops) > 0 || len(activity[0].Starts) > 0) { - segments = trimSegmentsByIdleGaps(segments, activity[0].Stops, activity[0].Starts) - } - checkoutBucket = buildSegmentBucket(segments, year, month, daysInMonth, scheduleWindows, now.Location()) - } else { - checkoutBucket = buildCheckoutBucket(checkouts, year, month, daysInMonth, scheduleWindows, now) + scheduleWindows, _ := buildScheduleLookup(daySchedules, year, month) + + loc := now.Location() + segments := buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, now) + if len(activity) > 0 && (len(activity[0].Stops) > 0 || len(activity[0].Starts) > 0) { + segments = trimSegmentsByIdleGaps(segments, activity[0].Stops, activity[0].Starts) } + // Trim manual log time ranges from checkout segments + segments = deductLogOverlaps(segments, logs, year, month, loc) + checkoutBucket := buildSegmentBucket(segments, year, month, daysInMonth, scheduleWindows, loc) // Zero out checkout attribution for generated days for day := range generatedSet { @@ -88,16 +84,6 @@ func BuildExportData( } } - // Build log minutes by day for deduction - logMinsByDay := make(map[int]int) - for _, l := range logs { - if l.Start.Year() == year && l.Start.Month() == month { - logMinsByDay[l.Start.Day()] += l.Minutes - } - } - - deductScheduleOverrun(checkoutBucket, logMinsByDay, scheduledMins, daysInMonth, generatedSet) - // Build per-day data: day -> task -> []ExportEntry type dayTask struct { task string diff --git a/internal/timetrack/placement.go b/internal/timetrack/placement.go new file mode 100644 index 0000000..bdc51f0 --- /dev/null +++ b/internal/timetrack/placement.go @@ -0,0 +1,77 @@ +package timetrack + +import ( + "fmt" + "sort" + "time" + + "github.com/Flyrell/hourgit/internal/entry" + "github.com/Flyrell/hourgit/internal/schedule" +) + +// FindAvailableSlot finds the first available time slot within the schedule +// windows for the given date that can accommodate the requested minutes, +// without overlapping any existing log entries. +func FindAvailableSlot( + existingLogs []entry.Entry, + windows []schedule.TimeWindow, + targetDate time.Time, + minutes int, + loc *time.Location, +) (time.Time, error) { + y, m, d := targetDate.Date() + + // Build occupied ranges from existing logs on targetDate + type timeRange struct { + from, to int // minutes from midnight + } + var occupied []timeRange + for _, l := range existingLogs { + ls := l.Start.In(loc) + if ls.Year() != y || ls.Month() != m || ls.Day() != d { + continue + } + fromMins := ls.Hour()*60 + ls.Minute() + occupied = append(occupied, timeRange{from: fromMins, to: fromMins + l.Minutes}) + } + sort.Slice(occupied, func(i, j int) bool { + return occupied[i].from < occupied[j].from + }) + + // Walk schedule windows chronologically, find first gap that fits + for _, w := range windows { + wFrom := w.From.Hour*60 + w.From.Minute + wTo := w.To.Hour*60 + w.To.Minute + + cursor := wFrom + for _, occ := range occupied { + if occ.to <= cursor { + continue // occupied range is before cursor + } + if occ.from >= wTo { + break // occupied range is past this window + } + // Check gap before this occupied range + if occ.from > cursor { + gap := occ.from - cursor + if gap >= minutes { + return time.Date(y, m, d, cursor/60, cursor%60, 0, 0, loc), nil + } + } + // Move cursor past this occupied range + if occ.to > cursor { + cursor = occ.to + } + } + + // Check remaining gap after all occupied ranges in this window + if cursor < wTo { + gap := wTo - cursor + if gap >= minutes { + return time.Date(y, m, d, cursor/60, cursor%60, 0, 0, loc), nil + } + } + } + + return time.Time{}, fmt.Errorf("no available slot for %s in today's schedule", entry.FormatMinutes(minutes)) +} diff --git a/internal/timetrack/placement_test.go b/internal/timetrack/placement_test.go new file mode 100644 index 0000000..928c796 --- /dev/null +++ b/internal/timetrack/placement_test.go @@ -0,0 +1,121 @@ +package timetrack + +import ( + "testing" + "time" + + "github.com/Flyrell/hourgit/internal/entry" + "github.com/Flyrell/hourgit/internal/schedule" + "github.com/stretchr/testify/assert" +) + +func TestFindAvailableSlot_EmptyDay(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + start, err := FindAvailableSlot(nil, windows, target, 60, time.UTC) + assert.NoError(t, err) + assert.Equal(t, time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), start) +} + +func TestFindAvailableSlot_LogAtStart(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Minutes: 60}, + } + + start, err := FindAvailableSlot(logs, windows, target, 60, time.UTC) + assert.NoError(t, err) + assert.Equal(t, time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), start) +} + +func TestFindAvailableSlot_FitsInSecondWindow(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 11, Minute: 0}}, + {From: schedule.TimeOfDay{Hour: 12, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Minutes: 120}, // fills first window + } + + start, err := FindAvailableSlot(logs, windows, target, 60, time.UTC) + assert.NoError(t, err) + assert.Equal(t, time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), start) +} + +func TestFindAvailableSlot_GapBetweenLogs(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Minutes: 60}, + {ID: "l2", Start: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Minutes: 60}, + } + + // 60min gap at 10:00-11:00 + start, err := FindAvailableSlot(logs, windows, target, 60, time.UTC) + assert.NoError(t, err) + assert.Equal(t, time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), start) +} + +func TestFindAvailableSlot_NoRoom(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 10, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Minutes: 60}, + } + + _, err := FindAvailableSlot(logs, windows, target, 30, time.UTC) + assert.Error(t, err) +} + +func TestFindAvailableSlot_SplitScheduleSkipsGap(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 11, Minute: 0}}, + {From: schedule.TimeOfDay{Hour: 12, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + // Need 3 hours — doesn't fit in first 2h window, but fits in second 5h window + start, err := FindAvailableSlot(nil, windows, target, 180, time.UTC) + assert.NoError(t, err) + assert.Equal(t, time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), start) +} + +func TestFindAvailableSlot_DifferentDayLogsIgnored(t *testing.T) { + windows := []schedule.TimeWindow{ + {From: schedule.TimeOfDay{Hour: 9, Minute: 0}, To: schedule.TimeOfDay{Hour: 17, Minute: 0}}, + } + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + logs := []entry.Entry{ + {ID: "l1", Start: time.Date(2025, 1, 3, 9, 0, 0, 0, time.UTC), Minutes: 480}, // different day + } + + start, err := FindAvailableSlot(logs, windows, target, 60, time.UTC) + assert.NoError(t, err) + assert.Equal(t, time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), start) +} + +func TestFindAvailableSlot_EmptyWindows(t *testing.T) { + target := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + _, err := FindAvailableSlot(nil, nil, target, 60, time.UTC) + assert.Error(t, err) + + _, err = FindAvailableSlot(nil, []schedule.TimeWindow{}, target, 60, time.UTC) + assert.Error(t, err) +} diff --git a/internal/timetrack/timetrack.go b/internal/timetrack/timetrack.go index b9ea270..60c7278 100644 --- a/internal/timetrack/timetrack.go +++ b/internal/timetrack/timetrack.go @@ -101,21 +101,18 @@ func BuildReport( } } - scheduleWindows, scheduledMins := buildScheduleLookup(daySchedules, year, month) - logBucket, logMinsByDay := buildLogBucket(logs, year, month) - - // Use segment-based approach when commits are available - var checkoutBucket map[string]map[int]int - if len(commits) > 0 { - segments := buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, now) - // Trim idle gaps if activity entries provided - if len(activity) > 0 && (len(activity[0].Stops) > 0 || len(activity[0].Starts) > 0) { - segments = trimSegmentsByIdleGaps(segments, activity[0].Stops, activity[0].Starts) - } - checkoutBucket = buildSegmentBucket(segments, year, month, daysInMonth, scheduleWindows, now.Location()) - } else { - checkoutBucket = buildCheckoutBucket(checkouts, year, month, daysInMonth, scheduleWindows, now) + scheduleWindows, _ := buildScheduleLookup(daySchedules, year, month) + logBucket, _ := buildLogBucket(logs, year, month) + + loc := now.Location() + segments := buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, now) + // Trim idle gaps if activity entries provided + if len(activity) > 0 && (len(activity[0].Stops) > 0 || len(activity[0].Starts) > 0) { + segments = trimSegmentsByIdleGaps(segments, activity[0].Stops, activity[0].Starts) } + // Trim manual log time ranges from checkout segments + segments = deductLogOverlaps(segments, logs, year, month, loc) + checkoutBucket := buildSegmentBucket(segments, year, month, daysInMonth, scheduleWindows, loc) // Zero out checkout attribution for generated days for day := range generatedSet { @@ -123,8 +120,6 @@ func BuildReport( delete(checkoutBucket[branch], day) } } - - deductScheduleOverrun(checkoutBucket, logMinsByDay, scheduledMins, daysInMonth, generatedSet) rows := mergeAndSortRows(checkoutBucket, logBucket) return ReportData{ @@ -278,50 +273,6 @@ func buildCheckoutBucket( return checkoutBucket } -// deductScheduleOverrun reduces checkout minutes proportionally when -// checkoutMins + logMins exceed the scheduled minutes for a day. -func deductScheduleOverrun(checkoutBucket map[string]map[int]int, logMinsByDay, scheduledMins map[int]int, daysInMonth int, generatedDays map[int]bool) { - for day := 1; day <= daysInMonth; day++ { - if generatedDays[day] { - continue - } - maxMins := scheduledMins[day] - if maxMins <= 0 { - continue - } - logMins := logMinsByDay[day] - availableForCheckouts := maxMins - logMins - if availableForCheckouts < 0 { - availableForCheckouts = 0 - } - - totalCheckoutMins := 0 - for _, dayMap := range checkoutBucket { - totalCheckoutMins += dayMap[day] - } - - if totalCheckoutMins > availableForCheckouts && totalCheckoutMins > 0 { - ratio := float64(availableForCheckouts) / float64(totalCheckoutMins) - roundedSum := 0 - largestBranch := "" - largestMins := 0 - for branch, dayMap := range checkoutBucket { - dayMap[day] = int(math.Round(float64(dayMap[day]) * ratio)) - roundedSum += dayMap[day] - if dayMap[day] > largestMins { - largestMins = dayMap[day] - largestBranch = branch - } - checkoutBucket[branch] = dayMap - } - // Clamp: if rounding pushed the total over available, subtract excess from the largest branch - if excess := roundedSum - availableForCheckouts; excess > 0 && largestBranch != "" { - checkoutBucket[largestBranch][day] -= excess - } - } - } -} - // mergeAndSortRows merges checkout and log buckets into sorted TaskRows. func mergeAndSortRows(checkoutBucket map[string]map[int]int, logBucket map[string]map[int]int) []TaskRow { rowMap := make(map[string]*TaskRow) @@ -396,9 +347,8 @@ func BuildDetailedReport( segments = trimSegmentsByIdleGaps(segments, activity[0].Stops, activity[0].Starts) } loc := now.Location() - - // Compute aggregated checkout bucket from segments (for schedule deduction) - checkoutBucket := buildSegmentBucket(segments, year, month, daysInMonth, scheduleWindows, loc) + // Trim manual log time ranges from checkout segments + segments = deductLogOverlaps(segments, logs, year, month, loc) // Index persisted checkout-generated entries by (task, day) for deduplication type taskDay struct { @@ -421,7 +371,6 @@ func BuildDetailedReport( rowMap := make(map[string]*DetailedTaskRow) // 1. Add log entries (both manual and checkout-generated) - logMinsByDay := make(map[int]int) for i, l := range logs { if l.Start.Year() != year || l.Start.Month() != month { continue @@ -457,26 +406,12 @@ func BuildDetailedReport( cd.Entries = append(cd.Entries, ce) cd.TotalMinutes += l.Minutes row.TotalMinutes += l.Minutes - logMinsByDay[day] += l.Minutes } - // 2. Compute deducted checkout minutes - deductScheduleOverrun(checkoutBucket, logMinsByDay, scheduledMins, daysInMonth, nil) - - // 3. Build segment cell entries for fine-grained in-memory entries + // 2. Build segment cell entries for fine-grained in-memory entries segEntries := buildSegmentCellEntries(segments, year, month, daysInMonth, scheduleWindows, loc) - // Compute total segment minutes per (branch, day) for proportional deduction - type branchDay struct { - branch string - day int - } - rawSegMins := make(map[branchDay]int) - for _, se := range segEntries { - rawSegMins[branchDay{se.branch, se.day}] += se.minutes - } - - // Add segment entries as in-memory entries, applying deduction ratio + // Add segment entries as in-memory entries (already trimmed by log overlaps) for _, se := range segEntries { dayDate := time.Date(year, month, se.day, 0, 0, 0, 0, time.UTC) if dayDate.Before(from) || dayDate.After(to) { @@ -489,14 +424,7 @@ func BuildDetailedReport( continue } - // Apply proportional deduction from schedule overrun - bdKey := branchDay{se.branch, se.day} - rawTotal := rawSegMins[bdKey] - deducted := checkoutBucket[se.branch][se.day] mins := se.minutes - if rawTotal > 0 && deducted < rawTotal { - mins = int(float64(se.minutes) * float64(deducted) / float64(rawTotal)) - } if mins <= 0 { continue } From 5239798cda759d8078ed72c93df3d45cebd1d17d Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Sun, 15 Mar 2026 00:12:28 +0100 Subject: [PATCH 2/7] feat: add from/to time range display to report detail panel Show HH:MM-HH:MM time range for each entry in the detail panel, prepended before duration and message, so users can see exactly when each tracked segment occurred. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/cli/report_table_view.go | 6 +- internal/cli/report_table_view_test.go | 103 +++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 internal/cli/report_table_view_test.go diff --git a/internal/cli/report_table_view.go b/internal/cli/report_table_view.go index bf3a8de..7f358b3 100644 --- a/internal/cli/report_table_view.go +++ b/internal/cli/report_table_view.go @@ -47,13 +47,17 @@ func (m reportModel) renderDetailPanel() string { marker = "> " } + fromStr := ce.Start.Format("15:04") + toStr := ce.Start.Add(time.Duration(ce.Minutes) * time.Minute).Format("15:04") + timeRange := fmt.Sprintf("%s–%s", fromStr, toStr) + durStr := entry.FormatMinutes(ce.Minutes) msg := ce.Message if msg == "" { msg = "(no message)" } - line := fmt.Sprintf("%s%s %s", marker, padRight(durStr, 7), msg) + line := fmt.Sprintf("%s%s %s %s", marker, padRight(timeRange, 13), padRight(durStr, 7), msg) if i == m.selectedEntryIdx { b.WriteString(headerStyle.Render(line)) } else { diff --git a/internal/cli/report_table_view_test.go b/internal/cli/report_table_view_test.go new file mode 100644 index 0000000..b3c44d8 --- /dev/null +++ b/internal/cli/report_table_view_test.go @@ -0,0 +1,103 @@ +package cli + +import ( + "testing" + "time" + + "github.com/Flyrell/hourgit/internal/timetrack" + "github.com/stretchr/testify/assert" +) + +func buildDetailPanelModel(entries []timetrack.CellEntry, selectedIdx int) reportModel { + row := timetrack.DetailedTaskRow{ + Name: "dev", + Days: map[int]*timetrack.CellData{ + 1: {Entries: entries, TotalMinutes: 0}, + }, + } + return reportModel{ + data: timetrack.DetailedReportData{ + Year: 2026, + Month: time.March, + DaysInMonth: 31, + Rows: []timetrack.DetailedTaskRow{row}, + }, + cursorRow: 0, + cursorCol: 0, + selectedEntryIdx: selectedIdx, + } +} + +func TestRenderDetailPanel_Empty(t *testing.T) { + m := reportModel{ + data: timetrack.DetailedReportData{ + DaysInMonth: 31, + Rows: []timetrack.DetailedTaskRow{{Days: map[int]*timetrack.CellData{}}}, + }, + cursorRow: 0, + cursorCol: 0, + } + assert.Empty(t, m.renderDetailPanel()) +} + +func TestRenderDetailPanel_TimeRange(t *testing.T) { + entries := []timetrack.CellEntry{ + { + Start: time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC), + Minutes: 180, + Message: "morning work", + }, + } + m := buildDetailPanelModel(entries, 0) + out := m.renderDetailPanel() + + assert.Contains(t, out, "09:00") + assert.Contains(t, out, "12:00") + assert.Contains(t, out, "3h") + assert.Contains(t, out, "morning work") +} + +func TestRenderDetailPanel_EmptyMessage(t *testing.T) { + entries := []timetrack.CellEntry{ + { + Start: time.Date(2026, 3, 1, 14, 0, 0, 0, time.UTC), + Minutes: 60, + Message: "", + }, + } + m := buildDetailPanelModel(entries, 0) + out := m.renderDetailPanel() + + assert.Contains(t, out, "(no message)") + assert.Contains(t, out, "14:00") + assert.Contains(t, out, "15:00") +} + +func TestRenderDetailPanel_SelectedMarker(t *testing.T) { + entries := []timetrack.CellEntry{ + {Start: time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC), Minutes: 60, Message: "first"}, + {Start: time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC), Minutes: 60, Message: "second"}, + } + + m := buildDetailPanelModel(entries, 1) + out := m.renderDetailPanel() + + assert.Contains(t, out, "> ") + assert.Contains(t, out, "first") + assert.Contains(t, out, "second") +} + +func TestRenderDetailPanel_MidnightCrossing(t *testing.T) { + entries := []timetrack.CellEntry{ + { + Start: time.Date(2026, 3, 1, 23, 30, 0, 0, time.UTC), + Minutes: 60, + Message: "late night", + }, + } + m := buildDetailPanelModel(entries, 0) + out := m.renderDetailPanel() + + assert.Contains(t, out, "23:30") + assert.Contains(t, out, "00:30") +} From b49f8c37a0c4aa9854f3cfee76c765b34decc5e9 Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Sun, 15 Mar 2026 11:57:07 +0100 Subject: [PATCH 3/7] fix: improve detail panel time range display and test coverage Use hyphen instead of en-dash for time range separator, fix padRight width to 11 to match actual display width, and guard against zero-value Start by falling back to duration-only display. Update tests to assert hyphenated ranges, add NotEmpty guards and time range checks to SelectedMarker test, add ZeroStart test, and fix TotalMinutes computation in helper. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/cli/report_table_view.go | 14 +++++--- internal/cli/report_table_view_test.go | 50 ++++++++++++++++++++------ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/internal/cli/report_table_view.go b/internal/cli/report_table_view.go index 7f358b3..72c5ac2 100644 --- a/internal/cli/report_table_view.go +++ b/internal/cli/report_table_view.go @@ -47,17 +47,21 @@ func (m reportModel) renderDetailPanel() string { marker = "> " } - fromStr := ce.Start.Format("15:04") - toStr := ce.Start.Add(time.Duration(ce.Minutes) * time.Minute).Format("15:04") - timeRange := fmt.Sprintf("%s–%s", fromStr, toStr) - durStr := entry.FormatMinutes(ce.Minutes) msg := ce.Message if msg == "" { msg = "(no message)" } - line := fmt.Sprintf("%s%s %s %s", marker, padRight(timeRange, 13), padRight(durStr, 7), msg) + var line string + if ce.Start.IsZero() { + line = fmt.Sprintf("%s%s %s", marker, padRight(durStr, 7), msg) + } else { + fromStr := ce.Start.Format("15:04") + toStr := ce.Start.Add(time.Duration(ce.Minutes) * time.Minute).Format("15:04") + timeRange := fmt.Sprintf("%s-%s", fromStr, toStr) + line = fmt.Sprintf("%s%s %s %s", marker, padRight(timeRange, 11), padRight(durStr, 7), msg) + } if i == m.selectedEntryIdx { b.WriteString(headerStyle.Render(line)) } else { diff --git a/internal/cli/report_table_view_test.go b/internal/cli/report_table_view_test.go index b3c44d8..0f74df7 100644 --- a/internal/cli/report_table_view_test.go +++ b/internal/cli/report_table_view_test.go @@ -1,6 +1,7 @@ package cli import ( + "strings" "testing" "time" @@ -9,10 +10,14 @@ import ( ) func buildDetailPanelModel(entries []timetrack.CellEntry, selectedIdx int) reportModel { + totalMin := 0 + for _, e := range entries { + totalMin += e.Minutes + } row := timetrack.DetailedTaskRow{ Name: "dev", Days: map[int]*timetrack.CellData{ - 1: {Entries: entries, TotalMinutes: 0}, + 1: {Entries: entries, TotalMinutes: totalMin}, }, } return reportModel{ @@ -51,8 +56,7 @@ func TestRenderDetailPanel_TimeRange(t *testing.T) { m := buildDetailPanelModel(entries, 0) out := m.renderDetailPanel() - assert.Contains(t, out, "09:00") - assert.Contains(t, out, "12:00") + assert.Contains(t, out, "09:00-12:00") assert.Contains(t, out, "3h") assert.Contains(t, out, "morning work") } @@ -69,8 +73,7 @@ func TestRenderDetailPanel_EmptyMessage(t *testing.T) { out := m.renderDetailPanel() assert.Contains(t, out, "(no message)") - assert.Contains(t, out, "14:00") - assert.Contains(t, out, "15:00") + assert.Contains(t, out, "14:00-15:00") } func TestRenderDetailPanel_SelectedMarker(t *testing.T) { @@ -82,9 +85,22 @@ func TestRenderDetailPanel_SelectedMarker(t *testing.T) { m := buildDetailPanelModel(entries, 1) out := m.renderDetailPanel() - assert.Contains(t, out, "> ") - assert.Contains(t, out, "first") - assert.Contains(t, out, "second") + lines := strings.Split(out, "\n") + var firstLine, secondLine string + for _, l := range lines { + if strings.Contains(l, "first") { + firstLine = l + } + if strings.Contains(l, "second") { + secondLine = l + } + } + assert.NotEmpty(t, firstLine, "should find line containing 'first'") + assert.NotEmpty(t, secondLine, "should find line containing 'second'") + assert.NotContains(t, firstLine, "> ", "first entry should not have selection marker") + assert.Contains(t, secondLine, "> ", "second entry should have selection marker") + assert.Contains(t, out, "09:00-10:00") + assert.Contains(t, out, "10:00-11:00") } func TestRenderDetailPanel_MidnightCrossing(t *testing.T) { @@ -98,6 +114,20 @@ func TestRenderDetailPanel_MidnightCrossing(t *testing.T) { m := buildDetailPanelModel(entries, 0) out := m.renderDetailPanel() - assert.Contains(t, out, "23:30") - assert.Contains(t, out, "00:30") + assert.Contains(t, out, "23:30-00:30") +} + +func TestRenderDetailPanel_ZeroStart(t *testing.T) { + entries := []timetrack.CellEntry{ + { + Minutes: 90, + Message: "duration only", + }, + } + m := buildDetailPanelModel(entries, 0) + out := m.renderDetailPanel() + + assert.Contains(t, out, "1h 30m") + assert.Contains(t, out, "duration only") + assert.NotContains(t, out, "00:00-") } From 9e3c9d0379543a9cc43e696227354239c5562054 Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Sun, 15 Mar 2026 12:15:39 +0100 Subject: [PATCH 4/7] feat: add from/to time fields to edit, log, and report overlays Support interdependent from/to/duration flag combinations in edit command (any two of three now works). Add from/to fields to report edit and add overlays with automatic recomputation. Fall back to 9:00 start time for duration-only log entries when no schedule slot is available. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/cli/edit.go | 95 +++++++++--- internal/cli/edit_test.go | 85 ++++++++-- internal/cli/log.go | 7 +- internal/cli/log_test.go | 49 +++++- internal/cli/report_overlay.go | 231 ++++++++++++++++++++++++---- internal/cli/report_overlay_test.go | 214 ++++++++++++++++++++++++-- internal/cli/report_table_update.go | 1 + 7 files changed, 597 insertions(+), 85 deletions(-) diff --git a/internal/cli/edit.go b/internal/cli/edit.go index 93d2fac..1ecb474 100644 --- a/internal/cli/edit.go +++ b/internal/cli/edit.go @@ -6,6 +6,7 @@ import ( "github.com/Flyrell/hourgit/internal/entry" "github.com/Flyrell/hourgit/internal/project" + "github.com/Flyrell/hourgit/internal/schedule" "github.com/spf13/cobra" ) @@ -211,9 +212,9 @@ func applyFlagEdits( hasTo := flagsChanged["to"] hasDate := flagsChanged["date"] - // Mutual exclusivity - if hasDuration && (hasFrom || hasTo) { - return e, fmt.Errorf("--duration and --from/--to are mutually exclusive") + // Over-specified: all three time flags + if hasDuration && hasFrom && hasTo { + return e, fmt.Errorf("--duration, --from, and --to cannot all be specified together") } // Handle date shift @@ -227,36 +228,78 @@ func applyFlagEdits( e.Start = time.Date(y, m, d, e.Start.Hour(), e.Start.Minute(), 0, 0, e.Start.Location()) } - // Handle time changes - if hasDuration { + // Handle time changes — interdependent from/to/duration + switch { + case hasDuration && hasFrom: + // --duration --from → set from, set minutes (to = from + duration) + fromTOD, err := schedule.ParseTimeOfDay(fromFlag) + if err != nil { + return e, fmt.Errorf("invalid --from time: %w", err) + } minutes, err := entry.ParseDuration(durationFlag) if err != nil { return e, err } + y, m, d := e.Start.Date() + e.Start = time.Date(y, m, d, fromTOD.Hour, fromTOD.Minute, 0, 0, e.Start.Location()) e.Minutes = minutes - } else if hasFrom || hasTo { - // Compute current end time - oldEnd := e.Start.Add(time.Duration(e.Minutes) * time.Minute) - oldFromStr := e.Start.Format("15:04") - oldToStr := oldEnd.Format("15:04") - - fromStr := oldFromStr - if hasFrom { - fromStr = fromFlag + + case hasDuration && hasTo: + // --duration --to → compute from = to - duration, set minutes + toTOD, err := schedule.ParseTimeOfDay(toFlag) + if err != nil { + return e, fmt.Errorf("invalid --to time: %w", err) } - toStr := oldToStr - if hasTo { - toStr = toFlag + minutes, err := entry.ParseDuration(durationFlag) + if err != nil { + return e, err } + y, m, d := e.Start.Date() + endTime := time.Date(y, m, d, toTOD.Hour, toTOD.Minute, 0, 0, e.Start.Location()) + e.Start = endTime.Add(-time.Duration(minutes) * time.Minute) + e.Minutes = minutes + case hasDuration: + // --duration alone → keep from, update minutes (to shifts) + minutes, err := entry.ParseDuration(durationFlag) + if err != nil { + return e, err + } + e.Minutes = minutes + + case hasFrom && hasTo: + // --from --to → compute minutes = to - from y, m, d := e.Start.Date() baseDate := time.Date(y, m, d, 0, 0, 0, 0, e.Start.Location()) - start, minutes, err := parseFromTo(fromStr, toStr, baseDate) + start, minutes, err := parseFromTo(fromFlag, toFlag, baseDate) if err != nil { return e, err } e.Start = start e.Minutes = minutes + + case hasFrom: + // --from alone → keep minutes, update from (to shifts) + fromTOD, err := schedule.ParseTimeOfDay(fromFlag) + if err != nil { + return e, fmt.Errorf("invalid --from time: %w", err) + } + y, m, d := e.Start.Date() + e.Start = time.Date(y, m, d, fromTOD.Hour, fromTOD.Minute, 0, 0, e.Start.Location()) + + case hasTo: + // --to alone → keep from, compute minutes = to - from + toTOD, err := schedule.ParseTimeOfDay(toFlag) + if err != nil { + return e, fmt.Errorf("invalid --to time: %w", err) + } + y, m, d := e.Start.Date() + endTime := time.Date(y, m, d, toTOD.Hour, toTOD.Minute, 0, 0, e.Start.Location()) + newMinutes := int(endTime.Sub(e.Start).Minutes()) + if newMinutes <= 0 { + return e, fmt.Errorf("--to (%s) must be after entry start (%s)", toTOD, e.Start.Format("15:04")) + } + e.Minutes = newMinutes } if flagsChanged["task"] { @@ -293,19 +336,23 @@ func applyInteractiveEdits(e entry.Entry, pk PromptKit) (entry.Entry, error) { if err != nil { return e, err } + fromTOD, err := schedule.ParseTimeOfDay(fromStr) + if err != nil { + return e, fmt.Errorf("invalid from time: %w", err) + } - // To - endTime := e.Start.Add(time.Duration(e.Minutes) * time.Minute) - toStr, err := pk.PromptWithDefault("To (e.g. 5pm, 17:00)", endTime.Format("15:04")) + // Duration + durationStr, err := pk.PromptWithDefault("Duration (e.g. 30m, 3h, 3h30m)", entry.FormatMinutes(e.Minutes)) if err != nil { return e, err } - - start, minutes, err := parseFromTo(fromStr, toStr, newDate) + minutes, err := entry.ParseDuration(durationStr) if err != nil { return e, err } - e.Start = start + + y, m, d := newDate.Date() + e.Start = time.Date(y, m, d, fromTOD.Hour, fromTOD.Minute, 0, 0, e.Start.Location()) e.Minutes = minutes // Task diff --git a/internal/cli/edit_test.go b/internal/cli/edit_test.go index 318ccc6..c58e19d 100644 --- a/internal/cli/edit_test.go +++ b/internal/cli/edit_test.go @@ -106,7 +106,7 @@ func TestEditFromToMode(t *testing.T) { func TestEditFromOnly(t *testing.T) { homeDir, repoDir, proj, _ := setupEditTest(t) - // Original: 9:00 - 12:00 (180 min). Change from to 10:00, keep end at 12:00. + // Original: 9:00 - 12:00 (180 min). Change from to 10:00, keep duration (3h) → to shifts to 13:00. flags := map[string]bool{"from": true} stdout, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "", "10am", "", "", "", "") @@ -116,7 +116,7 @@ func TestEditFromOnly(t *testing.T) { e, err := entry.ReadEntry(homeDir, proj.Slug, "ed01234") require.NoError(t, err) assert.Equal(t, 10, e.Start.Hour()) - assert.Equal(t, 120, e.Minutes) // 10:00 - 12:00 = 2h + assert.Equal(t, 180, e.Minutes) // duration preserved: 3h } func TestEditToOnly(t *testing.T) { @@ -211,14 +211,77 @@ func TestEditEmptyMessageError(t *testing.T) { assert.Contains(t, err.Error(), "message is required") } -func TestEditDurationFromToMutuallyExclusive(t *testing.T) { - homeDir, repoDir, _, _ := setupEditTest(t) +func TestEditDurationAndFrom(t *testing.T) { + homeDir, repoDir, proj, _ := setupEditTest(t) + // Original: 9:00-12:00 (180 min). --duration 2h --from 10am → 10:00-12:00 flags := map[string]bool{"duration": true, "from": true} - _, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "2h", "9am", "", "", "", "") + stdout, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "2h", "10am", "", "", "", "") + + require.NoError(t, err) + assert.Contains(t, stdout, "updated entry") + + e, err := entry.ReadEntry(homeDir, proj.Slug, "ed01234") + require.NoError(t, err) + assert.Equal(t, 10, e.Start.Hour()) + assert.Equal(t, 120, e.Minutes) +} + +func TestEditDurationAndTo(t *testing.T) { + homeDir, repoDir, proj, _ := setupEditTest(t) + + // Original: 9:00-12:00 (180 min). --duration 2h --to 2pm → from = 12:00, 2h + flags := map[string]bool{"duration": true, "to": true} + stdout, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "2h", "", "2pm", "", "", "") + + require.NoError(t, err) + assert.Contains(t, stdout, "updated entry") + + e, err := entry.ReadEntry(homeDir, proj.Slug, "ed01234") + require.NoError(t, err) + assert.Equal(t, 12, e.Start.Hour()) + assert.Equal(t, 120, e.Minutes) +} + +func TestEditDurationFromToOverSpecified(t *testing.T) { + homeDir, repoDir, _, _ := setupEditTest(t) + + flags := map[string]bool{"duration": true, "from": true, "to": true} + _, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "2h", "9am", "11am", "", "", "") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot all be specified") +} + +func TestEditToBeforeStart(t *testing.T) { + homeDir, repoDir, _, _ := setupEditTest(t) + + // Original: 9:00-12:00. Set --to to 8am → error (before 9:00 start) + flags := map[string]bool{"to": true} + _, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "", "", "8am", "", "", "") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "must be after") +} + +func TestEditInvalidFromFlag(t *testing.T) { + homeDir, repoDir, _, _ := setupEditTest(t) + + flags := map[string]bool{"from": true} + _, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "", "invalid", "", "", "", "") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid --from time") +} + +func TestEditInvalidToFlag(t *testing.T) { + homeDir, repoDir, _, _ := setupEditTest(t) + + flags := map[string]bool{"to": true} + _, err := execEdit(homeDir, repoDir, "", "ed01234", flags, "", "", "invalid", "", "", "") assert.Error(t, err) - assert.Contains(t, err.Error(), "mutually exclusive") + assert.Contains(t, err.Error(), "invalid --to time") } func TestEditExceeds24Hours(t *testing.T) { @@ -327,8 +390,8 @@ func TestEditInteractiveMode(t *testing.T) { return "2025-06-16", nil // same date case "From (e.g. 9am, 14:00)": return "10:00", nil // change from 9:00 to 10:00 - case "To (e.g. 5pm, 17:00)": - return "12:00", nil // same end + case "Duration (e.g. 30m, 3h, 3h30m)": + return "2h", nil // 2h duration → to = 12:00 case "Task": return "coding", nil // same task case "Message": @@ -349,7 +412,7 @@ func TestEditInteractiveMode(t *testing.T) { e, err := entry.ReadEntry(homeDir, proj.Slug, "ed01234") require.NoError(t, err) assert.Equal(t, 10, e.Start.Hour()) - assert.Equal(t, 120, e.Minutes) // 10:00 - 12:00 + assert.Equal(t, 120, e.Minutes) // from 10:00, duration 2h } func TestEditRegisteredAsSubcommand(t *testing.T) { @@ -376,8 +439,8 @@ func TestEditDateAndFrom(t *testing.T) { assert.Equal(t, time.August, e.Start.Month()) assert.Equal(t, 1, e.Start.Day()) assert.Equal(t, 10, e.Start.Hour()) - // Original end was 12:00, new from is 10:00 → 2h - assert.Equal(t, 120, e.Minutes) + // --from keeps duration (3h), to shifts + assert.Equal(t, 180, e.Minutes) } func TestEditScheduleWarningOutsideHours(t *testing.T) { diff --git a/internal/cli/log.go b/internal/cli/log.go index b8b0920..d042573 100644 --- a/internal/cli/log.go +++ b/internal/cli/log.go @@ -115,13 +115,12 @@ func runLog( if err != nil { return err } - // Try to place at the first available schedule slot start, err = findDurationSlot(homeDir, proj, baseDate, minutes, now) if err != nil { - // Fall back to now - minutes if no schedule/slot available + // No schedule or no room — place at beginning of day (9:00); + // the schedule warning system will inform the user y, m, d := baseDate.Date() - start = time.Date(y, m, d, now.Hour(), now.Minute(), 0, 0, now.Location()). - Add(-time.Duration(minutes) * time.Minute) + start = time.Date(y, m, d, 9, 0, 0, 0, now.Location()) } } else { if fromFlag == "" { diff --git a/internal/cli/log_test.go b/internal/cli/log_test.go index 6674d4c..ca3891f 100644 --- a/internal/cli/log_test.go +++ b/internal/cli/log_test.go @@ -65,6 +65,7 @@ func TestLogDurationMode(t *testing.T) { assert.Len(t, entries, 1) assert.Equal(t, 180, entries[0].Minutes) assert.Equal(t, "did some work", entries[0].Message) + assert.Equal(t, 9, entries[0].Start.Hour()) } func TestLogFromToMode(t *testing.T) { @@ -455,10 +456,19 @@ func TestLogDurationModeUnscheduledDayFallback(t *testing.T) { homeDir, repoDir, proj := setupLogTest(t) // 2025-01-11 is a Saturday — no schedule, so findDurationSlot fails - // and falls back to now - duration (fixedNow is 14:00, so 14:00 - 3h = 11:00) - stdout, err := execLog(homeDir, repoDir, "", "3h", "", "", "2025-01-11", "", "weekend work") + // and falls back to 9:00 default; the schedule warning system will warn the user + confirmed := false + pk := PromptKit{ + Confirm: func(prompt string) (bool, error) { + confirmed = true + return true, nil + }, + } + + stdout, err := execLogWithPrompts(homeDir, repoDir, "", "3h", "", "", "2025-01-11", "", "weekend work", pk) require.NoError(t, err) + assert.True(t, confirmed, "should have warned about unscheduled day") assert.Contains(t, stdout, "logged") assert.Contains(t, stdout, "3h") @@ -469,7 +479,40 @@ func TestLogDurationModeUnscheduledDayFallback(t *testing.T) { assert.Equal(t, 2025, entries[0].Start.Year()) assert.Equal(t, time.January, entries[0].Start.Month()) assert.Equal(t, 11, entries[0].Start.Day()) - assert.Equal(t, 11, entries[0].Start.Hour()) + assert.Equal(t, 9, entries[0].Start.Hour()) +} + +func TestLogDurationModeNoSlotAvailable(t *testing.T) { + homeDir, repoDir, proj := setupLogTest(t) + + // Fill up Monday (2025-06-16, 9am-5pm = 8h) with an existing 8h entry + e := entry.Entry{ + ID: "00f0001", + Start: time.Date(2025, 6, 16, 9, 0, 0, 0, time.UTC), + Minutes: 480, + Message: "full day", + } + require.NoError(t, entry.WriteEntry(homeDir, proj.Slug, e)) + + // Try to log 1h on same day — no slot available, falls back to 9:00 + // and schedule warning fires about exceeding budget + confirmed := false + pk := PromptKit{ + Confirm: func(prompt string) (bool, error) { + confirmed = true + return true, nil + }, + } + + stdout, err := execLogWithPrompts(homeDir, repoDir, "", "1h", "", "", "2025-06-16", "", "extra work", pk) + + require.NoError(t, err) + assert.True(t, confirmed, "should have warned about budget overrun") + assert.Contains(t, stdout, "logged") + + entries, err := entry.ReadAllEntries(homeDir, proj.Slug) + require.NoError(t, err) + assert.Len(t, entries, 2) } func TestLogFromToModeWithDate(t *testing.T) { diff --git a/internal/cli/report_overlay.go b/internal/cli/report_overlay.go index 553297e..72ced70 100644 --- a/internal/cli/report_overlay.go +++ b/internal/cli/report_overlay.go @@ -7,6 +7,7 @@ import ( "github.com/Flyrell/hourgit/internal/entry" "github.com/Flyrell/hourgit/internal/hashutil" + "github.com/Flyrell/hourgit/internal/schedule" "github.com/Flyrell/hourgit/internal/timetrack" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" @@ -166,7 +167,9 @@ func (o *entrySelectorOverlay) selectedEntry() timetrack.CellEntry { type editField int const ( - editFieldDuration editField = iota + editFieldFrom editField = iota + editFieldTo + editFieldDuration editFieldTask editFieldMessage editFieldConfirm @@ -174,6 +177,8 @@ const ( type editOverlay struct { entry timetrack.CellEntry + from string + to string duration string task string message string @@ -182,8 +187,11 @@ type editOverlay struct { } func newEditOverlay(ce timetrack.CellEntry) *editOverlay { + endTime := ce.Start.Add(time.Duration(ce.Minutes) * time.Minute) return &editOverlay{ entry: ce, + from: ce.Start.Format("15:04"), + to: endTime.Format("15:04"), duration: entry.FormatMinutes(ce.Minutes), task: ce.Task, message: ce.Message, @@ -194,6 +202,10 @@ func (o *editOverlay) Init() tea.Cmd { return nil } func (o *editOverlay) activeTextField() *textField { switch o.field { + case editFieldFrom: + return &textField{&o.from} + case editFieldTo: + return &textField{&o.to} case editFieldDuration: return &textField{&o.duration} case editFieldTask: @@ -204,6 +216,70 @@ func (o *editOverlay) activeTextField() *textField { return nil } +// recomputeFromField recalculates to = from + duration when leaving the from field. +func (o *editOverlay) recomputeFromField() { + fromTOD, err := schedule.ParseTimeOfDay(o.from) + if err != nil { + return + } + mins, err := entry.ParseDuration(o.duration) + if err != nil { + return + } + endTime := time.Date(2000, 1, 1, fromTOD.Hour, fromTOD.Minute, 0, 0, time.UTC). + Add(time.Duration(mins) * time.Minute) + o.to = endTime.Format("15:04") +} + +// recomputeToField recalculates duration = to - from when leaving the to field. +func (o *editOverlay) recomputeToField() { + fromTOD, err := schedule.ParseTimeOfDay(o.from) + if err != nil { + return + } + toTOD, err := schedule.ParseTimeOfDay(o.to) + if err != nil { + return + } + fromMins := fromTOD.Hour*60 + fromTOD.Minute + toMins := toTOD.Hour*60 + toTOD.Minute + if toMins > fromMins { + o.duration = entry.FormatMinutes(toMins - fromMins) + } +} + +// recomputeDurationField recalculates to = from + duration when leaving the duration field. +func (o *editOverlay) recomputeDurationField() { + o.recomputeFromField() // same logic: to = from + duration +} + +func (o *editOverlay) advanceField() { + prev := o.field + if o.field < editFieldConfirm { + o.field++ + } + o.recomputeOnLeave(prev) +} + +func (o *editOverlay) retreatField() { + prev := o.field + if o.field > editFieldFrom { + o.field-- + } + o.recomputeOnLeave(prev) +} + +func (o *editOverlay) recomputeOnLeave(prev editField) { + switch prev { + case editFieldFrom: + o.recomputeFromField() + case editFieldTo: + o.recomputeToField() + case editFieldDuration: + o.recomputeDurationField() + } +} + func (o *editOverlay) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case tea.KeyMsg: @@ -211,31 +287,38 @@ func (o *editOverlay) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "esc": return o, overlayResultMsg("cancel", nil) case "tab", "down": - if o.field < editFieldConfirm { - o.field++ - } + o.advanceField() case "shift+tab", "up": - if o.field > editFieldDuration { - o.field-- - } + o.retreatField() case "enter": if o.field == editFieldConfirm { - // Validate duration - mins, err := entry.ParseDuration(o.duration) + fromTOD, err := schedule.ParseTimeOfDay(o.from) if err != nil { - o.err = "Invalid duration (e.g. 2h30m, 90m)" + o.err = "Invalid from time (e.g. 9am, 14:00)" + return o, nil + } + toTOD, err := schedule.ParseTimeOfDay(o.to) + if err != nil { + o.err = "Invalid to time (e.g. 5pm, 17:00)" return o, nil } + fromMins := fromTOD.Hour*60 + fromTOD.Minute + toMins := toTOD.Hour*60 + toTOD.Minute + if toMins <= fromMins { + o.err = "To must be after From" + return o, nil + } + mins := toMins - fromMins + // Update entry start and minutes + y, mo, d := o.entry.Start.Date() + o.entry.Start = time.Date(y, mo, d, fromTOD.Hour, fromTOD.Minute, 0, 0, o.entry.Start.Location()) o.entry.Minutes = mins o.entry.Task = o.task o.entry.Message = o.message o.err = "" return o, overlayResultMsg("edit", nil) } - // Enter on a field moves to next - if o.field < editFieldConfirm { - o.field++ - } + o.advanceField() case "backspace": if tf := o.activeTextField(); tf != nil { tf.deleteChar() @@ -261,6 +344,8 @@ func (o *editOverlay) View() string { value string field editField }{ + {"From", o.from, editFieldFrom}, + {"To", o.to, editFieldTo}, {"Duration", o.duration, editFieldDuration}, {"Task", o.task, editFieldTask}, {"Message", o.message, editFieldMessage}, @@ -305,7 +390,9 @@ func (o *editOverlay) View() string { type addField int const ( - addFieldDuration addField = iota + addFieldFrom addField = iota + addFieldTo + addFieldDuration addFieldTask addFieldMessage addFieldConfirm @@ -316,6 +403,8 @@ type addOverlay struct { month time.Month year int task string // pre-filled from selected row + from string + to string duration string message string field addField @@ -328,6 +417,7 @@ func newAddOverlay(day int, month time.Month, year int, task string) *addOverlay month: month, year: year, task: task, + from: "9:00", } } @@ -335,6 +425,10 @@ func (o *addOverlay) Init() tea.Cmd { return nil } func (o *addOverlay) activeTextField() *textField { switch o.field { + case addFieldFrom: + return &textField{&o.from} + case addFieldTo: + return &textField{&o.to} case addFieldDuration: return &textField{&o.duration} case addFieldTask: @@ -345,6 +439,70 @@ func (o *addOverlay) activeTextField() *textField { return nil } +// recomputeFromField recalculates to = from + duration when leaving the from field. +func (o *addOverlay) recomputeFromField() { + fromTOD, err := schedule.ParseTimeOfDay(o.from) + if err != nil { + return + } + mins, err := entry.ParseDuration(o.duration) + if err != nil { + return + } + endTime := time.Date(2000, 1, 1, fromTOD.Hour, fromTOD.Minute, 0, 0, time.UTC). + Add(time.Duration(mins) * time.Minute) + o.to = endTime.Format("15:04") +} + +// recomputeToField recalculates duration = to - from when leaving the to field. +func (o *addOverlay) recomputeToField() { + fromTOD, err := schedule.ParseTimeOfDay(o.from) + if err != nil { + return + } + toTOD, err := schedule.ParseTimeOfDay(o.to) + if err != nil { + return + } + fromMins := fromTOD.Hour*60 + fromTOD.Minute + toMins := toTOD.Hour*60 + toTOD.Minute + if toMins > fromMins { + o.duration = entry.FormatMinutes(toMins - fromMins) + } +} + +// recomputeDurationField recalculates to = from + duration when leaving the duration field. +func (o *addOverlay) recomputeDurationField() { + o.recomputeFromField() // same logic: to = from + duration +} + +func (o *addOverlay) advanceField() { + prev := o.field + if o.field < addFieldConfirm { + o.field++ + } + o.recomputeOnLeave(prev) +} + +func (o *addOverlay) retreatField() { + prev := o.field + if o.field > addFieldFrom { + o.field-- + } + o.recomputeOnLeave(prev) +} + +func (o *addOverlay) recomputeOnLeave(prev addField) { + switch prev { + case addFieldFrom: + o.recomputeFromField() + case addFieldTo: + o.recomputeToField() + case addFieldDuration: + o.recomputeDurationField() + } +} + func (o *addOverlay) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case tea.KeyMsg: @@ -352,30 +510,44 @@ func (o *addOverlay) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "esc": return o, overlayResultMsg("cancel", nil) case "tab", "down": - if o.field < addFieldConfirm { - o.field++ - } + o.advanceField() case "shift+tab", "up": - if o.field > addFieldDuration { - o.field-- - } + o.retreatField() case "enter": if o.field == addFieldConfirm { + fromTOD, err := schedule.ParseTimeOfDay(o.from) + if err != nil { + o.err = "Invalid from time (e.g. 9am, 14:00)" + return o, nil + } if o.duration == "" { o.err = "Duration is required" return o, nil } - _, err := entry.ParseDuration(o.duration) + mins, err := entry.ParseDuration(o.duration) if err != nil { o.err = "Invalid duration (e.g. 2h30m, 90m)" return o, nil } + // Validate to if provided + if o.to != "" { + toTOD, err := schedule.ParseTimeOfDay(o.to) + if err != nil { + o.err = "Invalid to time (e.g. 5pm, 17:00)" + return o, nil + } + toMins := toTOD.Hour*60 + toTOD.Minute + fromMins := fromTOD.Hour*60 + fromTOD.Minute + if toMins <= fromMins { + o.err = "To must be after From" + return o, nil + } + } + _ = mins o.err = "" return o, overlayResultMsg("add", nil) } - if o.field < addFieldConfirm { - o.field++ - } + o.advanceField() case "backspace": if tf := o.activeTextField(); tf != nil { tf.deleteChar() @@ -401,6 +573,8 @@ func (o *addOverlay) View() string { value string field addField }{ + {"From", o.from, addFieldFrom}, + {"To", o.to, addFieldTo}, {"Duration", o.duration, addFieldDuration}, {"Task", o.task, addFieldTask}, {"Message", o.message, addFieldMessage}, @@ -440,6 +614,11 @@ func (o *addOverlay) View() string { } func (o *addOverlay) buildEntry(now time.Time) (entry.Entry, error) { + fromTOD, err := schedule.ParseTimeOfDay(o.from) + if err != nil { + return entry.Entry{}, fmt.Errorf("invalid from time: %w", err) + } + mins, err := entry.ParseDuration(o.duration) if err != nil { return entry.Entry{}, err @@ -453,7 +632,7 @@ func (o *addOverlay) buildEntry(now time.Time) (entry.Entry, error) { return entry.Entry{ ID: hashutil.GenerateID("add"), - Start: time.Date(o.year, o.month, o.day, 9, 0, 0, 0, time.UTC), + Start: time.Date(o.year, o.month, o.day, fromTOD.Hour, fromTOD.Minute, 0, 0, time.UTC), Minutes: mins, Message: msg, Task: task, diff --git a/internal/cli/report_overlay_test.go b/internal/cli/report_overlay_test.go index 52edf7b..e41e080 100644 --- a/internal/cli/report_overlay_test.go +++ b/internal/cli/report_overlay_test.go @@ -82,22 +82,33 @@ func TestEntrySelectorOverlay_View(t *testing.T) { } func TestEditOverlay_FieldNavigation(t *testing.T) { - ce := timetrack.CellEntry{Minutes: 60, Message: "test", Task: "task"} + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + Task: "task", + } o := newEditOverlay(ce) - assert.Equal(t, editFieldDuration, o.field) + assert.Equal(t, editFieldFrom, o.field) - // Tab to next field + // Tab through all fields: From → To → Duration → Task → Message → Confirm updated, _ := o.Update(tea.KeyMsg{Type: tea.KeyTab}) o = updated.(*editOverlay) + assert.Equal(t, editFieldTo, o.field) + + updated, _ = o.Update(tea.KeyMsg{Type: tea.KeyTab}) + o = updated.(*editOverlay) + assert.Equal(t, editFieldDuration, o.field) + + updated, _ = o.Update(tea.KeyMsg{Type: tea.KeyTab}) + o = updated.(*editOverlay) assert.Equal(t, editFieldTask, o.field) - // Tab again updated, _ = o.Update(tea.KeyMsg{Type: tea.KeyTab}) o = updated.(*editOverlay) assert.Equal(t, editFieldMessage, o.field) - // Tab to confirm updated, _ = o.Update(tea.KeyMsg{Type: tea.KeyTab}) o = updated.(*editOverlay) assert.Equal(t, editFieldConfirm, o.field) @@ -109,38 +120,90 @@ func TestEditOverlay_FieldNavigation(t *testing.T) { } func TestEditOverlay_TextInput(t *testing.T) { - ce := timetrack.CellEntry{Minutes: 60, Message: "", Task: ""} + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "", + Task: "", + } o := newEditOverlay(ce) - // Clear duration and type new one - o.duration = "" - for _, ch := range "2h" { + // First field is From — clear it and type new value + o.from = "" + for _, ch := range "10am" { updated, _ := o.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{ch}}) o = updated.(*editOverlay) } - assert.Equal(t, "2h", o.duration) + assert.Equal(t, "10am", o.from) // Backspace updated, _ := o.Update(tea.KeyMsg{Type: tea.KeyBackspace}) o = updated.(*editOverlay) - assert.Equal(t, "2", o.duration) + assert.Equal(t, "10a", o.from) } -func TestEditOverlay_InvalidDuration(t *testing.T) { - ce := timetrack.CellEntry{Minutes: 60, Message: "test", Task: "task"} +func TestEditOverlay_InvalidFrom(t *testing.T) { + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + Task: "task", + } o := newEditOverlay(ce) - o.duration = "invalid" + o.from = "invalid" o.field = editFieldConfirm updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) o = updated.(*editOverlay) assert.Nil(t, cmd) // No result — stays in overlay - assert.Contains(t, o.err, "Invalid duration") + assert.Contains(t, o.err, "Invalid from time") +} + +func TestEditOverlay_InvalidTo(t *testing.T) { + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + Task: "task", + } + o := newEditOverlay(ce) + o.to = "invalid" + o.field = editFieldConfirm + + updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) + o = updated.(*editOverlay) + assert.Nil(t, cmd) + assert.Contains(t, o.err, "Invalid to time") +} + +func TestEditOverlay_ToBeforeFrom(t *testing.T) { + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + Task: "task", + } + o := newEditOverlay(ce) + o.from = "5pm" + o.to = "9am" + o.field = editFieldConfirm + + updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) + o = updated.(*editOverlay) + assert.Nil(t, cmd) + assert.Contains(t, o.err, "To must be after From") } func TestEditOverlay_ValidSubmit(t *testing.T) { - ce := timetrack.CellEntry{Minutes: 60, Message: "test", Task: "task"} + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + Task: "task", + } o := newEditOverlay(ce) + o.from = "9:00" + o.to = "11:30" o.duration = "2h30m" o.task = "updated-task" o.message = "updated-msg" @@ -154,16 +217,24 @@ func TestEditOverlay_ValidSubmit(t *testing.T) { require.True(t, ok) assert.Equal(t, "edit", result.action) assert.Equal(t, 150, o.entry.Minutes) // 2h30m + assert.Equal(t, 9, o.entry.Start.Hour()) assert.Equal(t, "updated-task", o.entry.Task) assert.Equal(t, "updated-msg", o.entry.Message) } func TestEditOverlay_View(t *testing.T) { - ce := timetrack.CellEntry{Minutes: 60, Message: "test", Task: "task"} + ce := timetrack.CellEntry{ + Minutes: 60, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + Task: "task", + } o := newEditOverlay(ce) view := o.View() assert.Contains(t, view, "Edit Entry") + assert.Contains(t, view, "From") + assert.Contains(t, view, "To") assert.Contains(t, view, "Duration") assert.Contains(t, view, "Task") assert.Contains(t, view, "Message") @@ -396,6 +467,8 @@ func TestReportModel_HandleEdit_PersistsInMemoryEntry(t *testing.T) { } editOv := newEditOverlay(ce) + editOv.from = "9:00" + editOv.to = "11:00" editOv.duration = "2h" editOv.task = "feature-x" editOv.message = "feature-x" @@ -432,3 +505,110 @@ func TestReportModel_HandleEdit_PersistsInMemoryEntry(t *testing.T) { assert.Nil(t, rm.overlay) assert.Contains(t, rm.footerMsg, "saved") } + +func TestEditOverlay_Interdependency_FromRecomputesTo(t *testing.T) { + ce := timetrack.CellEntry{ + Minutes: 120, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + } + o := newEditOverlay(ce) + // Initial: from=9:00, to=11:00, duration=2h + assert.Equal(t, "09:00", o.from) + assert.Equal(t, "11:00", o.to) + assert.Equal(t, "2h", o.duration) + + // Change from to 10:00 and tab away → to should become 12:00 (keeping 2h duration) + o.from = "10:00" + // Advance field triggers recompute + o.advanceField() + assert.Equal(t, "12:00", o.to) + assert.Equal(t, "2h", o.duration) +} + +func TestEditOverlay_Interdependency_ToRecomputesDuration(t *testing.T) { + ce := timetrack.CellEntry{ + Minutes: 120, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + } + o := newEditOverlay(ce) + // Navigate to To field + o.field = editFieldTo + + // Change to to 14:00 and tab away → duration should become 5h (9:00 to 14:00) + o.to = "14:00" + o.advanceField() + assert.Equal(t, "5h", o.duration) + assert.Equal(t, "09:00", o.from) // from unchanged +} + +func TestEditOverlay_Interdependency_DurationRecomputesTo(t *testing.T) { + ce := timetrack.CellEntry{ + Minutes: 120, + Start: time.Date(2025, 1, 5, 9, 0, 0, 0, time.UTC), + Message: "test", + } + o := newEditOverlay(ce) + // Navigate to Duration field + o.field = editFieldDuration + + // Change duration to 4h and tab away → to should become 13:00 (9:00 + 4h) + o.duration = "4h" + o.advanceField() + assert.Equal(t, "13:00", o.to) + assert.Equal(t, "09:00", o.from) // from unchanged +} + +func TestAddOverlay_FieldNavigation(t *testing.T) { + o := newAddOverlay(5, time.January, 2025, "my-task") + + assert.Equal(t, addFieldFrom, o.field) + + // Tab through: From → To → Duration → Task → Message → Confirm + o.advanceField() + assert.Equal(t, addFieldTo, o.field) + o.advanceField() + assert.Equal(t, addFieldDuration, o.field) + o.advanceField() + assert.Equal(t, addFieldTask, o.field) + o.advanceField() + assert.Equal(t, addFieldMessage, o.field) + o.advanceField() + assert.Equal(t, addFieldConfirm, o.field) + + // Back + o.retreatField() + assert.Equal(t, addFieldMessage, o.field) +} + +func TestAddOverlay_Interdependency(t *testing.T) { + o := newAddOverlay(5, time.January, 2025, "task") + o.from = "9:00" + o.duration = "3h" + + // Leaving from → recomputes to + o.field = addFieldFrom + o.advanceField() + assert.Equal(t, "12:00", o.to) + + // Change to and leave → recomputes duration + o.to = "14:00" + o.field = addFieldTo + o.advanceField() + assert.Equal(t, "5h", o.duration) +} + +func TestAddOverlay_BuildEntryUsesFrom(t *testing.T) { + o := newAddOverlay(5, time.January, 2025, "my-task") + o.from = "10:30" + o.duration = "2h" + o.message = "did stuff" + + now := time.Date(2025, 1, 5, 14, 0, 0, 0, time.UTC) + e, err := o.buildEntry(now) + require.NoError(t, err) + assert.Equal(t, 120, e.Minutes) + assert.Equal(t, 10, e.Start.Hour()) + assert.Equal(t, 30, e.Start.Minute()) +} diff --git a/internal/cli/report_table_update.go b/internal/cli/report_table_update.go index 67b3cbd..1247cd8 100644 --- a/internal/cli/report_table_update.go +++ b/internal/cli/report_table_update.go @@ -149,6 +149,7 @@ func (m reportModel) handleEdit() (tea.Model, tea.Cmd) { if ce.Persisted && ce.Entry != nil { // Update existing persisted entry + ce.Entry.Start = ce.Start ce.Entry.Minutes = ce.Minutes ce.Entry.Task = ce.Task ce.Entry.Message = ce.Message From 02fb3d6897df6d854a4cf9bd5cbbed63197e8223 Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Sun, 15 Mar 2026 12:28:04 +0100 Subject: [PATCH 5/7] refactor: extract shared timeFields helper and add buildEntry guard for to <= from Deduplicate from/to/duration recomputation logic between editOverlay and addOverlay into a shared timeFields struct. Add a defensive guard in addOverlay.buildEntry() that rejects to <= from, preventing negative Minutes values. Includes TestAddOverlay_BuildEntryToBeforeFrom test. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/cli/report_overlay.go | 256 ++++++++++++---------------- internal/cli/report_overlay_test.go | 107 +++++++----- 2 files changed, 181 insertions(+), 182 deletions(-) diff --git a/internal/cli/report_overlay.go b/internal/cli/report_overlay.go index 72ced70..a2cfdde 100644 --- a/internal/cli/report_overlay.go +++ b/internal/cli/report_overlay.go @@ -35,6 +35,9 @@ var ( overlayMutedStyle = lipgloss.NewStyle().Faint(true) ) +// defaultStartHour is the default start hour used when no schedule is available. +const defaultStartHour = 9 + // --- Text field helper --- // textField provides insertChar/deleteChar on a string pointer. @@ -50,6 +53,53 @@ func (f textField) deleteChar() { } } +// --- Time fields helper --- + +// timeFields provides shared from/to/duration recomputation logic +// used by both editOverlay and addOverlay. +type timeFields struct { + from string + to string + duration string +} + +// recomputeFromField recalculates to = from + duration when leaving the from field. +func (tf *timeFields) recomputeFromField() { + fromTOD, err := schedule.ParseTimeOfDay(tf.from) + if err != nil { + return + } + mins, err := entry.ParseDuration(tf.duration) + if err != nil { + return + } + endTime := time.Date(2000, 1, 1, fromTOD.Hour, fromTOD.Minute, 0, 0, time.UTC). + Add(time.Duration(mins) * time.Minute) + tf.to = endTime.Format("15:04") +} + +// recomputeToField recalculates duration = to - from when leaving the to field. +func (tf *timeFields) recomputeToField() { + fromTOD, err := schedule.ParseTimeOfDay(tf.from) + if err != nil { + return + } + toTOD, err := schedule.ParseTimeOfDay(tf.to) + if err != nil { + return + } + fromMins := fromTOD.Hour*60 + fromTOD.Minute + toMins := toTOD.Hour*60 + toTOD.Minute + if toMins > fromMins { + tf.duration = entry.FormatMinutes(toMins - fromMins) + } +} + +// recomputeDurationField recalculates to = from + duration when leaving the duration field. +func (tf *timeFields) recomputeDurationField() { + tf.recomputeFromField() // same logic: to = from + duration +} + // --- Confirmation overlay helper --- // confirmOverlay provides shared Update/button-rendering for yes/no confirmation overlays. @@ -176,25 +226,25 @@ const ( ) type editOverlay struct { - entry timetrack.CellEntry - from string - to string - duration string - task string - message string - field editField - err string + entry timetrack.CellEntry + times timeFields + task string + message string + field editField + err string } func newEditOverlay(ce timetrack.CellEntry) *editOverlay { endTime := ce.Start.Add(time.Duration(ce.Minutes) * time.Minute) return &editOverlay{ - entry: ce, - from: ce.Start.Format("15:04"), - to: endTime.Format("15:04"), - duration: entry.FormatMinutes(ce.Minutes), - task: ce.Task, - message: ce.Message, + entry: ce, + times: timeFields{ + from: ce.Start.Format("15:04"), + to: endTime.Format("15:04"), + duration: entry.FormatMinutes(ce.Minutes), + }, + task: ce.Task, + message: ce.Message, } } @@ -203,11 +253,11 @@ func (o *editOverlay) Init() tea.Cmd { return nil } func (o *editOverlay) activeTextField() *textField { switch o.field { case editFieldFrom: - return &textField{&o.from} + return &textField{&o.times.from} case editFieldTo: - return &textField{&o.to} + return &textField{&o.times.to} case editFieldDuration: - return &textField{&o.duration} + return &textField{&o.times.duration} case editFieldTask: return &textField{&o.task} case editFieldMessage: @@ -216,43 +266,6 @@ func (o *editOverlay) activeTextField() *textField { return nil } -// recomputeFromField recalculates to = from + duration when leaving the from field. -func (o *editOverlay) recomputeFromField() { - fromTOD, err := schedule.ParseTimeOfDay(o.from) - if err != nil { - return - } - mins, err := entry.ParseDuration(o.duration) - if err != nil { - return - } - endTime := time.Date(2000, 1, 1, fromTOD.Hour, fromTOD.Minute, 0, 0, time.UTC). - Add(time.Duration(mins) * time.Minute) - o.to = endTime.Format("15:04") -} - -// recomputeToField recalculates duration = to - from when leaving the to field. -func (o *editOverlay) recomputeToField() { - fromTOD, err := schedule.ParseTimeOfDay(o.from) - if err != nil { - return - } - toTOD, err := schedule.ParseTimeOfDay(o.to) - if err != nil { - return - } - fromMins := fromTOD.Hour*60 + fromTOD.Minute - toMins := toTOD.Hour*60 + toTOD.Minute - if toMins > fromMins { - o.duration = entry.FormatMinutes(toMins - fromMins) - } -} - -// recomputeDurationField recalculates to = from + duration when leaving the duration field. -func (o *editOverlay) recomputeDurationField() { - o.recomputeFromField() // same logic: to = from + duration -} - func (o *editOverlay) advanceField() { prev := o.field if o.field < editFieldConfirm { @@ -272,11 +285,11 @@ func (o *editOverlay) retreatField() { func (o *editOverlay) recomputeOnLeave(prev editField) { switch prev { case editFieldFrom: - o.recomputeFromField() + o.times.recomputeFromField() case editFieldTo: - o.recomputeToField() + o.times.recomputeToField() case editFieldDuration: - o.recomputeDurationField() + o.times.recomputeDurationField() } } @@ -292,12 +305,12 @@ func (o *editOverlay) Update(msg tea.Msg) (tea.Model, tea.Cmd) { o.retreatField() case "enter": if o.field == editFieldConfirm { - fromTOD, err := schedule.ParseTimeOfDay(o.from) + fromTOD, err := schedule.ParseTimeOfDay(o.times.from) if err != nil { o.err = "Invalid from time (e.g. 9am, 14:00)" return o, nil } - toTOD, err := schedule.ParseTimeOfDay(o.to) + toTOD, err := schedule.ParseTimeOfDay(o.times.to) if err != nil { o.err = "Invalid to time (e.g. 5pm, 17:00)" return o, nil @@ -344,9 +357,9 @@ func (o *editOverlay) View() string { value string field editField }{ - {"From", o.from, editFieldFrom}, - {"To", o.to, editFieldTo}, - {"Duration", o.duration, editFieldDuration}, + {"From", o.times.from, editFieldFrom}, + {"To", o.times.to, editFieldTo}, + {"Duration", o.times.duration, editFieldDuration}, {"Task", o.task, editFieldTask}, {"Message", o.message, editFieldMessage}, } @@ -399,16 +412,14 @@ const ( ) type addOverlay struct { - day int - month time.Month - year int - task string // pre-filled from selected row - from string - to string - duration string - message string - field addField - err string + day int + month time.Month + year int + task string // pre-filled from selected row + times timeFields + message string + field addField + err string } func newAddOverlay(day int, month time.Month, year int, task string) *addOverlay { @@ -417,7 +428,9 @@ func newAddOverlay(day int, month time.Month, year int, task string) *addOverlay month: month, year: year, task: task, - from: "9:00", + times: timeFields{ + from: fmt.Sprintf("%d:00", defaultStartHour), + }, } } @@ -426,11 +439,11 @@ func (o *addOverlay) Init() tea.Cmd { return nil } func (o *addOverlay) activeTextField() *textField { switch o.field { case addFieldFrom: - return &textField{&o.from} + return &textField{&o.times.from} case addFieldTo: - return &textField{&o.to} + return &textField{&o.times.to} case addFieldDuration: - return &textField{&o.duration} + return &textField{&o.times.duration} case addFieldTask: return &textField{&o.task} case addFieldMessage: @@ -439,43 +452,6 @@ func (o *addOverlay) activeTextField() *textField { return nil } -// recomputeFromField recalculates to = from + duration when leaving the from field. -func (o *addOverlay) recomputeFromField() { - fromTOD, err := schedule.ParseTimeOfDay(o.from) - if err != nil { - return - } - mins, err := entry.ParseDuration(o.duration) - if err != nil { - return - } - endTime := time.Date(2000, 1, 1, fromTOD.Hour, fromTOD.Minute, 0, 0, time.UTC). - Add(time.Duration(mins) * time.Minute) - o.to = endTime.Format("15:04") -} - -// recomputeToField recalculates duration = to - from when leaving the to field. -func (o *addOverlay) recomputeToField() { - fromTOD, err := schedule.ParseTimeOfDay(o.from) - if err != nil { - return - } - toTOD, err := schedule.ParseTimeOfDay(o.to) - if err != nil { - return - } - fromMins := fromTOD.Hour*60 + fromTOD.Minute - toMins := toTOD.Hour*60 + toTOD.Minute - if toMins > fromMins { - o.duration = entry.FormatMinutes(toMins - fromMins) - } -} - -// recomputeDurationField recalculates to = from + duration when leaving the duration field. -func (o *addOverlay) recomputeDurationField() { - o.recomputeFromField() // same logic: to = from + duration -} - func (o *addOverlay) advanceField() { prev := o.field if o.field < addFieldConfirm { @@ -495,11 +471,11 @@ func (o *addOverlay) retreatField() { func (o *addOverlay) recomputeOnLeave(prev addField) { switch prev { case addFieldFrom: - o.recomputeFromField() + o.times.recomputeFromField() case addFieldTo: - o.recomputeToField() + o.times.recomputeToField() case addFieldDuration: - o.recomputeDurationField() + o.times.recomputeDurationField() } } @@ -515,35 +491,22 @@ func (o *addOverlay) Update(msg tea.Msg) (tea.Model, tea.Cmd) { o.retreatField() case "enter": if o.field == addFieldConfirm { - fromTOD, err := schedule.ParseTimeOfDay(o.from) + fromTOD, err := schedule.ParseTimeOfDay(o.times.from) if err != nil { o.err = "Invalid from time (e.g. 9am, 14:00)" return o, nil } - if o.duration == "" { - o.err = "Duration is required" - return o, nil - } - mins, err := entry.ParseDuration(o.duration) + toTOD, err := schedule.ParseTimeOfDay(o.times.to) if err != nil { - o.err = "Invalid duration (e.g. 2h30m, 90m)" + o.err = "Invalid to time (e.g. 5pm, 17:00)" return o, nil } - // Validate to if provided - if o.to != "" { - toTOD, err := schedule.ParseTimeOfDay(o.to) - if err != nil { - o.err = "Invalid to time (e.g. 5pm, 17:00)" - return o, nil - } - toMins := toTOD.Hour*60 + toTOD.Minute - fromMins := fromTOD.Hour*60 + fromTOD.Minute - if toMins <= fromMins { - o.err = "To must be after From" - return o, nil - } + fromMins := fromTOD.Hour*60 + fromTOD.Minute + toMins := toTOD.Hour*60 + toTOD.Minute + if toMins <= fromMins { + o.err = "To must be after From" + return o, nil } - _ = mins o.err = "" return o, overlayResultMsg("add", nil) } @@ -573,9 +536,9 @@ func (o *addOverlay) View() string { value string field addField }{ - {"From", o.from, addFieldFrom}, - {"To", o.to, addFieldTo}, - {"Duration", o.duration, addFieldDuration}, + {"From", o.times.from, addFieldFrom}, + {"To", o.times.to, addFieldTo}, + {"Duration", o.times.duration, addFieldDuration}, {"Task", o.task, addFieldTask}, {"Message", o.message, addFieldMessage}, } @@ -614,15 +577,22 @@ func (o *addOverlay) View() string { } func (o *addOverlay) buildEntry(now time.Time) (entry.Entry, error) { - fromTOD, err := schedule.ParseTimeOfDay(o.from) + fromTOD, err := schedule.ParseTimeOfDay(o.times.from) if err != nil { return entry.Entry{}, fmt.Errorf("invalid from time: %w", err) } - mins, err := entry.ParseDuration(o.duration) + toTOD, err := schedule.ParseTimeOfDay(o.times.to) if err != nil { - return entry.Entry{}, err + return entry.Entry{}, fmt.Errorf("invalid to time: %w", err) + } + + fromMins := fromTOD.Hour*60 + fromTOD.Minute + toMins := toTOD.Hour*60 + toTOD.Minute + if toMins <= fromMins { + return entry.Entry{}, fmt.Errorf("to must be after from") } + mins := toMins - fromMins task := o.task msg := o.message diff --git a/internal/cli/report_overlay_test.go b/internal/cli/report_overlay_test.go index e41e080..8c08cab 100644 --- a/internal/cli/report_overlay_test.go +++ b/internal/cli/report_overlay_test.go @@ -129,17 +129,17 @@ func TestEditOverlay_TextInput(t *testing.T) { o := newEditOverlay(ce) // First field is From — clear it and type new value - o.from = "" + o.times.from = "" for _, ch := range "10am" { updated, _ := o.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{ch}}) o = updated.(*editOverlay) } - assert.Equal(t, "10am", o.from) + assert.Equal(t, "10am", o.times.from) // Backspace updated, _ := o.Update(tea.KeyMsg{Type: tea.KeyBackspace}) o = updated.(*editOverlay) - assert.Equal(t, "10a", o.from) + assert.Equal(t, "10a", o.times.from) } func TestEditOverlay_InvalidFrom(t *testing.T) { @@ -150,7 +150,7 @@ func TestEditOverlay_InvalidFrom(t *testing.T) { Task: "task", } o := newEditOverlay(ce) - o.from = "invalid" + o.times.from = "invalid" o.field = editFieldConfirm updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) @@ -167,7 +167,7 @@ func TestEditOverlay_InvalidTo(t *testing.T) { Task: "task", } o := newEditOverlay(ce) - o.to = "invalid" + o.times.to = "invalid" o.field = editFieldConfirm updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) @@ -184,8 +184,8 @@ func TestEditOverlay_ToBeforeFrom(t *testing.T) { Task: "task", } o := newEditOverlay(ce) - o.from = "5pm" - o.to = "9am" + o.times.from = "5pm" + o.times.to = "9am" o.field = editFieldConfirm updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) @@ -202,9 +202,9 @@ func TestEditOverlay_ValidSubmit(t *testing.T) { Task: "task", } o := newEditOverlay(ce) - o.from = "9:00" - o.to = "11:30" - o.duration = "2h30m" + o.times.from = "9:00" + o.times.to = "11:30" + o.times.duration = "2h30m" o.task = "updated-task" o.message = "updated-msg" o.field = editFieldConfirm @@ -243,7 +243,9 @@ func TestEditOverlay_View(t *testing.T) { func TestAddOverlay_ValidSubmit(t *testing.T) { o := newAddOverlay(5, time.January, 2025, "my-task") - o.duration = "1h" + o.times.from = "9:00" + o.times.to = "10:00" + o.times.duration = "1h" o.message = "new work" o.field = addFieldConfirm @@ -256,20 +258,34 @@ func TestAddOverlay_ValidSubmit(t *testing.T) { assert.Equal(t, "add", result.action) } -func TestAddOverlay_EmptyDuration(t *testing.T) { +func TestAddOverlay_EmptyTo(t *testing.T) { o := newAddOverlay(5, time.January, 2025, "task") - o.duration = "" + o.times.to = "" o.field = addFieldConfirm updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) o = updated.(*addOverlay) assert.Nil(t, cmd) - assert.Contains(t, o.err, "Duration is required") + assert.Contains(t, o.err, "Invalid to time") +} + +func TestAddOverlay_ToBeforeFrom(t *testing.T) { + o := newAddOverlay(5, time.January, 2025, "task") + o.times.from = "5pm" + o.times.to = "9am" + o.field = addFieldConfirm + + updated, cmd := o.Update(tea.KeyMsg{Type: tea.KeyEnter}) + o = updated.(*addOverlay) + assert.Nil(t, cmd) + assert.Contains(t, o.err, "To must be after From") } func TestAddOverlay_BuildEntry(t *testing.T) { o := newAddOverlay(5, time.January, 2025, "my-task") - o.duration = "2h" + o.times.from = "9:00" + o.times.to = "11:00" + o.times.duration = "2h" o.message = "did stuff" now := time.Date(2025, 1, 5, 14, 0, 0, 0, time.UTC) @@ -284,7 +300,9 @@ func TestAddOverlay_BuildEntry(t *testing.T) { func TestAddOverlay_BuildEntryMessageFallback(t *testing.T) { o := newAddOverlay(5, time.January, 2025, "my-task") - o.duration = "1h" + o.times.from = "9:00" + o.times.to = "10:00" + o.times.duration = "1h" o.message = "" // empty message should fall back to task now := time.Date(2025, 1, 5, 14, 0, 0, 0, time.UTC) @@ -467,9 +485,9 @@ func TestReportModel_HandleEdit_PersistsInMemoryEntry(t *testing.T) { } editOv := newEditOverlay(ce) - editOv.from = "9:00" - editOv.to = "11:00" - editOv.duration = "2h" + editOv.times.from = "9:00" + editOv.times.to = "11:00" + editOv.times.duration = "2h" editOv.task = "feature-x" editOv.message = "feature-x" editOv.field = editFieldConfirm @@ -514,16 +532,16 @@ func TestEditOverlay_Interdependency_FromRecomputesTo(t *testing.T) { } o := newEditOverlay(ce) // Initial: from=9:00, to=11:00, duration=2h - assert.Equal(t, "09:00", o.from) - assert.Equal(t, "11:00", o.to) - assert.Equal(t, "2h", o.duration) + assert.Equal(t, "09:00", o.times.from) + assert.Equal(t, "11:00", o.times.to) + assert.Equal(t, "2h", o.times.duration) // Change from to 10:00 and tab away → to should become 12:00 (keeping 2h duration) - o.from = "10:00" + o.times.from = "10:00" // Advance field triggers recompute o.advanceField() - assert.Equal(t, "12:00", o.to) - assert.Equal(t, "2h", o.duration) + assert.Equal(t, "12:00", o.times.to) + assert.Equal(t, "2h", o.times.duration) } func TestEditOverlay_Interdependency_ToRecomputesDuration(t *testing.T) { @@ -537,10 +555,10 @@ func TestEditOverlay_Interdependency_ToRecomputesDuration(t *testing.T) { o.field = editFieldTo // Change to to 14:00 and tab away → duration should become 5h (9:00 to 14:00) - o.to = "14:00" + o.times.to = "14:00" o.advanceField() - assert.Equal(t, "5h", o.duration) - assert.Equal(t, "09:00", o.from) // from unchanged + assert.Equal(t, "5h", o.times.duration) + assert.Equal(t, "09:00", o.times.from) // from unchanged } func TestEditOverlay_Interdependency_DurationRecomputesTo(t *testing.T) { @@ -554,10 +572,10 @@ func TestEditOverlay_Interdependency_DurationRecomputesTo(t *testing.T) { o.field = editFieldDuration // Change duration to 4h and tab away → to should become 13:00 (9:00 + 4h) - o.duration = "4h" + o.times.duration = "4h" o.advanceField() - assert.Equal(t, "13:00", o.to) - assert.Equal(t, "09:00", o.from) // from unchanged + assert.Equal(t, "13:00", o.times.to) + assert.Equal(t, "09:00", o.times.from) // from unchanged } func TestAddOverlay_FieldNavigation(t *testing.T) { @@ -584,25 +602,26 @@ func TestAddOverlay_FieldNavigation(t *testing.T) { func TestAddOverlay_Interdependency(t *testing.T) { o := newAddOverlay(5, time.January, 2025, "task") - o.from = "9:00" - o.duration = "3h" + o.times.from = "9:00" + o.times.duration = "3h" // Leaving from → recomputes to o.field = addFieldFrom o.advanceField() - assert.Equal(t, "12:00", o.to) + assert.Equal(t, "12:00", o.times.to) // Change to and leave → recomputes duration - o.to = "14:00" + o.times.to = "14:00" o.field = addFieldTo o.advanceField() - assert.Equal(t, "5h", o.duration) + assert.Equal(t, "5h", o.times.duration) } -func TestAddOverlay_BuildEntryUsesFrom(t *testing.T) { +func TestAddOverlay_BuildEntryUsesFromTo(t *testing.T) { o := newAddOverlay(5, time.January, 2025, "my-task") - o.from = "10:30" - o.duration = "2h" + o.times.from = "10:30" + o.times.to = "12:30" + o.times.duration = "2h" o.message = "did stuff" now := time.Date(2025, 1, 5, 14, 0, 0, 0, time.UTC) @@ -612,3 +631,13 @@ func TestAddOverlay_BuildEntryUsesFrom(t *testing.T) { assert.Equal(t, 10, e.Start.Hour()) assert.Equal(t, 30, e.Start.Minute()) } + +func TestAddOverlay_BuildEntryToBeforeFrom(t *testing.T) { + o := newAddOverlay(5, time.January, 2025, "task") + o.times.from = "5pm" + o.times.to = "9am" + + _, err := o.buildEntry(time.Now()) + require.Error(t, err) + assert.Contains(t, err.Error(), "to must be after from") +} From 7bbee5aeaeef1591275c98116ad7a1548671b180 Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Sun, 15 Mar 2026 12:29:33 +0100 Subject: [PATCH 6/7] docs: update edit flag docs and use defaultStartHour constant in log Update README.md and web docs to reflect that any two of --duration, --from, and --to can be combined in the edit command. Replace hardcoded hour 9 with defaultStartHour constant in log fallback placement. Co-Authored-By: Claude Opus 4.6 (1M context) --- README.md | 2 +- internal/cli/log.go | 4 ++-- web/docs/commands/time-tracking.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a3eff3c..7c5e725 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ hourgit edit [--duration ] [--from