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

jotaen · Mar 6, 2024 · 3a545e8 · 3a545e8
1 parent 96b3b05
commit 3a545e8
Show file tree

Hide file tree

Showing 3 changed files with 166 additions and 18 deletions.
diff --git a/klog/app/cli/index.go b/klog/app/cli/index.go
@@ -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'.
 `
 }
 

diff --git a/klog/app/cli/start.go b/klog/app/cli/start.go
@@ -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
@@ -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 {

diff --git a/klog/app/cli/start_test.go b/klog/app/cli/start_test.go
@@ -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
@@ -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)
+	})
 }