Skip to content

feat(core): add support for multi positional args #979

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

Merged
merged 9 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
21 changes: 3 additions & 18 deletions internal/args/args.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package args

import (
"fmt"
"reflect"
"regexp"
"strings"
Expand Down Expand Up @@ -45,15 +44,15 @@ var (
// ["arg1=1", "arg2=2", "arg3"] => {"arg1": "1", "arg2": "2", "arg3":"" }
func SplitRawMap(rawArgs []string) map[string]struct{} {
argsMap := map[string]struct{}{}
for _, arg := range SplitRawNoError(rawArgs) {
for _, arg := range SplitRaw(rawArgs) {
argsMap[arg[0]] = struct{}{}
}
return argsMap
}

// SplitRawNoError creates a slice that maps arg names to their values.
// SplitRaw creates a slice that maps arg names to their values.
// ["arg1=1", "arg2=2", "arg3"] => { {"arg1", "1"}, {"arg2", "2"}, {"arg3",""} }
func SplitRawNoError(rawArgs []string) [][2]string {
func SplitRaw(rawArgs []string) [][2]string {
keyValue := [][2]string{}
for _, arg := range rawArgs {
tmp := strings.SplitN(arg, "=", 2)
Expand All @@ -65,20 +64,6 @@ func SplitRawNoError(rawArgs []string) [][2]string {
return keyValue
}

// SplitRaw creates a slice that maps arg names to their values.
// ["arg1=1", "arg2=2", "arg3"] => { {"arg1", "1"}, {"arg2", "2"}, {"arg3",""} }
func SplitRaw(rawArgs []string) ([][2]string, error) {
keyValue := [][2]string{}
for _, arg := range rawArgs {
tmp := strings.SplitN(arg, "=", 2)
if len(tmp) < 2 || (len(tmp) == 2 && strings.HasSuffix(arg, "=")) {
return nil, fmt.Errorf("arg '%v' must have a value", tmp[0])
}
keyValue = append(keyValue, [2]string{tmp[0], tmp[1]})
}
return keyValue, nil
}

func getInterfaceFromReflectValue(reflectValue reflect.Value) interface{} {
i := reflectValue.Interface()
if reflectValue.CanAddr() {
Expand Down
7 changes: 2 additions & 5 deletions internal/args/unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ func UnmarshalStruct(args []string, data interface{}) error {

// Map arg names to their values.
// ["arg1=1", "arg2=2", "arg3"] => [ ["arg1","1"], ["arg2","2"], ["arg3",""] ]
argsSlice, err := SplitRaw(args)
if err != nil {
return err
}
argsSlice := SplitRaw(args)

processedArgNames := make(map[string]bool)

Expand Down Expand Up @@ -344,7 +341,7 @@ func unmarshalScalar(value string, dest reflect.Value) error {
return err
case reflect.Bool:
switch value {
case "", "true":
case "true":
dest.SetBool(true)
case "false":
dest.SetBool(false)
Expand Down
17 changes: 7 additions & 10 deletions internal/args/unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,49 +436,46 @@ func TestUnmarshalStruct(t *testing.T) {
"bool",
},
data: &Basic{},
error: "arg 'bool' must have a value",
error: "cannot unmarshal arg 'bool': *bool is not unmarshalable: invalid boolean value",
}))

t.Run("bool-without-value", run(TestCase{
args: []string{
"bool=",
},
data: &Basic{},
error: "arg 'bool' must have a value",
error: "cannot unmarshal arg 'bool': *bool is not unmarshalable: invalid boolean value",
}))

t.Run("string-without-equal", run(TestCase{
args: []string{
"string",
},
data: &Basic{},
error: "arg 'string' must have a value",
expected: &Basic{},
//error: "arg 'string' must have a value",
}))

t.Run("string-without-value", run(TestCase{
args: []string{
"string=",
},
data: &Basic{},
error: "arg 'string' must have a value",
expected: &Basic{},
}))

t.Run("strings-without-equal", run(TestCase{
args: []string{
"strings",
},
data: &Slice{},
error: "arg 'strings' must have a value",
error: "cannot unmarshal arg 'strings': missing index on the array",
}))

// TODO: decide if we want to fix it
// According to specs, this should not trigger an error
t.Run("strings-without-value", run(TestCase{
args: []string{
"strings=",
},
data: &Slice{},
error: "arg 'strings' must have a value",
error: "cannot unmarshal arg 'strings': missing index on the array",
}))
}
func TestIsUmarshalableValue(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions internal/core/arg_specs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package core

import (
"fmt"
"strings"

"github.com/scaleway/scaleway-sdk-go/scw"
)
Expand Down Expand Up @@ -84,6 +85,10 @@ func (a *ArgSpec) Prefix() string {
return a.Name + "="
}

func (a *ArgSpec) IsPartOfMapOrSlice() bool {
return strings.Contains(a.Name, sliceSchema) || strings.Contains(a.Name, mapSchema)
}

type DefaultFunc func() (value string, doc string)

func ZoneArgSpec(zones ...scw.Zone) *ArgSpec {
Expand Down
80 changes: 80 additions & 0 deletions internal/core/args.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package core

import "strings"

type RawArgs []string

func (args RawArgs) GetPositionalArgs() []string {

positionalArgs := []string(nil)
for _, arg := range args {
if isPositionalArg(arg) {
positionalArgs = append(positionalArgs, arg)
}
}
return positionalArgs
}

func (args RawArgs) Get(argName string) (string, bool) {
for _, arg := range args {
name, value := splitArg(arg)
if name == argName {
return value, true
}
}
return "", false
}

func (args RawArgs) RemoveAllPositional() RawArgs {
return args.filter(func(arg string) bool {
return !isPositionalArg(arg)
})
}

func (args RawArgs) Add(name string, value string) RawArgs {
return append(args, name+"="+value)
}

func (args RawArgs) Remove(argName string) RawArgs {
return args.filter(func(arg string) bool {
name, _ := splitArg(arg)
return name != argName
})
}

func (args RawArgs) filter(test func(string) bool) RawArgs {
argsCopy := RawArgs{}
for _, arg := range args {
if test(arg) {
argsCopy = append(argsCopy, arg)
}
}
return argsCopy
}

func (args RawArgs) GetSliceOrMapKeys(prefix string) []string {
keys := []string(nil)
for _, arg := range args {
name, _ := splitArg(arg)
if !strings.HasPrefix(name, prefix+".") {
continue
}

name = strings.TrimPrefix(name, prefix+".")
keys = append(keys, strings.SplitN(name, ".", 2)[0])
}
return keys
}

func splitArg(arg string) (name string, value string) {
part := strings.SplitN(arg, "=", 2)
if len(part) == 1 {
return "", part[0]
}
return part[0], part[1]
}

func isPositionalArg(arg string) bool {
pos := strings.IndexRune(arg, '=')
return pos == -1
}
94 changes: 23 additions & 71 deletions internal/core/autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ type AutoCompleteNodeType uint

const (
AutoCompleteNodeTypeCommand AutoCompleteNodeType = iota
AutoCompleteNodeTypePositionalArgument
AutoCompleteNodeTypeArgument
AutoCompleteNodeTypeFlag
AutoCompleteNodeTypeFlagValueConst
Expand Down Expand Up @@ -103,16 +102,6 @@ func NewAutoCompleteCommandNode() *AutoCompleteNode {
}
}

// NewAutoCompletePositionalArgNode creates a new node corresponding to a command positional argument.
// These nodes are not necessarily leaf nodes.
func NewAutoCompletePositionalArgNode(argSpec *ArgSpec) *AutoCompleteNode {
return &AutoCompleteNode{
Children: make(map[string]*AutoCompleteNode),
ArgSpec: argSpec,
Type: AutoCompleteNodeTypePositionalArgument,
}
}

// NewArgAutoCompleteNode creates a new node corresponding to a command argument.
// These nodes are leaf nodes.
func NewAutoCompleteArgNode(argSpec *ArgSpec) *AutoCompleteNode {
Expand Down Expand Up @@ -169,6 +158,11 @@ func (node *AutoCompleteNode) GetChildOrCreate(name string) *AutoCompleteNode {
// - plural argument name + alphanumeric: arguments.key1=
func (node *AutoCompleteNode) GetChildMatch(name string) (*AutoCompleteNode, bool) {
for key, child := range node.Children {
if key == positionalValueNodeID {
continue
}
key = "^" + key + "$"
key = strings.ReplaceAll(key, ".", "\\.")
key = strings.ReplaceAll(key, sliceSchema, "[0-9]+")
key = strings.ReplaceAll(key, mapSchema, "[0-9a-zA-Z-]+")
r := regexp.MustCompile(key)
Expand All @@ -182,11 +176,11 @@ func (node *AutoCompleteNode) GetChildMatch(name string) (*AutoCompleteNode, boo
// isLeafCommand returns true only if n is a node with no child command (namespace, verb, resource) or a positional arg.
// A leaf command can have 2 types of children: arguments or flags
func (node *AutoCompleteNode) isLeafCommand() bool {
if node.Type != AutoCompleteNodeTypeCommand && node.Type != AutoCompleteNodeTypePositionalArgument {
if node.Type != AutoCompleteNodeTypeCommand {
return false
}
for _, child := range node.Children {
if child.Type == AutoCompleteNodeTypeCommand || child.Type == AutoCompleteNodeTypePositionalArgument {
if child.Type == AutoCompleteNodeTypeCommand {
return false
}
}
Expand All @@ -210,17 +204,10 @@ func BuildAutoCompleteTree(commands *Commands) *AutoCompleteNode {

node.Command = cmd

// Create node for positional argument if the command has one.
positionalArg := cmd.ArgSpecs.GetPositionalArg()
if positionalArg != nil {
node.Children[positionalValueNodeID] = NewAutoCompletePositionalArgNode(positionalArg)
node = node.Children[positionalValueNodeID]
node.addGlobalFlags()
}

// We consider ArgSpecs as leaf in the autocomplete tree.
for _, argSpec := range cmd.ArgSpecs {
if argSpec == positionalArg {
if argSpec.Positional {
node.Children[positionalValueNodeID] = NewAutoCompleteArgNode(argSpec)
continue
}
node.Children[argSpec.Name+"="] = NewAutoCompleteArgNode(argSpec)
Expand Down Expand Up @@ -278,10 +265,6 @@ func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string
// Do nothing
// Arguments do not have children: they are not used to go deeper into the tree

case children.Type == AutoCompleteNodeTypePositionalArgument && isCompletingArgValue(word):
// Do nothing
// Setting a positional argument with `key=value` notation is not allowed.

default:
// word is a namespace or verb or resource or flag or flag value
node = children
Expand Down Expand Up @@ -310,23 +293,12 @@ func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string

// handle boolean arg
default:
children, exist := node.Children[word+"="]
if exist && children.Type == AutoCompleteNodeTypeArgument && i > nodeIndexInWords {
// We need to check i > nodeIndexInWords
// because the same word may be used for both a command and an argument.
// example:
// scw instance ip delete ip<tab>
// Here, we don't want to register the first `ip` into `completedArgs`
completedArgs[word+"="] = struct{}{}
if _, exist := node.Children[positionalValueNodeID]; exist && i > nodeIndexInWords {
completedArgs[word] = struct{}{}
}
}
}

if isCompletingPositionalArgValue(node, wordToComplete) {
suggestions := AutoCompleteArgValue(ctx, node.Children[positionalValueNodeID].ArgSpec, wordToComplete)
return newAutoCompleteResponse(suggestions)
}

if isCompletingArgValue(wordToComplete) {
argName, argValuePrefix := splitArgWord(wordToComplete)
argNode, exist := node.GetChildMatch(argName)
Expand All @@ -344,10 +316,19 @@ func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string
return newAutoCompleteResponse(suggestions)
}

// We are trying to complete a node: either a command name or an arg name or a flagname

// We are trying to complete a node: either a command name or an arg name or a flagname or a positional args
suggestions := []string(nil)
for key := range node.Children {
for key, child := range node.Children {

if key == positionalValueNodeID {
for _, positionalSuggestion := range AutoCompleteArgValue(ctx, child.ArgSpec, wordToComplete) {
if _, exists := completedArgs[positionalSuggestion]; !exists {
suggestions = append(suggestions, positionalSuggestion)
}
}
continue
}

if !hasPrefix(key, wordToComplete) {
continue
}
Expand Down Expand Up @@ -398,35 +379,6 @@ func AutoCompleteArgValue(ctx context.Context, argSpec *ArgSpec, argValuePrefix
return suggestions
}

// isCompletingPositionalArgValue detects if the word to complete is a positional argument on a given node.
// Returns false on the following cases:
// - node has no positional argument
// - a flag is being completed
func isCompletingPositionalArgValue(node *AutoCompleteNode, wordToComplete string) bool {
// Return false if node has no value node children
valueNode, exist := node.Children[positionalValueNodeID]
if !exist {
return false
}

// return false if this value node children is not of type positional arg
if valueNode.Type != AutoCompleteNodeTypePositionalArgument {
return false
}

// Catch when a flag is being completed.
for child := range node.Children {
if child == positionalValueNodeID || wordToComplete == "" {
continue
}
if strings.HasPrefix(child, wordToComplete) {
return false
}
}

return true
}

func isCompletingArgValue(wordToComplete string) bool {
wordParts := strings.SplitN(wordToComplete, "=", 2)
return len(wordParts) == 2
Expand Down
Loading