Skip to content

Commit

Permalink
Support --resume-nth/-N flag for klog start
Browse files Browse the repository at this point in the history
Resolves #282.

You can now specify which entry to resume – e.g.:

- `--resume-nth=1` takes over the summary of the first entry
- `--resume-nth=2` takes over the summary of the second entry
- `--resume-nth=-1` takes over the summary of the last entry
- `--resume-nth=-2` takes over the summary of the second-to-last entry

You can also use the shorthand `-N`. Remember to omit the delimiter
after the flag name then, so it would be `-N1`, `-N2`, `-N-1`, `-N-2`,
etc.
  • Loading branch information
jotaen committed Mar 6, 2024
1 parent 96b3b05 commit af66ac4
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 18 deletions.
5 changes: 4 additions & 1 deletion klog/app/cli/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ You can specify input data in one of these 3 ways:
Run 'klog bookmarks --help' to learn about bookmark usage.
One general note about flags: for all flags that have values, you can either use a space or an equals sign as delimiter, i.e. both '--flag value' and '--flag=value' are fine.
Some general notes on flag usage:
- For flags with values, you can either use a space or an equals sign as delimiter. E.g., both '--flag value' and '--flag=value' are fine.
- For shorthand flags with values, you specify the value without a delimiter. E.g., '-n2' (if the long form is '--number 2').
- For shorthand flags without values, you can compact them. E.g., '-abc' is the same as '-a -b -c'.
`
}

Expand Down
59 changes: 43 additions & 16 deletions klog/app/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (

type Start struct {
SummaryText klog.EntrySummary `name:"summary" short:"s" placeholder:"TEXT" help:"Summary text for this entry."`
Resume bool `name:"resume" short:"R" help:"Take over summary of last entry (if applicable)."`
Resume bool `name:"resume" short:"R" help:"Take over summary of last entry (if applicable). If the target record is new or empty, it looks at the previous record."`
ResumeNth int `name:"resume-nth" short:"N" help:"Take over summary of nth entry. If INT is positive, it counts from the start (beginning with '1'); if negative, it counts from the end (beginning with '-1')"`
util.AtDateAndTimeArgs
util.NoStyleArgs
util.WarnArgs
Expand Down Expand Up @@ -64,44 +65,70 @@ func (opt *Start) Run(ctx app.Context) app.Error {

func (opt *Start) Summary(currentRecord klog.Record, previousRecord klog.Record) (klog.EntrySummary, app.Error) {
// Check for conflicting flags.
if opt.SummaryText != nil && opt.Resume {
if opt.SummaryText != nil && (opt.Resume || opt.ResumeNth != 0) {
return nil, app.NewErrorWithCode(
app.LOGICAL_ERROR,
"Conflicting flags: --summary and --resume cannot be used at the same time",
"",
nil,
)
}
if opt.Resume && opt.ResumeNth != 0 {
return nil, app.NewError(
"Illegal flag combination",
"Cannot combine --resume and --resume-nth",
nil,
)
}

// Return summary flag, if specified.
if opt.SummaryText != nil {
return opt.SummaryText, nil
}

// Skip if resume flag wasn’t specified.
if !opt.Resume {
// If --resume was specified: return summary of last entry from current record, if
// it has any entries. Otherwise, return summary of last entry from previous record,
// if exists.
if opt.Resume {
if e, ok := findNthEntry(currentRecord, -1); ok {
return e.Summary(), nil
}
if previousRecord != nil {
if e, ok := findNthEntry(previousRecord, -1); ok {
return e.Summary(), nil
}
}
return nil, nil
}

// Return summary of last entry from current record, if it has any entries.
if len(currentRecord.Entries()) > 0 {
return lastEntrySummary(currentRecord), nil
}

// Return summary of last entry from previous record, if exists.
if previousRecord != nil {
return lastEntrySummary(previousRecord), nil
// If --resume-nth was specified: return summary of nth-entry. In contrast to --resume,
// don’t fall back to previous record, as that would be unintuitive here.
if opt.ResumeNth != 0 {
if e, ok := findNthEntry(currentRecord, opt.ResumeNth); ok {
return e.Summary(), nil
}
return nil, app.NewError(
"No such entry",
"",
nil,
)
}

return nil, nil
}

func lastEntrySummary(r klog.Record) klog.EntrySummary {
func findNthEntry(r klog.Record, nr int) (klog.Entry, bool) {
entriesCount := len(r.Entries())
if entriesCount == 0 {
return nil
i := func() int {
if nr > 0 {
return nr - 1
}
return entriesCount + nr
}()
if i < 0 || i > entriesCount-1 {
return klog.Entry{}, false
}
return r.Entries()[entriesCount-1].Summary()
return r.Entries()[i], true
}

type PreviousRecordSpy struct {
Expand Down
120 changes: 119 additions & 1 deletion klog/app/cli/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ func TestStartWithResume(t *testing.T) {
`, state.writtenFileContents)
})

