Merge pull request #181 from fission-suite/feat/add-strings-option

feat:add strings option; re-implement file ignore

.gitignore

@@ -6,3 +6,4 @@ cover.html
 examples/adder/local/local
 examples/adder/remote/client/client
 examples/adder/remote/server/server
+coverage.out

cli/helptext.go

@@ -301,7 +301,12 @@ func generateSynopsis(width int, cmd *cmds.Command, path string) string {
 				}
 			}
 		}
-		appendText("[" + sopt + "]")
+
+		if opt.Type() == cmds.Strings {
+			appendText("[" + sopt + "]...")
+		} else {
+			appendText("[" + sopt + "]")
+		}
 	}
 	if len(cmd.Arguments) > 0 {
 		appendText("[--]")

cli/helptext_test.go

@@ -15,6 +15,7 @@ func TestSynopsisGenerator(t *testing.T) {
 		},
 		Options: []cmds.Option{
 			cmds.StringOption("opt", "o", "Option"),
+			cmds.StringsOption("var-opt", "Variadic Option"),
 		},
 		Helptext: cmds.HelpText{
 			SynopsisOptionsValues: map[string]string{
@@ -31,6 +32,9 @@ func TestSynopsisGenerator(t *testing.T) {
 	if !strings.Contains(syn, "[--opt=<OPTION> | -o]") {
 		t.Fatal("Synopsis should contain option descriptor")
 	}
+	if !strings.Contains(syn, "[--var-opt=<var-opt>]...") {
+		t.Fatal("Synopsis should contain option descriptor")
+	}
 	if !strings.Contains(syn, "<required>") {
 		t.Fatal("Synopsis should contain required argument")
 	}

cli/parse.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"reflect"
 	"strings"
 
 	osh "github.com/Kubuxu/go-os-helper"
@@ -67,6 +68,16 @@ func isRecursive(req *cmds.Request) bool {
 	return rec && ok
 }
 
+func getIgnoreRulesFile(req *cmds.Request) string {
+	rulesFile, _ := req.Options[cmds.IgnoreRules].(string)
+	return rulesFile
+}
+
+func getIgnoreRules(req *cmds.Request) []string {
+	rules, _ := req.Options[cmds.Ignore].([]string)
+	return rules
+}
+
 func stdinName(req *cmds.Request) string {
 	name, _ := req.Options[cmds.StdinName].(string)
 	return name
@@ -85,6 +96,19 @@ func (st *parseState) peek() string {
 	return st.cmdline[st.i]
 }
 
+func setOpts(kv kv, kvType reflect.Kind, opts cmds.OptMap) error {
+
+	if kvType == cmds.Strings {
+		res, _ := opts[kv.Key].([]string)
+		opts[kv.Key] = append(res, kv.Value.(string))
+	} else if _, exists := opts[kv.Key]; !exists {
+		opts[kv.Key] = kv.Value
+	} else {
+		return fmt.Errorf("multiple values for option %q", kv.Key)
+	}
+	return nil
+}
+
 func parse(req *cmds.Request, cmdline []string, root *cmds.Command) (err error) {
 	var (
 		path = make([]string, 0, len(cmdline))
@@ -116,13 +140,13 @@ L:
 			if err != nil {
 				return err
 			}
-
-			if _, exists := opts[k]; exists {
-				return fmt.Errorf("multiple values for option %q", k)
+			kvType, err := getOptType(k, optDefs)
+			if err != nil {
+				return err // shouldn't happen b/c k,v was parsed from optsDef
+			}
+			if err := setOpts(kv{Key: k, Value: v}, kvType, opts); err != nil {
+				return err
 			}
-
-			k = optDefs[k].Name()
-			opts[k] = v
 
 		case strings.HasPrefix(param, "-") && param != "-":
 			// short options
@@ -134,11 +158,13 @@ L:
 			for _, kv := range kvs {
 				kv.Key = optDefs[kv.Key].Names()[0]
 
-				if _, exists := opts[kv.Key]; exists {
-					return fmt.Errorf("multiple values for option %q", kv.Key)
+				kvType, err := getOptType(kv.Key, optDefs)
+				if err != nil {
+					return err // shouldn't happen b/c kvs was parsed from optsDef
+				}
+				if err := setOpts(kv, kvType, opts); err != nil {
+					return err
 				}
-
-				opts[kv.Key] = kv.Value
 			}
 		default:
 			arg := param
@@ -294,8 +320,13 @@ func parseArgs(req *cmds.Request, root *cmds.Command, stdin *os.File) error {
 							return err
 						}
 					}
-
-					nf, err := appendFile(fpath, argDef, isRecursive(req), isHidden(req))
+					rulesFile := getIgnoreRulesFile(req)
+					ignoreRules := getIgnoreRules(req)
+					filter, err := files.NewFilter(rulesFile, ignoreRules, isHidden(req))
+					if err != nil {
+						return err
+					}
+					nf, err := appendFile(fpath, argDef, isRecursive(req), filter)
 					if err != nil {
 						return err
 					}
@@ -497,7 +528,7 @@ func getArgDef(i int, argDefs []cmds.Argument) *cmds.Argument {
 const notRecursiveFmtStr = "'%s' is a directory, use the '-%s' flag to specify directories"
 const dirNotSupportedFmtStr = "invalid path '%s', argument '%s' does not support directories"
 
-func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (files.Node, error) {
+func appendFile(fpath string, argDef *cmds.Argument, recursive bool, filter *files.Filter) (files.Node, error) {
 	stat, err := os.Lstat(fpath)
 	if err != nil {
 		return nil, err
@@ -521,8 +552,7 @@ func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (fi
 
 		return files.NewReaderFile(file), nil
 	}
-
-	return files.NewSerialFile(fpath, hidden, stat)
+	return files.NewSerialFileWithFilter(fpath, filter, stat)
 }
 
 // Inform the user if a file is waiting on input
@@ -574,3 +604,10 @@ func (r *messageReader) Read(b []byte) (int, error) {
 func (r *messageReader) Close() error {
 	return r.r.Close()
 }
+
+func getOptType(k string, optDefs map[string]cmds.Option) (reflect.Kind, error) {
+	if opt, ok := optDefs[k]; ok {
+		return opt.Type(), nil
+	}
+	return reflect.Invalid, fmt.Errorf("unknown option %q", k)
+}

cli/parse_test.go

@@ -3,10 +3,12 @@ package cli
 import (
 	"context"
 	"fmt"
+	"github.com/ipfs/go-ipfs-files"
 	"io"
 	"io/ioutil"
 	"net/url"
 	"os"
+	"path"
 	"strings"
 	"testing"
 
@@ -33,7 +35,14 @@ func sameKVs(a kvs, b kvs) bool {
 		return false
 	}
 	for k, v := range a {
-		if v != b[k] {
+		if ks, ok := v.([]string); ok {
+			bks, _ := b[k].([]string)
+			for i := 0; i < len(ks); i++ {
+				if ks[i] != bks[i] {
+					return false
+				}
+			}
+		} else if v != b[k] {
 			return false
 		}
 	}
@@ -72,6 +81,7 @@ func TestOptionParsing(t *testing.T) {
 		Options: []cmds.Option{
 			cmds.StringOption("string", "s", "a string"),
 			cmds.BoolOption("bool", "b", "a bool"),
+			cmds.StringsOption("strings", "r", "strings array"),
 		},
 		Subcommands: map[string]*cmds.Command{
 			"test": &cmds.Command{},
@@ -141,6 +151,7 @@ func TestOptionParsing(t *testing.T) {
 	test("-b test false", kvs{"bool": true}, words{"false"})
 	test("-b --string foo test bar", kvs{"bool": true, "string": "foo"}, words{"bar"})
 	test("-b=false --string bar", kvs{"bool": false, "string": "bar"}, words{})
+	test("--strings a --strings b", kvs{"strings": []string{"a", "b"}}, words{})
 	testFail("foo test")
 	test("defaults", kvs{"opt": "def"}, words{})
 	test("defaults -o foo", kvs{"opt": "foo"}, words{})
@@ -552,3 +563,149 @@ func Test_urlBase(t *testing.T) {
 		}
 	}
 }
+
+func TestFileArgs(t *testing.T) {
+	rootCmd := &cmds.Command{
+		Subcommands: map[string]*cmds.Command{
+			"fileOp": {
+				Arguments: []cmds.Argument{
+					cmds.FileArg("path", true, true, "The path to the file to be operated upon.").EnableRecursive().EnableStdin(),
+				},
+				Options: []cmds.Option{
+					cmds.OptionRecursivePath, // a builtin option that allows recursive paths (-r, --recursive)
+					cmds.OptionHidden,
+					cmds.OptionIgnoreRules,
+					cmds.OptionIgnore,
+				},
+			},
+		},
+	}
+	mkTempFile := func(t *testing.T, dir, pattern, content string) *os.File {
+		pat := "test_tmpFile_"
+		if pattern != "" {
+			pat = pattern
+		}
+		tmpFile, err := ioutil.TempFile(dir, pat)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if _, err := io.WriteString(tmpFile, content); err != nil {
+			t.Fatal(err)
+		}
+		return tmpFile
+	}
+	tmpDir1, err := ioutil.TempDir("", "parsetest_fileargs_tmpdir_")
+	if err != nil {
+		t.Fatal(err)
+	}
+	tmpDir2, err := ioutil.TempDir("", "parsetest_utildir_")
+	if err != nil {
+		t.Fatal(err)
+	}
+	tmpFile1 := mkTempFile(t, "", "", "test1")
+	tmpFile2 := mkTempFile(t, tmpDir1, "", "toBeIgnored")
+	tmpFile3 := mkTempFile(t, tmpDir1, "", "test3")
+	ignoreFile := mkTempFile(t, tmpDir2, "", path.Base(tmpFile2.Name()))
+	tmpHiddenFile := mkTempFile(t, tmpDir1, ".test_hidden_file_*", "test")
+	defer func() {
+		for _, f := range []string{
+			tmpDir1,
+			tmpFile1.Name(),
+			tmpFile2.Name(),
+			tmpHiddenFile.Name(),
+			tmpFile3.Name(),
+			ignoreFile.Name(),
+			tmpDir2,
+		} {
+			os.Remove(f)
+		}
+	}()
+	var testCases = []struct {
+		cmd      words
+		f        *os.File
+		args     words
+		parseErr error
+	}{
+		{
+			cmd:      words{"fileOp"},
+			args:     nil,
+			parseErr: fmt.Errorf("argument %q is required", "path"),
+		},
+		{
+			cmd: words{"fileOp", "--ignore", path.Base(tmpFile2.Name()), tmpDir1, tmpFile1.Name()}, f: nil,
+			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
+			parseErr: fmt.Errorf(notRecursiveFmtStr, tmpDir1, "r"),
+		}, {
+			cmd: words{"fileOp", tmpFile1.Name(), "--ignore", path.Base(tmpFile2.Name()), "--ignore"}, f: nil,
+			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
+			parseErr: fmt.Errorf("missing argument for option %q", "ignore"),
+		},
+		{
+			cmd: words{"fileOp", "-r", "--ignore", path.Base(tmpFile2.Name()), tmpDir1, tmpFile1.Name()}, f: nil,
+			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
+			parseErr: nil,
+		},
+		{
+			cmd: words{"fileOp", "--hidden", "-r", "--ignore", path.Base(tmpFile2.Name()), tmpDir1, tmpFile1.Name()}, f: nil,
+			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name(), tmpHiddenFile.Name()},
+			parseErr: nil,
+		},
+		{
+			cmd: words{"fileOp", "-r", "--ignore", path.Base(tmpFile2.Name()), tmpDir1, tmpFile1.Name(), "--ignore", "anotherRule"}, f: nil,
+			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
+			parseErr: nil,
+		},
+		{
+			cmd: words{"fileOp", "-r", "--ignore-rules-path", ignoreFile.Name(), tmpDir1, tmpFile1.Name()}, f: nil,
+			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
+			parseErr: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		req, err := Parse(context.Background(), tc.cmd, tc.f, rootCmd)
+		if err == nil {
+			err = req.Command.CheckArguments(req)
+		}
+		if !errEq(err, tc.parseErr) {
+			t.Fatalf("parsing request for cmd %q: expected error %q, got %q", tc.cmd, tc.parseErr, err)
+		}
+		if err != nil {
+			continue
+		}
+
+		if len(tc.args) == 0 {
+			continue
+		}
+		expectedFileMap := make(map[string]bool)
+		for _, arg := range tc.args {
+			expectedFileMap[path.Base(arg)] = false
+		}
+		it := req.Files.Entries()
+		for it.Next() {
+			name := it.Name()
+			if _, ok := expectedFileMap[name]; ok {
+				expectedFileMap[name] = true
+			} else {
+				t.Errorf("found unexpected file %q in request %v", name, req)
+			}
+			file := it.Node()
+			files.Walk(file, func(fpath string, nd files.Node) error {
+				if fpath != "" {
+					if _, ok := expectedFileMap[fpath]; ok {
+						expectedFileMap[fpath] = true
+					} else {
+						t.Errorf("found unexpected file %q in request file arguments", fpath)
+					}
+				}
+				return nil
+			})
+		}
+		for p, found := range expectedFileMap {
+			if !found {
+				t.Errorf("failed to find expected path %q in req %v", p, req)
+			}
+		}
+	}
+}

command_test.go

@@ -17,6 +17,7 @@ func TestOptionValidation(t *testing.T) {
 		Options: []Option{
 			IntOption("b", "beep", "enables beeper"),
 			StringOption("B", "boop", "password for booper"),
+			StringsOption("S", "shoop", "what to shoop"),
 		},
 		Run: noop,
 	}
@@ -57,6 +58,10 @@ func TestOptionValidation(t *testing.T) {
 		{opts: map[string]interface{}{"foo": 5}},
 		{opts: map[string]interface{}{EncLong: "json"}},
 		{opts: map[string]interface{}{"beep": "100"}},
+		{opts: map[string]interface{}{"S": [2]string{"a", "b"}}},
+		{
+			opts:            map[string]interface{}{"S": true},
+			NewRequestError: `Option "S" should be type "array", but got type "bool"`},
 		{
 			opts:            map[string]interface{}{"beep": ":)"},
 			NewRequestError: `Could not convert value ":)" to type "int" (for option "-beep")`,

go.mod

@@ -4,9 +4,9 @@ go 1.14
 
 require (
 	github.com/Kubuxu/go-os-helper v0.0.1
-	github.com/ipfs/go-ipfs-files v0.0.6
+	github.com/ipfs/go-ipfs-files v0.0.7
 	github.com/ipfs/go-log v0.0.1
 	github.com/rs/cors v1.7.0
 	github.com/texttheater/golang-levenshtein v0.0.0-20180516184445-d188e65d659e
-	golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f
+	golang.org/x/crypto v0.0.0-20200115085410-6d4e4cb37c7d
 )

option.go

@@ -17,6 +17,7 @@ const (
 	Uint64  = reflect.Uint64
 	Float   = reflect.Float64
 	String  = reflect.String
+	Strings = reflect.Array
 )
 
 type OptMap map[string]interface{}
@@ -117,6 +118,9 @@ var converters = map[reflect.Kind]converter{
 	String: func(v string) (interface{}, error) {
 		return v, nil
 	},
+	Strings: func(v string) (interface{}, error) {
+		return v, nil
+	},
 }
 
 func (o *option) Parse(v string) (interface{}, error) {
@@ -180,3 +184,6 @@ func FloatOption(names ...string) Option {
 func StringOption(names ...string) Option {
 	return NewOption(String, names...)
 }
+func StringsOption(names ...string) Option {
+	return NewOption(Strings, names...)
+}

opts.go

@@ -16,6 +16,8 @@ const (
 	StdinName    = "stdin-name"
 	Hidden       = "hidden"
 	HiddenShort  = "H"
+	Ignore       = "ignore"
+	IgnoreRules  = "ignore-rules-path"
 )
 
 // options that are used by this package
@@ -26,3 +28,5 @@ var OptionTimeout = StringOption(TimeoutOpt, "Set a global timeout on the comman
 var OptionDerefArgs = BoolOption(DerefLong, "Symlinks supplied in arguments are dereferenced")
 var OptionStdinName = StringOption(StdinName, "Assign a name if the file source is stdin.")
 var OptionHidden = BoolOption(Hidden, HiddenShort, "Include files that are hidden. Only takes effect on recursive add.")
+var OptionIgnore = StringsOption(Ignore, "A rule (.gitignore-stype) defining which file(s) should be ignored (variadic, experimental)")
+var OptionIgnoreRules = StringOption(IgnoreRules, "A path to a file with .gitgnore-style ignore rules (experimental)")