Merge pull request #635 from rsteube/action-cleanup

action: removed internal util functions

action.go

@@ -65,7 +65,11 @@ func (a Action) nestedAction(c Context, maxDepth int) Action {
 		return ActionMessage("maximum recursion depth exceeded")
 	}
 	if a.rawValues == nil && a.callback != nil {
-		result := a.callback(c).nestedAction(c, maxDepth-1).noSpace(string(a.meta.Nospace)).withUsage(a.meta.Usage)
+		result := a.callback(c).nestedAction(c, maxDepth-1)
+		if usage := a.meta.Usage; usage != "" {
+			result.meta.Usage = usage
+		}
+		result.meta.Nospace.Merge(a.meta.Nospace)
 		result.meta.Messages.Merge(a.meta.Messages)
 		return result
 	}
@@ -76,9 +80,10 @@ func (a Action) nestedAction(c Context, maxDepth int) Action {
 func (a Action) NoSpace(suffixes ...rune) Action {
 	return ActionCallback(func(c Context) Action {
 		if len(suffixes) == 0 {
-			return a.noSpace("*")
+			a.meta.Nospace.Add('*')
 		}
-		return a.noSpace(string(suffixes))
+		a.meta.Nospace.Add(suffixes...)
+		return a
 	})
 }
 
@@ -92,7 +97,10 @@ func (a Action) Usage(usage string, args ...interface{}) Action {
 // Usage sets the usage using a function.
 func (a Action) UsageF(f func() string) Action {
 	return ActionCallback(func(c Context) Action {
-		return a.withUsage(f())
+		if usage := f(); usage != "" {
+			a.meta.Usage = usage
+		}
+		return a
 	})
 }
 
@@ -205,15 +213,3 @@ func (a Action) UniqueList(divider string) Action {
 		return a.Invoke(c).Filter(c.Parts).ToA().NoSpace()
 	})
 }
-
-func (a Action) noSpace(suffixes string) Action {
-	a.meta.Nospace = a.meta.Nospace.Add(suffixes)
-	return a
-}
-
-func (a Action) withUsage(usage string) Action {
-	if usage != "" {
-		a.meta.Usage = usage
-	}
-	return a
-}

action_test.go

@@ -53,9 +53,6 @@ func TestActionCallback(t *testing.T) {
 	expected := InvokedAction{
 		Action{
 			rawValues: common.RawValuesFrom("a", "b", "c"),
-			meta: common.Meta{
-				Nospace: "",
-			},
 		},
 	}
 	actual := a.Invoke(Context{})
@@ -108,11 +105,11 @@ func TestNoSpace(t *testing.T) {
 			Suffix("").
 			ToA()
 	})
-	if a.meta.Nospace != "" {
-		t.Fatal("uninvoked nospace should be empty")
+	if a.meta.Nospace.Matches("x") {
+		t.Fatal("uninvoked nospace should not match")
 	}
-	if a.Invoke(Context{}).meta.Nospace != "*" {
-		t.Fatal("invoked nospace should be `*`")
+	if !a.Invoke(Context{}).meta.Nospace.Matches("x") {
+		t.Fatal("invoked nospace should match")
 	}
 }
 
@@ -124,7 +121,7 @@ func TestActionDirectories(t *testing.T) {
 			"internal/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"pkg/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"third_party/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
-		).noSpace("/").Invoke(Context{}),
+		).NoSpace('/').Invoke(Context{}),
 		ActionDirectories().Invoke(Context{CallbackValue: ""}).Filter([]string{"vendor/"}),
 	)
 
@@ -135,22 +132,22 @@ func TestActionDirectories(t *testing.T) {
 			"internal/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"pkg/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"third_party/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
-		).noSpace("/").Invoke(Context{}).Prefix("./"),
+		).NoSpace('/').Invoke(Context{}).Prefix("./"),
 		ActionDirectories().Invoke(Context{CallbackValue: "./"}).Filter([]string{"./vendor/"}),
 	)
 
 	assertEqual(t,
 		ActionStyledValues(
 			"_test/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"cmd/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
-		).noSpace("/").Invoke(Context{}).Prefix("example/"),
+		).NoSpace('/').Invoke(Context{}).Prefix("example/"),
 		ActionDirectories().Invoke(Context{CallbackValue: "example/"}),
 	)
 
 	assertEqual(t,
 		ActionStyledValues(
 			"cmd/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
-		).noSpace("/").Invoke(Context{}).Prefix("example/"),
+		).NoSpace('/').Invoke(Context{}).Prefix("example/"),
 		ActionDirectories().Invoke(Context{CallbackValue: "example/cm"}),
 	)
 }