t.Run("Resuming fails if summary tag is specified as well", func(t *testing.T) {
t.Run("Fails if --summary flag is specified as well", func(t *testing.T) {
_, err := NewTestingContext()._SetRecords(`1623-12-13
8:13 - 9:44
Work
Expand All @@ -376,4 +376,122 @@ func TestStartWithResume(t *testing.T) {
}).Run)
require.Error(t, err)
})

t.Run("Fails if --resume-nth flag is specified as well", func(t *testing.T) {
_, err := NewTestingContext()._SetRecords(`1623-12-13
8:13 - 9:44
Work
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
Resume: true,
ResumeNth: 1,
}).Run)
require.Error(t, err)
})
}

func TestStartWithResumeNth(t *testing.T) {
t.Run("Takes over first entry", func(t *testing.T) {
for _, nth := range []int{1, -3} {
state, err := NewTestingContext()._SetRecords(`1623-12-13
1h Foo
2h Bar
3h Asdf
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
ResumeNth: nth,
}).Run)
require.Nil(t, err)
assert.Equal(t, `1623-12-13
1h Foo
2h Bar
3h Asdf
12:49 - ? Foo
`, state.writtenFileContents)
}
})

t.Run("Takes over second entry", func(t *testing.T) {
for _, nth := range []int{2, -2} {
state, err := NewTestingContext()._SetRecords(`1623-12-13
1h Foo
2h Bar
3h Asdf
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
ResumeNth: nth,
}).Run)
require.Nil(t, err)
assert.Equal(t, `1623-12-13
1h Foo
2h Bar
3h Asdf
12:49 - ? Bar
`, state.writtenFileContents)
}
})

t.Run("Takes over empty entry", func(t *testing.T) {
for _, nth := range []int{3, -1} {
state, err := NewTestingContext()._SetRecords(`1623-12-13
1h Foo
2h Bar
3h
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
ResumeNth: nth,
}).Run)
require.Nil(t, err)
assert.Equal(t, `1623-12-13
1h Foo
2h Bar
3h
12:49 - ?
`, state.writtenFileContents)
}
})

t.Run("Fails if out of bounds", func(t *testing.T) {
for _, nth := range []int{4, 5, 10, -4, -5, -10} {
_, err := NewTestingContext()._SetRecords(`1623-12-13
1h Foo
2h Bar
3h Asdf
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
ResumeNth: nth,
}).Run)
require.Error(t, err)
}
})

t.Run("Doesn’t fall back to previous record", func(t *testing.T) {
_, err := NewTestingContext()._SetRecords(`1623-12-12
1h Foo
2h Bar
3h Asdf
1623-12-13
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
ResumeNth: -1,
}).Run)
require.Error(t, err)
})

t.Run("Fails if --summary flag is specified as well", func(t *testing.T) {
_, err := NewTestingContext()._SetRecords(`1623-12-13
8:13 - 9:44
Work
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
ResumeNth: 1,
SummaryText: klog.Ɀ_EntrySummary_("Test"),
}).Run)
require.Error(t, err)
})

t.Run("Fails if --resume flag is specified as well", func(t *testing.T) {
_, err := NewTestingContext()._SetRecords(`1623-12-13
8:13 - 9:44
Work
`)._SetNow(1623, 12, 13, 12, 49)._Run((&Start{
Resume: true,
ResumeNth: 1,
}).Run)
require.Error(t, err)
})
}

0 comments on commit af66ac4

Please sign in to comment.