Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support --resume-nth/-N flag for klog start #301

Merged
merged 1 commit into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
})
}
Loading