@@ -164,7 +161,7 @@ func TestActionFiles(t *testing.T) {
 			"internal/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"pkg/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"third_party/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
-		).noSpace("/").Invoke(Context{}),
+		).NoSpace('/').Invoke(Context{}),
 		ActionFiles(".md").Invoke(Context{CallbackValue: ""}).Filter([]string{"vendor/"}),
 	)
 
@@ -175,7 +172,7 @@ func TestActionFiles(t *testing.T) {
 			"cmd/", style.Of("fg-default", "bg-default", style.Blue, style.Bold),
 			"main.go", style.Of("fg-default", "bg-default"),
 			"main_test.go", style.Of("fg-default", "bg-default"),
-		).noSpace("/").Invoke(Context{}).Prefix("example/"),
+		).NoSpace('/').Invoke(Context{}).Prefix("example/"),
 		ActionFiles().Invoke(Context{CallbackValue: "example/"}).Filter([]string{"example/example"}),
 	)
 }
@@ -197,7 +194,7 @@ func TestActionFilesChdir(t *testing.T) {
 		ActionStyledValues(
 			"action.go", style.Of("fg-default", "bg-default"),
 			"snippet.go", style.Of("fg-default", "bg-default"),
-		).noSpace("/").Invoke(Context{}).Prefix("elvish/"),
+		).NoSpace('/').Invoke(Context{}).Prefix("elvish/"),
 		ActionFiles().Chdir("internal/shell").Invoke(Context{CallbackValue: "elvish/"}),
 	)
 
@@ -222,13 +219,13 @@ func TestActionMessageSuppress(t *testing.T) {
 			ActionMessage("example message").Suppress("example"),
 			ActionValues("test"),
 		).ToA().Invoke(Context{}),
-		ActionValues("test").noSpace("*").Invoke(Context{}), // TODO suppress does not reset nospace (is that even possible?)
+		ActionValues("test").NoSpace('*').Invoke(Context{}), // TODO suppress does not reset nospace (is that even possible?)
 	)
 }
 
 func TestActionExecCommand(t *testing.T) {
 	assertEqual(t,
-		ActionMessage("go unknown: unknown command").noSpace("/").Invoke(Context{}).Prefix("docs/"),
+		ActionMessage("go unknown: unknown command").NoSpace('/').Invoke(Context{}).Prefix("docs/"),
 		ActionExecCommand("go", "unknown")(func(output []byte) Action { return ActionValues() }).Invoke(Context{CallbackValue: "docs/"}),
 	)
 

compat.go

@@ -40,8 +40,11 @@ func cobraValuesFor(action InvokedAction) []string {
 
 func cobraDirectiveFor(action InvokedAction) cobra.ShellCompDirective {
 	directive := cobra.ShellCompDirectiveNoFileComp
-	if action.meta.Nospace != "" {
-		directive = directive | cobra.ShellCompDirectiveNoSpace
+	for _, val := range action.rawValues {
+		if action.meta.Nospace.Matches(val.Value) {
+			directive = directive | cobra.ShellCompDirectiveNoSpace
+			break
+		}
 	}
 	return directive
 }

defaultActions.go

@@ -209,11 +209,11 @@ func ActionMultiParts(divider string, callback func(c Context) Action) Action {
 		}
 		c.Parts = parts
 
-		nospace := "*"
+		nospace := '*'
 		if runes := []rune(divider); len(runes) > 0 {
-			nospace = string(runes[len(runes)-1])
+			nospace = runes[len(runes)-1]
 		}
-		return callback(c).Invoke(c).Prefix(prefix).ToA().noSpace(nospace)
+		return callback(c).Invoke(c).Prefix(prefix).ToA().NoSpace(nospace)
 	})
 }
 
@@ -243,7 +243,7 @@ func ActionStyleConfig() Action {
 			})
 		case 1:
 			return ActionMultiParts(",", func(c Context) Action {
-				return ActionStyles(c.Parts...).Invoke(c).Filter(c.Parts).ToA()
+				return ActionStyles(c.Parts...).Invoke(c).Filter(c.Parts).ToA().NoSpace()
 			})
 		default:
 			return ActionValues()

internal/common/suffix.go

@@ -1,36 +1,57 @@
 package common
 
 import (
+	"encoding/json"
 	"sort"
 	"strings"
 )
 
-type SuffixMatcher string
+type SuffixMatcher struct {
+	string
+}
 
