Skip to content

Commit a846f48

Browse files
jotaenjotaen
committed
Make auto-resolution aggregation-specific
1 parent 908de46 commit a846f48

File tree

2 files changed

+133
-33
lines changed

2 files changed

+133
-33
lines changed

‎klog/app/cli/report.go‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ If you want a consecutive, chronological stream, you can use the '--fill' flag.
3939
func (opt *Report) Run(ctx app.Context) app.Error {
4040
opt.DecimalArgs.Apply(&ctx)
4141
opt.NoStyleArgs.Apply(&ctx)
42-
cErr := opt.ApplyChart()
42+
cErr := opt.canonicaliseOpts()
4343
if cErr != nil {
4444
return cErr
4545
}
@@ -58,7 +58,7 @@ func (opt *Report) Run(ctx app.Context) app.Error {
5858
return nErr
5959
}
6060
records = service.Sort(records, true)
61-
aggregator := opt.findAggregator()
61+
aggregator := opt.aggregator()
6262
recordGroups, dates := groupByDate(aggregator.DateHash, records)
6363
if opt.Fill {
6464
dates = allDatesRange(records[0].Date(), records[len(records)-1].Date())
@@ -145,15 +145,41 @@ func (opt *Report) Run(ctx app.Context) app.Error {
145145
return nil
146146
}
147147

148-
func (opt *Report) findAggregator() report.Aggregator {
149-
category := (func() string {
150-
if opt.AggregateBy == "" {
151-
return "d"
152-
} else {
153-
return strings.ToLower(opt.AggregateBy[:1])
148+
func (opt *Report) canonicaliseOpts() app.Error {
149+
if opt.AggregateBy == "" {
150+
opt.AggregateBy = "d"
151+
} else {
152+
opt.AggregateBy = strings.ToLower(opt.AggregateBy[:1])
153+
}
154+
155+
if opt.ChartResolution == 0 {
156+
// If the resolution wasn’t explicitly specified, use a default value
157+
// that aims for a good balance between granularity and overall row width
158+
// in the context of the desired aggregation mode.
159+
switch opt.AggregateBy {
160+
case "y":
161+
opt.ChartResolution = 60 * 8 * 7 // Full working week
162+
case "q":
163+
opt.ChartResolution = 60 * 8 // Full working day
164+
case "m":
165+
opt.ChartResolution = 60 * 4 // Half working day
166+
case "w":
167+
opt.ChartResolution = 60
168+
default: // "d"
169+
opt.ChartResolution = 15
154170
}
155-
})()
156-
switch category {
171+
} else if opt.ChartResolution > 0 {
172+
// When chart resolution is specified, automatically assume --chart
173+
// to be given as well.
174+
opt.Chart = true
175+
} else if opt.ChartResolution < 0 {
176+
return app.NewErrorWithCode(app.LOGICAL_ERROR, "Invalid resolution", "The resolution must be a positive integer", nil)
177+
}
178+
return nil
179+
}
180+
181+
func (opt *Report) aggregator() report.Aggregator {
182+
switch opt.AggregateBy {
157183
case "y":
158184
return report.NewYearAggregator()
159185
case "q":
@@ -193,22 +219,6 @@ func groupByDate(hashProvider func(klog.Date) period.Hash, rs []klog.Record) (ma
193219
return days, order
194220
}
195221

196-
func (opt *Report) ApplyChart() app.Error {
197-
if opt.ChartResolution == 0 {
198-
// Unless specified otherwise, set the default resolution to 15 minutes
199-
// per rendered block. This should make for a good balance between granularity
200-
// and row width in the context of the (default) daily aggregation mode.
201-
opt.ChartResolution = 15
202-
} else if opt.ChartResolution > 0 {
203-
// When chart resolution is specified, automatically assume --chart
204-
// to be given as well.
205-
opt.Chart = true
206-
} else if opt.ChartResolution < 0 {
207-
return app.NewErrorWithCode(app.LOGICAL_ERROR, "Invalid scale factor", "The scale factor must be positive integer", nil)
208-
}
209-
return nil
210-
}
211-
212222
func renderBar(minutesPerUnit int, d klog.Duration) string {
213223
block := "▇"
214224
blocksCount := func() int {

‎klog/app/cli/report_test.go‎

Lines changed: 97 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,8 @@ func TestYearReport(t *testing.T) {
282282
}
283283

284284
func TestReportWithChart(t *testing.T) {
285-
state, err := NewTestingContext()._SetRecords(`
285+
t.Run("Daily (default) aggregation", func(t *testing.T) {
286+
state, err := NewTestingContext()._SetRecords(`
286287
2025-01-11
287288
-2h
288289
@@ -313,8 +314,8 @@ func TestReportWithChart(t *testing.T) {
313314
2025-01-25
314315
9h
315316
`)._Run((&Report{Chart: true}).Run)
316-
require.Nil(t, err)
317-
assert.Equal(t, `
317+
require.Nil(t, err)
318+
assert.Equal(t, `
318319
Total
319320
2025 Jan Sat 11. -2h
320321
Mon 13. 1m ▇
@@ -328,10 +329,99 @@ func TestReportWithChart(t *testing.T) {
328329
========
329330
39h38m
330331
`, state.printBuffer)
332+
})
333+
334+
t.Run("Weekly aggregation", func(t *testing.T) {
335+
state, err := NewTestingContext()._SetRecords(`
336+
2025-01-01
337+
40h
338+
339+
2025-01-08
340+
48h45m
341+
342+
2025-01-15
343+
31h15m
344+
`)._Run((&Report{Chart: true, AggregateBy: "w", WarnArgs: util.WarnArgs{NoWarn: true}}).Run)
345+
require.Nil(t, err)
346+
assert.Equal(t, `
347+
Total
348+
2025 Week 1 40h ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
349+
Week 2 48h45m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
350+
Week 3 31h15m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
351+
========
352+
120h
353+
`, state.printBuffer)
354+
})
355+
356+
t.Run("Monthly aggregation", func(t *testing.T) {
357+
state, err := NewTestingContext()._SetRecords(`
358+
2025-01-01
359+
173h
360+
361+
2025-02-01
362+
208h30m
363+
364+
2025-03-01
365+
126h15m
366+
`)._Run((&Report{Chart: true, AggregateBy: "m", WarnArgs: util.WarnArgs{NoWarn: true}}).Run)
367+
require.Nil(t, err)
368+
assert.Equal(t, `
369+
Total
370+
2025 Jan 173h ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
371+
Feb 208h30m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
372+
Mar 126h15m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
373+
========
374+
507h45m
375+
`, state.printBuffer)
376+
})
377+
378+
t.Run("Quarterly aggregation", func(t *testing.T) {
379+
state, err := NewTestingContext()._SetRecords(`
380+
2025-01-01
381+
316h
382+
383+
2025-04-01
384+
392h30m
385+
386+
2025-07-01
387+
237h45m
388+
`)._Run((&Report{Chart: true, AggregateBy: "q", WarnArgs: util.WarnArgs{NoWarn: true}}).Run)
389+
require.Nil(t, err)
390+
assert.Equal(t, `
391+
Total
392+
2025 Q1 316h ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
393+
Q2 392h30m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
394+
Q3 237h45m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
395+
========
396+
946h15m
397+
`, state.printBuffer)
398+
})
399+
400+
t.Run("Yearly aggregation", func(t *testing.T) {
401+
state, err := NewTestingContext()._SetRecords(`
402+
2025-01-01
403+
1735h
404+
405+
2026-01-01
406+
2154h45m
407+
408+
2027-01-01
409+
1189h15m
410+
`)._Run((&Report{Chart: true, AggregateBy: "y", WarnArgs: util.WarnArgs{NoWarn: true}}).Run)
411+
require.Nil(t, err)
412+
assert.Equal(t, `
413+
Total
414+
2025 1735h ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
415+
2026 2154h45m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
416+
2027 1189h15m ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇
417+
========
418+
5079h
419+
`, state.printBuffer)
420+
})
331421
}
332422

333423
func TestReportWithScaledChart(t *testing.T) {
334-
t.Run("Custom scale factor (large)", func(t *testing.T) {
424+
t.Run("Custom resolution (large)", func(t *testing.T) {
335425
state, err := NewTestingContext()._SetRecords(`
336426
2025-01-14
337427
12h
@@ -349,7 +439,7 @@ func TestReportWithScaledChart(t *testing.T) {
349439
`, state.printBuffer)
350440
})
351441

352-
t.Run("Custom scale factor (small)", func(t *testing.T) {
442+
t.Run("Custom resolution (small)", func(t *testing.T) {
353443
state, err := NewTestingContext()._SetRecords(`
354444
2025-01-14
355445
1h30m
@@ -403,7 +493,7 @@ func TestReportWithScaledChart(t *testing.T) {
403493
`, state.printBuffer)
404494
})
405495

406-
t.Run("Invalid scale factor", func(t *testing.T) {
496+
t.Run("Invalid resolution", func(t *testing.T) {
407497
_, err := NewTestingContext()._SetRecords(`
408498
2025-01-14
409499
12h
@@ -412,6 +502,6 @@ func TestReportWithScaledChart(t *testing.T) {
412502
18h37m
413503
`)._Run((&Report{ChartResolution: -10}).Run)
414504
require.Error(t, err)
415-
assert.Equal(t, "Invalid scale factor", err.Error())
505+
assert.Equal(t, "Invalid resolution", err.Error())
416506
})
417507
}

0 commit comments

Comments
 (0)