Repository: rsteube/carapace
Pull request #234 (2021-02-17 15:36:16)

updated traverse function


Patch:
diff --git a/carapace.go b/carapace.go
index 15187df..f4f81b8 100644
--- a/carapace.go
+++ b/carapace.go
@@ -13,6 +13,7 @@ import (
 	"strings"
 
 	"github.com/rsteube/carapace/internal/bash"
+	"github.com/rsteube/carapace/internal/common"
 	"github.com/rsteube/carapace/internal/elvish"
 	"github.com/rsteube/carapace/internal/fish"
 	"github.com/rsteube/carapace/internal/oil"
@@ -144,10 +145,7 @@ func addCompletionCommand(cmd *cobra.Command) {
 					CallbackValue = current
 
 					targetCmd, targetArgs, err := findTarget(cmd)
-					if err != nil && // ignore specific errors regarding the flag currently being completed (bit fragile, but works)
-						err.Error() != fmt.Sprintf("unknown flag: %v", CallbackValue) && // unknown flag is not current one being completed (may be partial)
-						err.Error() != fmt.Sprintf("flag needs an argument: %v", CallbackValue) && // flag without argument is not current one being completed (matches a flag name)
-						(len(CallbackValue) < 2 || err.Error() != fmt.Sprintf("flag needs an argument: '%v' in %v", string(CallbackValue[len(CallbackValue)-1]), CallbackValue)) { // TODO support series of shorthand flags (skip the actual flag character and just check prefix/suffix or do regex match)
+					if err != nil {
 						fmt.Fprintln(io.MultiWriter(os.Stdout, logger.Writer()), ActionMessage(err.Error()).Value(shell))
 						return
 					}
@@ -222,26 +220,7 @@ func findTarget(cmd *cobra.Command) (*cobra.Command, []string, error) {
 	if len(os.Args) > 5 {
 		origArg = os.Args[5:]
 	}
-	return traverse(cmd, origArg)
-}
-
-func traverse(cmd *cobra.Command, args []string) (*cobra.Command, []string, error) {
-	// ignore flag parse errors (like a missing argument for the flag currently being completed)
-	a := args
-	if len(args) > 0 && args[len(args)-1] == "" {
-		a = args[0 : len(args)-1]
-	}
-
-	targetCmd, targetArgs, err := cmd.Root().Traverse(a)
-	if len(args) > 0 && args[len(args)-1] == "" {
-		targetArgs = append(targetArgs, "")
-	}
-	if err != nil {
-		return targetCmd, targetArgs, err
-	}
-
-	err = targetCmd.ParseFlags(targetArgs)
-	return targetCmd, targetCmd.Flags().Args(), err
+	return common.TraverseLenient(cmd, origArg)
 }
 
 func determineShell() string {
diff --git a/internal/common/traverse.go b/internal/common/traverse.go
new file mode 100644
index 0000000..d977def
--- /dev/null
+++ b/internal/common/traverse.go
@@ -0,0 +1,65 @@
+package common
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/spf13/cobra"
+)
+
+func TraverseLenient(cmd *cobra.Command, args []string) (*cobra.Command, []string, error) {
+	a := args
+
+	// needed so that completion for positional argument that has no value yet to work
+	if len(args) > 0 && args[len(args)-1] == "" {
+		a = args[0 : len(args)-1]
+	}
+
+	targetCmd, targetArgs, err := cmd.Root().Traverse(a)
+	if len(args) > 0 && args[len(args)-1] == "" {
+		targetArgs = append(targetArgs, "")
+	}
+	if err != nil {
+		return targetCmd, targetArgs, filterError(args, err)
+	}
+
+	err = targetCmd.ParseFlags(targetArgs)
+	return targetCmd, targetCmd.Flags().Args(), filterError(args, err)
+}
+
+func filterError(args []string, err error) error {
+	if err == nil || len(args) == 0 {
+		return err
+	}
+
+	msg := err.Error()
+	current := args[len(args)-1]
+	previous := args[len(args)-2]
+
+	if strings.HasPrefix(previous, "--") && msg == fmt.Sprintf("flag needs an argument: %v", previous) {
+		// ignore long flag argument currently being completed
+		return nil
+	}
+
+	if strings.HasPrefix(current, "--") && msg == fmt.Sprintf("flag needs an argument: %v", current) {
+		// ignore long flag argument currently being completed
+		return nil
+	}
+
+	if strings.HasPrefix(current, "--") && msg == fmt.Sprintf("unknown flag: %v", current) {
+		// ignore long flag curently being completed
+		return nil
+	}
+
+	if strings.HasPrefix(previous, "-") && msg == fmt.Sprintf("flag needs an argument: '%v' in -%v", previous[len(previous)-1:], previous[len(previous)-1:]) {
+		// ignore shorthand flag argument currently being completed
+		return nil
+	}
+
+	if strings.HasPrefix(current, "-") && msg == fmt.Sprintf("flag needs an argument: '%v' in -%v", current[len(current)-1:], current[len(current)-1:]) {
+		// ignore shorthand flag currently being completed
+		return nil
+	}
+
+	return err
+}


Test patch:
diff --git a/internal/common/traverse_test.go b/internal/common/traverse_test.go
new file mode 100644
index 0000000..b39ac29
--- /dev/null
+++ b/internal/common/traverse_test.go
@@ -0,0 +1,68 @@
+package common
+
+import (
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+var testCmd = &cobra.Command{
+	Use: "test",
+}
+
+var testSubCmd = &cobra.Command{
+	Use: "sub",
+}
+
+func init() {
+	testCmd.Flags().StringP("optarg", "o", "", "optarg flag")
+	testCmd.Flags().StringP("string", "s", "", "string flag")
+	testCmd.Flags().BoolP("bool", "b", false, "bool flag")
+
+	testSubCmd.Flags().StringP("suboptarg", "o", "", "optarg flag")
+	testSubCmd.Flags().StringP("substring", "s", "", "string flag")
+	testSubCmd.Flags().BoolP("subbool", "b", false, "bool flag")
+	testCmd.AddCommand(testSubCmd)
+}
+
+func testTraverseLenient(t *testing.T, args ...string) {
+	if _, _, err := TraverseLenient(testCmd, args); err != nil {
+		t.Error(err.Error())
+	}
+}
+
+func TestNoArg(t *testing.T) {
+	testTraverseLenient(t)
+}
+
+func TestEmptyArg(t *testing.T) {
+	testTraverseLenient(t, "")
+}
+
+func TestLongFlagIncomplete(t *testing.T) {
+	testTraverseLenient(t, "test", "--stri")
+}
+
+func TestLongFlagNoArg(t *testing.T) {
+	testTraverseLenient(t, "test", "--string")
+}
+
+func TestLongFlagEmptyArg(t *testing.T) {
+	testTraverseLenient(t, "test", "--string", "")
+}
+
+func TestShortFlagNoArg(t *testing.T) {
+	testTraverseLenient(t, "test", "-s")
+}
+
+func TestShortFlagEmptyArg(t *testing.T) {
+	testTraverseLenient(t, "test", "-s", "")
+}
+
+func TestShortFlagChainNoArg(t *testing.T) {
+	testTraverseLenient(t, "test", "-bs")
+}
+
+func TestShortFlagChainEmptyArg(t *testing.T) {
+	testTraverseLenient(t, "test", "-bs", "")
+}