-func (sm SuffixMatcher) Add(s string) SuffixMatcher {
-	if strings.Contains(string(sm), "*") || strings.Contains(s, "*") {
-		return SuffixMatcher("*")
+func (sm *SuffixMatcher) Add(suffixes ...rune) {
+	if strings.Contains(sm.string, "*") || strings.Contains(string(suffixes), "*") {
+		sm.string = "*"
+		return
 	}
 
-	unique := []rune(sm)
-	for _, r := range []rune(s) {
-		if !strings.Contains(string(sm), string(r)) {
+	unique := []rune(sm.string)
+	for _, r := range suffixes {
+		if !strings.Contains(sm.string, string(r)) {
 			unique = append(unique, r)
 		}
 	}
 	sort.Sort(ByRune(unique))
-	return SuffixMatcher(unique)
+	sm.string = string(unique)
+}
+
+func (sm *SuffixMatcher) Merge(other SuffixMatcher) {
+	for _, r := range []rune(other.string) {
+		sm.Add(r)
+	}
 }
 
 func (sm SuffixMatcher) Matches(s string) bool {
-	for _, r := range []rune(sm) {
+	for _, r := range []rune(sm.string) {
 		if r == '*' || strings.HasSuffix(s, string(r)) {
 			return true
 		}
 	}
 	return false
 }
 
+func (sm SuffixMatcher) MarshalJSON() ([]byte, error) {
+	return json.Marshal(sm.string)
+}
+
+func (sm *SuffixMatcher) UnmarshalJSON(data []byte) (err error) {
+	if err = json.Unmarshal(data, &sm.string); err != nil {
+		return err
+	}
+	return
+}
+
 type ByRune []rune
 
 func (r ByRune) Len() int           { return len(r) }

internal/common/suffix_test.go

@@ -0,0 +1,12 @@
+package common
+
+import "testing"
+
+func TestSuffixMatcherAdd(t *testing.T) {
+	sm := SuffixMatcher{""}
+
+	sm.Add('*')
+	if sm.string != "*" {
+		t.Errorf(`should be "*" [was: "%v"]`, sm)
+	}
+}

internal/shell/bash/action.go

@@ -92,7 +92,7 @@ func ActionRawValues(currentWord string, meta common.Meta, values common.RawValu
 			// prevent insertion of partial display values by prefixing one with space
 			values[0].Display = " " + values[0].Display
 		}
-		meta.Nospace = meta.Nospace.Add("*")
+		meta.Nospace.Add('*')
 	}
 
 	vals := make([]string, len(values))

internalActions.go

@@ -113,7 +113,7 @@ func actionFlags(cmd *cobra.Command) Action {
 		})
 
 		if isShorthandSeries {
-			return ActionValuesDescribed(vals...).Invoke(c).Prefix(c.CallbackValue).ToA().noSpace("*")
+			return ActionValuesDescribed(vals...).Invoke(c).Prefix(c.CallbackValue).ToA().NoSpace('*')
 		}
 		for i := 0; i < len(vals); i = i + 2 { // TODO experimental - hardcoded multiparts completion if flags are "grouped" with `.`
 			if strings.Contains(vals[i], ".") {

invokedAction.go

@@ -37,7 +37,7 @@ func (a InvokedAction) Merge(others ...InvokedAction) InvokedAction {
 		for _, c := range other.rawValues {
 			uniqueRawValues[c.Value] = c
 		}
-		nospace = nospace.Add(string(other.meta.Nospace))
+		nospace.Merge(other.meta.Nospace)
 		if other.meta.Usage != "" {
 			usage = other.meta.Usage
 		}
@@ -49,7 +49,9 @@ func (a InvokedAction) Merge(others ...InvokedAction) InvokedAction {
 		rawValues = append(rawValues, c)
 	}
 
-	invoked := InvokedAction{actionRawValues(rawValues...).noSpace(string(nospace)).withUsage(usage)}
+	invoked := InvokedAction{actionRawValues(rawValues...)}
+	invoked.meta.Usage = usage
+	invoked.meta.Nospace.Merge(nospace)
 	invoked.meta.Messages = messages // TODO verify & optimize
 	return invoked
 }
@@ -142,16 +144,16 @@ func (a InvokedAction) ToMultiPartsA(dividers ...string) Action {
 			vals = append(vals, val)
 		}
 
-		var sm common.SuffixMatcher
+		a := actionRawValues(vals...)
 		for _, divider := range dividers {
 			if runes := []rune(divider); len(runes) == 0 {
-				sm = sm.Add("*")
+				a.meta.Nospace.Add('*')
 				break
 			} else {
-				sm = sm.Add(string(runes[len(runes)-1]))
+				a.meta.Nospace.Add(runes[len(runes)-1])
 			}
 		}
-		return actionRawValues(vals...).noSpace(string(sm))
+		return a
 	})
 }