Repository: rsteube/carapace
Pull request #277 (2021-04-10 09:06:11)

Action: embed skipcache


Patch:
diff --git a/action.go b/action.go
index 717c57e..8996114 100644
--- a/action.go
+++ b/action.go
@@ -31,6 +31,7 @@ type Action struct {
 	rawValues []common.RawValue
 	callback  CompletionCallback
 	nospace   bool
+	skipcache bool
 }
 
 type ActionMap map[string]Action
@@ -73,13 +74,10 @@ func (a Action) Cache(timeout time.Duration, keys ...pkgcache.CacheKey) Action {
 				if rawValues, err := cache.Load(cacheFile, timeout); err == nil {
 					return actionRawValues(rawValues...)
 				} else {
-					oldState := skipCache
-					skipCache = false // TODO find a better solution for this
 					invokedAction := (Action{callback: cachedCallback}).Invoke(c)
-					if !skipCache {
+					if !invokedAction.skipcache {
 						_ = cache.Write(cacheFile, invokedAction.rawValues)
 					}
-					skipCache = oldState || skipCache
 					return invokedAction.ToA()
 				}
 			}
@@ -105,18 +103,20 @@ func (a Action) Invoke(c Context) InvokedAction {
 func (a InvokedAction) Merge(others ...InvokedAction) InvokedAction {
 	uniqueRawValues := make(map[string]common.RawValue)
 	nospace := a.nospace
+	skipcache := a.skipcache
 	for _, other := range append([]InvokedAction{a}, others...) {
 		for _, c := range other.rawValues {
 			uniqueRawValues[c.Value] = c
 		}
 		nospace = a.nospace || other.nospace
+		skipcache = a.skipcache || other.skipcache
 	}
 
 	rawValues := make([]common.RawValue, 0, len(uniqueRawValues))
 	for _, c := range uniqueRawValues {
 		rawValues = append(rawValues, c)
 	}
-	return InvokedAction(actionRawValues(rawValues...).noSpace(nospace))
+	return InvokedAction(actionRawValues(rawValues...).noSpace(nospace).skipCache(skipcache))
 }
 
 func (a Action) noSpace(state bool) Action {
@@ -124,6 +124,11 @@ func (a Action) noSpace(state bool) Action {
 	return a
 }
 
+func (a Action) skipCache(state bool) Action {
+	a.skipcache = a.skipcache || state
+	return a
+}
+
 func (a InvokedAction) Filter(values []string) InvokedAction {
 	toremove := make(map[string]bool)
 	for _, v := range values {
@@ -135,7 +140,7 @@ func (a InvokedAction) Filter(values []string) InvokedAction {
 			filtered = append(filtered, rawValue)
 		}
 	}
-	return InvokedAction(actionRawValues(filtered...).noSpace(a.nospace))
+	return InvokedAction(actionRawValues(filtered...).noSpace(a.nospace).skipCache(a.skipcache))
 }
 
 func (a InvokedAction) Prefix(prefix string) InvokedAction {
@@ -182,7 +187,7 @@ func (a InvokedAction) ToMultiPartsA(divider string) Action {
 
 func (a Action) nestedAction(c Context, maxDepth int) Action {
 	if a.rawValues == nil && a.callback != nil && maxDepth > 0 {
-		return a.callback(c).nestedAction(c, maxDepth-1).noSpace(a.nospace)
+		return a.callback(c).nestedAction(c, maxDepth-1).noSpace(a.nospace).skipCache(a.skipcache)
 	} else {
 		return a
 	}
@@ -219,7 +224,7 @@ func ActionCallback(callback CompletionCallback) Action {
 }
 
 // ActionBool completes true/false
-func ActionBool() Action {
+func actionBool() Action {
 	return ActionValues("true", "false")
 }
 
@@ -322,14 +327,12 @@ func actionRawValues(rawValues ...common.RawValue) Action {
 	}
 }
 
-// TODO move this into Action and update Merge/Suffix/Prefix functions to keep the state if true
-var skipCache bool
-
 // ActionMessage displays a help messages in places where no completions can be generated
 func ActionMessage(msg string) Action {
 	return ActionCallback(func(c Context) Action {
-		skipCache = true                                                                                        // TODO find a better solution - any call to ActionMessage i assumed to be an error for now
-		return ActionValuesDescribed("_", "", "ERR", msg).Invoke(c).Prefix(c.CallbackValue).ToA().noSpace(true) // needs to be prefixed with current callback value to not be filtered out
+		return ActionValuesDescribed("_", "", "ERR", msg).
+			Invoke(c).Prefix(c.CallbackValue).ToA(). // needs to be prefixed with current callback value to not be filtered out
+			noSpace(true).skipCache(true)
 	})
 }
 
diff --git a/carapace.go b/carapace.go
index 0d7bbf5..328826f 100644
--- a/carapace.go
+++ b/carapace.go
@@ -284,7 +284,7 @@ func initLogger() (err error) {
 	return
 }
 
-type testing interface {
+type testingT interface {
 	Error(args ...interface{})
 }
 
@@ -292,8 +292,8 @@ type testing interface {
 //   func TestCarapace(t *testing.T) {
 //       carapace.Test(t)
 //   }
-func Test(t testing) {
+func Test(t testingT) {
 	for _, e := range storage.check() {
-		testing(t).Error(e)
+		testingT(t).Error(e)
 	}
 }
diff --git a/example/cmd/root.go b/example/cmd/root.go
index 5ac306b..815d3bb 100644
--- a/example/cmd/root.go
+++ b/example/cmd/root.go
@@ -22,7 +22,6 @@ func init() {
 	rootCmd.Flags().StringArrayP("array", "a", []string{}, "multiflag")
 
 	carapace.Gen(rootCmd).FlagCompletion(carapace.ActionMap{
-		"toggle":         carapace.ActionBool(),
 		"persistentFlag": carapace.ActionValues("p1", "p2", "p3"),
 	})
 }


Test patch:
diff --git a/action_test.go b/action_test.go
new file mode 100644
index 0000000..013b5f7
--- /dev/null
+++ b/action_test.go
@@ -0,0 +1,68 @@
+package carapace
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/rsteube/carapace/internal/assert"
+	"github.com/rsteube/carapace/internal/common"
+)
+
+func assertEqual(t *testing.T, expected, actual InvokedAction) {
+	assert.Equal(t, fmt.Sprintf("%+v\n", expected), fmt.Sprintf("%+v\n", actual))
+}
+
+func TestActionCallback(t *testing.T) {
+	a := ActionCallback(func(c Context) Action {
+		return ActionCallback(func(c Context) Action {
+			return ActionCallback(func(c Context) Action {
+				return ActionValues("a", "b", "c")
+			})
+		})
+	})
+	expected := InvokedAction{
+		rawValues: common.RawValuesFrom("a", "b", "c"),
+		nospace:   false,
+		skipcache: false,
+	}
+	actual := a.Invoke(Context{})
+	assertEqual(t, expected, actual)
+}
+
+func TestSkipCache(t *testing.T) {
+	a := ActionCallback(func(c Context) Action {
+		return ActionValues().Invoke(c).Merge(
+			ActionCallback(func(c Context) Action {
+				return ActionMessage("skipcache")
+			}).Invoke(c)).
+			Filter([]string{""}).
+			Prefix("").
+			Suffix("").
+			ToA()
+	})
+	if a.skipcache {
+		t.Fatal("uninvoked skipcache should be false")
+	}
+	if !a.Invoke(Context{}).skipcache {
+		t.Fatal("invoked skipcache should be true")
+	}
+}
+
+func TestNoSpace(t *testing.T) {
+	a := ActionCallback(func(c Context) Action {
+		return ActionValues().Invoke(c).Merge(
+			ActionMultiParts("", func(c Context) Action {
+				return ActionMessage("nospace")
+			}).Invoke(c)).
+			Filter([]string{""}).
+			Prefix("").
+			Suffix("").
+			ToA()
+	})
+	if a.nospace {
+		t.Fatal("uninvoked nospace should be false")
+	}
+	if !a.Invoke(Context{}).nospace {
+		t.Fatal("invoked nospace should be true")
+	}
+}