@cryptotaxi247 / kubo / commits / f16853903

Improve command line parsing

Etienne Laurin committed Apr 25, 2015 at 09:02 UTC f168539030ac37f6b4131101a5a019790fd752d5
10 files changed +230 -120
cmd/ipfs/main.go
+14 -7
@@ -93,6 +93,12 @@ func main() {
93 fmt.Fprintf(w, "Use 'ipfs %s --help' for information about this command\n", cmdPath)
94 }
95
96 + // Handle `ipfs help'
97 + if len(os.Args) == 2 && os.Args[1] == "help" {
98 + printHelp(false, os.Stdout)
99 + os.Exit(0)
100 + }
101 +
102 // parse the commandline into a command invocation
103 parseErr := invoc.Parse(ctx, os.Args[1:])
104
@@ -110,13 +116,6 @@ func main() {
116 }
117 }
118
113 - // here we handle the cases where
114 - // - commands with no Run func are invoked directly.
115 - // - the main command is invoked.
116 - if invoc.cmd == nil || invoc.cmd.Run == nil {
117 - printHelp(false, os.Stdout)
118 - os.Exit(0)
119 - }
119
120 // ok now handle parse error (which means cli input was wrong,
121 // e.g. incorrect number of args, or nonexistent subcommand)
@@ -132,6 +131,14 @@ func main() {
131 os.Exit(1)
132 }
133
134 + // here we handle the cases where
135 + // - commands with no Run func are invoked directly.
136 + // - the main command is invoked.
137 + if invoc.cmd == nil || invoc.cmd.Run == nil {
138 + printHelp(false, os.Stdout)
139 + os.Exit(0)
140 + }
141 +
142 // ok, finally, run the command invocation.
143 intrh, ctx := invoc.SetupInterruptHandler(ctx)
144 defer intrh.Close()
commands/cli/helptext.go
+11 -2
@@ -14,7 +14,8 @@ const (
14 requiredArg = "<%v>"
15 optionalArg = "[<%v>]"
16 variadicArg = "%v..."
17 - optionFlag = "-%v"
17 + shortFlag = "-%v"
18 + longFlag = "--%v"
19 optionType = "(%v)"
20
21 whitespace = "\r\n\t "
@@ -219,6 +220,14 @@ func argumentText(cmd *cmds.Command) []string {
220 return lines
221 }
222
223 +func optionFlag(flag string) string {
224 + if len(flag) == 1 {
225 + return fmt.Sprintf(shortFlag, flag)
226 + } else {
227 + return fmt.Sprintf(longFlag, flag)
228 + }
229 +}
230 +
231 func optionText(cmd ...*cmds.Command) []string {
232 // get a slice of the options we want to list out
233 options := make([]cmds.Option, 0)
@@ -241,7 +250,7 @@ func optionText(cmd ...*cmds.Command) []string {
250
251 names := sortByLength(opt.Names())
252 if len(names) >= j+1 {
244 - lines[i] += fmt.Sprintf(optionFlag, names[j])
253 + lines[i] += optionFlag(names[j])
254 }
255 if len(names) > j+1 {
256 lines[i] += ", "
commands/cli/parse.go
+124 -66
@@ -2,7 +2,6 @@ package cli
2
3 import (
4 "bytes"
5 - "errors"
5 "fmt"
6 "os"
7 "runtime"
@@ -13,20 +12,12 @@ import (
12 u "github.com/ipfs/go-ipfs/util"
13 )
14
16 -// ErrInvalidSubcmd signals when the parse error is not found
17 -var ErrInvalidSubcmd = errors.New("subcommand not found")
18 -
15 // Parse parses the input commandline string (cmd, flags, and args).
16 // returns the corresponding command Request object.
17 func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *cmds.Command, []string, error) {
22 - path, input, cmd := parsePath(input, root)
23 - if len(path) == 0 {
24 - return nil, nil, path, ErrInvalidSubcmd
25 - }
26 -
27 - opts, stringVals, err := parseOptions(input)
18 + path, opts, stringVals, cmd, err := parseOpts(input, root)
19 if err != nil {
29 - return nil, cmd, path, err
20 + return nil, nil, path, err
21 }
22
23 optDefs, err := root.GetOptions(path)
@@ -34,14 +25,6 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c
25 return nil, cmd, path, err
26 }
27
37 - // check to make sure there aren't any undefined options
38 - for k := range opts {
39 - if _, found := optDefs[k]; !found {
40 - err = fmt.Errorf("Unrecognized option: -%s", k)
41 - return nil, cmd, path, err
42 - }
43 - }
44 -
28 req, err := cmds.NewRequest(path, opts, nil, nil, cmd, optDefs)
29 if err != nil {
30 return nil, cmd, path, err
@@ -75,67 +58,142 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c
58 return req, cmd, path, nil
59 }
60
78 -// parsePath separates the command path and the opts and args from a command string
79 -// returns command path slice, rest slice, and the corresponding *cmd.Command
80 -func parsePath(input []string, root *cmds.Command) ([]string, []string, *cmds.Command) {
81 - cmd := root
82 - path := make([]string, 0, len(input))
83 - input2 := make([]string, 0, len(input))
61 +// Parse a command line made up of sub-commands, short arguments, long arguments and positional arguments
62 +func parseOpts(args []string, root *cmds.Command) (
63 + path []string,
64 + opts map[string]interface{},
65 + stringVals []string,
66 + cmd *cmds.Command,
67 + err error,
68 +) {
69 + path = make([]string, 0, len(args))
70 + stringVals = make([]string, 0, len(args))
71 + optDefs := map[string]cmds.Option{}
72 + opts = map[string]interface{}{}
73 + cmd = root
74 +
75 + // parseFlag checks that a flag is valid and saves it into opts
76 + // Returns true if the optional second argument is used
77 + parseFlag := func(name string, arg *string, mustUse bool) (bool, error) {
78 + if _, ok := opts[name]; ok {
79 + return false, fmt.Errorf("Duplicate values for option '%s'", name)
80 + }
81
85 - for i, blob := range input {
86 - if strings.HasPrefix(blob, "-") {
87 - input2 = append(input2, blob)
88 - continue
82 + optDef, found := optDefs[name]
83 + if !found {
84 + err = fmt.Errorf("Unrecognized option '%s'", name)
85 + return false, err
86 }
87
91 - sub := cmd.Subcommand(blob)
92 - if sub == nil {
93 - input2 = append(input2, input[i:]...)
94 - break
88 + if optDef.Type() == cmds.Bool {
89 + if mustUse {
90 + return false, fmt.Errorf("Option '%s' takes no arguments, but was passed '%s'", name, *arg)
91 + }
92 + opts[name] = ""
93 + return false, nil
94 + } else {
95 + if arg == nil {
96 + return true, fmt.Errorf("Missing argument for option '%s'", name)
97 + }
98 + opts[name] = *arg
99 + return true, nil
100 }
96 - cmd = sub
97 - path = append(path, blob)
101 }
102
100 - return path, input2, cmd
101 -}
102 -
103 -// parseOptions parses the raw string values of the given options
104 -// returns the parsed options as strings, along with the CLI args
105 -func parseOptions(input []string) (map[string]interface{}, []string, error) {
106 - opts := make(map[string]interface{})
107 - args := []string{}
108 -
109 - for i := 0; i < len(input); i++ {
110 - blob := input[i]
103 + optDefs, err = root.GetOptions(path)
104 + if err != nil {
105 + return
106 + }
107
112 - if strings.HasPrefix(blob, "-") {
113 - name := blob[1:]
114 - value := ""
108 + consumed := false
109 + for i, arg := range args {
110 + switch {
111 + case consumed:
112 + // arg was already consumed by the preceding flag
113 + consumed = false
114 + continue
115
116 - // support single and double dash
117 - if strings.HasPrefix(name, "-") {
118 - name = name[1:]
116 + case arg == "--":
117 + // treat all remaining arguments as positional arguments
118 + stringVals = append(stringVals, args[i+1:]...)
119 + return
120 +
121 + case strings.HasPrefix(arg, "--"):
122 + // arg is a long flag, with an optional argument specified
123 + // using `=' or in args[i+1]
124 + var slurped bool
125 + var next *string
126 + split := strings.SplitN(arg, "=", 2)
127 + if len(split) == 2 {
128 + slurped = false
129 + arg = split[0]
130 + next = &split[1]
131 + } else {
132 + slurped = true
133 + if i+1 < len(args) {
134 + next = &args[i+1]
135 + } else {
136 + next = nil
137 + }
138 }
120 -
121 - if strings.Contains(name, "=") {
122 - split := strings.SplitN(name, "=", 2)
123 - name = split[0]
124 - value = split[1]
139 + consumed, err = parseFlag(arg[2:], next, len(split) == 2)
140 + if err != nil {
141 + return
142 }
126 -
127 - if _, ok := opts[name]; ok {
128 - return nil, nil, fmt.Errorf("Duplicate values for option '%s'", name)
143 + if !slurped {
144 + consumed = false
145 }
146
131 - opts[name] = value
147 + case strings.HasPrefix(arg, "-") && arg != "-":
148 + // args is one or more flags in short form, followed by an optional argument
149 + // all flags except the last one have type bool
150 + for arg = arg[1:]; len(arg) != 0; arg = arg[1:] {
151 + var rest *string
152 + var slurped bool
153 + mustUse := false
154 + if len(arg) > 1 {
155 + slurped = false
156 + str := arg[1:]
157 + if len(str) > 0 && str[0] == '=' {
158 + str = str[1:]
159 + mustUse = true
160 + }
161 + rest = &str
162 + } else {
163 + slurped = true
164 + if i+1 < len(args) {
165 + rest = &args[i+1]
166 + } else {
167 + rest = nil
168 + }
169 + }
170 + var end bool
171 + end, err = parseFlag(arg[0:1], rest, mustUse)
172 + if err != nil {
173 + return
174 + }
175 + if end {
176 + consumed = slurped
177 + break
178 + }
179 + }
180
133 - } else {
134 - args = append(args, blob)
181 + default:
182 + // arg is a sub-command or a positional argument
183 + sub := cmd.Subcommand(arg)
184 + if sub != nil {
185 + cmd = sub
186 + path = append(path, arg)
187 + optDefs, err = root.GetOptions(path)
188 + if err != nil {
189 + return
190 + }
191 + } else {
192 + stringVals = append(stringVals, arg)
193 + }
194 }
195 }
137 -
138 - return opts, args, nil
196 + return
197 }
198
199 func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursive bool) ([]string, []files.File, error) {
@@ -171,7 +229,7 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi
229 // and the last arg definition is not variadic (or there are no definitions), return an error
230 notVariadic := len(argDefs) == 0 || !argDefs[len(argDefs)-1].Variadic
231 if notVariadic && numInputs > len(argDefs) {
174 - return nil, nil, fmt.Errorf("Expected %v arguments, got %v", len(argDefs), numInputs)
232 + return nil, nil, fmt.Errorf("Expected %v arguments, got %v: %v", len(argDefs), numInputs, inputs)
233 }
234
235 stringArgs := make([]string, 0, numInputs)
commands/cli/parse_test.go
+61 -25
@@ -1,7 +1,7 @@
1 package cli
2
3 import (
4 - //"fmt"
4 + "strings"
5 "testing"
6
7 "github.com/ipfs/go-ipfs/commands"
@@ -11,43 +11,79 @@ func TestOptionParsing(t *testing.T) {
11 subCmd := &commands.Command{}
12 cmd := &commands.Command{
13 Options: []commands.Option{
14 - commands.StringOption("b", "some option"),
14 + commands.StringOption("string", "s", "a string"),
15 + commands.BoolOption("bool", "b", "a bool"),
16 },
17 Subcommands: map[string]*commands.Command{
18 "test": subCmd,
19 },
20 }
21
21 - opts, input, err := parseOptions([]string{"--beep", "-boop=lol", "test2", "-c", "beep", "--foo=5"})
22 - /*for k, v := range opts {
23 - fmt.Printf("%s: %s\n", k, v)
24 - }
25 - fmt.Printf("%s\n", input)*/
26 - if err != nil {
27 - t.Error("Should have passed")
28 - }
29 - if len(opts) != 4 || opts["beep"] != "" || opts["boop"] != "lol" || opts["c"] != "" || opts["foo"] != "5" {
30 - t.Errorf("Returned options were defferent than expected: %v", opts)
31 - }
32 - if len(input) != 2 || input[0] != "test2" || input[1] != "beep" {
33 - t.Errorf("Returned input was different than expected: %v", input)
22 + type kvs map[string]interface{}
23 + type words []string
24 +
25 + sameWords := func(a words, b words) bool {
26 + for i, w := range a {
27 + if w != b[i] {
28 + return false
29 + }
30 + }
31 + return true
32 }
33
36 - _, _, err = parseOptions([]string{"-beep=1", "-boop=2", "-beep=3"})
37 - if err == nil {
38 - t.Error("Should have failed (duplicate option name)")
34 + sameKVs := func(a kvs, b kvs) bool {
35 + if len(a) != len(b) {
36 + return false
37 + }
38 + for k, v := range a {
39 + if v != b[k] {
40 + return false
41 + }
42 + }
43 + return true
44 }
45
41 - path, args, sub := parsePath([]string{"test", "beep", "boop"}, cmd)
42 - if len(path) != 1 || path[0] != "test" {
43 - t.Errorf("Returned path was defferent than expected: %v", path)
46 + testHelper := func(args string, expectedOpts kvs, expectedWords words, expectErr bool) {
47 + _, opts, input, _, err := parseOpts(strings.Split(args, " "), cmd)
48 + if expectErr {
49 + if err == nil {
50 + t.Errorf("Command line '%v' parsing should have failed", args)
51 + }
52 + } else if err != nil {
53 + t.Errorf("Command line '%v' failed to parse: %v", args, err)
54 + } else if !sameWords(input, expectedWords) || !sameKVs(opts, expectedOpts) {
55 + t.Errorf("Command line '%v':\n parsed as %v %v\n instead of %v %v",
56 + args, opts, input, expectedOpts, expectedWords)
57 + }
58 }
45 - if len(args) != 2 || args[0] != "beep" || args[1] != "boop" {
46 - t.Errorf("Returned args were different than expected: %v", args)
59 +
60 + testFail := func(args string) {
61 + testHelper(args, kvs{}, words{}, true)
62 }
48 - if sub != subCmd {
49 - t.Errorf("Returned command was different than expected")
63 +
64 + test := func(args string, expectedOpts kvs, expectedWords words) {
65 + testHelper(args, expectedOpts, expectedWords, false)
66 }
67 +
68 + test("-", kvs{}, words{"-"})
69 + testFail("-b -b")
70 + test("beep boop", kvs{}, words{"beep", "boop"})
71 + test("test beep boop", kvs{}, words{"beep", "boop"})
72 + testFail("-s")
73 + test("-s foo", kvs{"s": "foo"}, words{})
74 + test("-sfoo", kvs{"s": "foo"}, words{})
75 + test("-s=foo", kvs{"s": "foo"}, words{})
76 + test("-b", kvs{"b": ""}, words{})
77 + test("-bs foo", kvs{"b": "", "s": "foo"}, words{})
78 + test("-sb", kvs{"s": "b"}, words{})
79 + test("-b foo", kvs{"b": ""}, words{"foo"})
80 + test("--bool foo", kvs{"bool": ""}, words{"foo"})
81 + testFail("--bool=foo")
82 + testFail("--string")
83 + test("--string foo", kvs{"string": "foo"}, words{})
84 + test("--string=foo", kvs{"string": "foo"}, words{})
85 + test("-- -b", kvs{}, words{"-b"})
86 + test("foo -b", kvs{"b": ""}, words{"foo"})
87 }
88
89 func TestArgumentParsing(t *testing.T) {
repo/fsrepo/fsrepo.go
+2 -2
@@ -40,7 +40,7 @@ Please run the ipfs migration tool before continuing.
40 ` + migrationInstructions
41
42 var (
43 - ErrNoRepo = errors.New("no ipfs repo found. please run: ipfs init")
43 + ErrNoRepo = func (path string) error { return fmt.Errorf("no ipfs repo found in '%s'. please run: ipfs init ", path) }
44 ErrNoVersion = errors.New("no version file found, please run 0-to-1 migration tool.\n" + migrationInstructions)
45 ErrOldRepo = errors.New("ipfs repo found in old '~/.go-ipfs' location, please run migration tool.\n" + migrationInstructions)
46 )
@@ -172,7 +172,7 @@ func checkInitialized(path string) error {
172 if isInitializedUnsynced(alt) {
173 return ErrOldRepo
174 }
175 - return ErrNoRepo
175 + return ErrNoRepo(path)
176 }
177 return nil
178 }
test/sharness/lib/test-lib.sh
+2 -2
@@ -105,7 +105,7 @@ test_wait_open_tcp_port_10_sec() {
105 # was setting really weird things and am not sure why.
106 test_config_set() {
107
108 - # grab flags (like -bool in "ipfs config -bool")
108 + # grab flags (like --bool in "ipfs config --bool")
109 test_cfg_flags="" # unset in case.
110 test "$#" = 3 && { test_cfg_flags=$1; shift; }
111
@@ -184,7 +184,7 @@ test_config_ipfs_gateway_writable() {
184 test_config_ipfs_gateway_readonly $1
185
186 test_expect_success "prepare config -- gateway writable" '
187 - test_config_set -bool Gateway.Writable true ||
187 + test_config_set --bool Gateway.Writable true ||
188 test_fsh cat "\"$IPFS_PATH/config\""
189 '
190 }
test/sharness/t0021-config.sh
+3 -3
@@ -7,7 +7,7 @@ test_description="Test config command"
7 # we use a function so that we can run it both offline + online
8 test_config_cmd_set() {
9
10 - # flags (like -bool in "ipfs config -bool")
10 + # flags (like --bool in "ipfs config --bool")
11 cfg_flags="" # unset in case.
12 test "$#" = 3 && { cfg_flags=$1; shift; }
13
@@ -41,8 +41,8 @@ test_config_cmd() {
41 test_config_cmd_set "beep" "boop"
42 test_config_cmd_set "beep1" "boop2"
43 test_config_cmd_set "beep1" "boop2"
44 - test_config_cmd_set "-bool" "beep2" "true"
45 - test_config_cmd_set "-bool" "beep2" "false"
44 + test_config_cmd_set "--bool" "beep2" "true"
45 + test_config_cmd_set "--bool" "beep2" "false"
46
47 }
48
test/sharness/t0080-repo.sh
+10 -10
@@ -17,7 +17,7 @@ test_expect_success "'ipfs add afile' succeeds" '
17 '
18
19 test_expect_success "added file was pinned" '
20 - ipfs pin ls -type=recursive >actual &&
20 + ipfs pin ls --type=recursive >actual &&
21 grep "$HASH" actual
22 '
23
@@ -49,7 +49,7 @@ test_expect_success "file no longer pinned" '
49 echo "$HASH_WELCOME_DOCS" >expected2 &&
50 ipfs refs -r "$HASH_WELCOME_DOCS" >>expected2 &&
51 echo QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn >> expected2 &&
52 - ipfs pin ls -type=recursive >actual2 &&
52 + ipfs pin ls --type=recursive >actual2 &&
53 test_sort_cmp expected2 actual2
54 '
55
@@ -102,10 +102,10 @@ test_expect_success "adding multiblock random file succeeds" '
102 MBLOCKHASH=`ipfs add -q multiblock`
103 '
104
105 -test_expect_success "'ipfs pin ls -type=indirect' is correct" '
105 +test_expect_success "'ipfs pin ls --type=indirect' is correct" '
106 ipfs refs "$MBLOCKHASH" >refsout &&
107 ipfs refs -r "$HASH_WELCOME_DOCS" >>refsout &&
108 - ipfs pin ls -type=indirect >indirectpins &&
108 + ipfs pin ls --type=indirect >indirectpins &&
109 test_sort_cmp refsout indirectpins
110 '
111
@@ -121,27 +121,27 @@ test_expect_success "pin something directly" '
121 test_cmp expected10 actual10
122 '
123
124 -test_expect_success "'ipfs pin ls -type=direct' is correct" '
124 +test_expect_success "'ipfs pin ls --type=direct' is correct" '
125 echo "$DIRECTPIN" >directpinexpected &&
126 - ipfs pin ls -type=direct >directpinout &&
126 + ipfs pin ls --type=direct >directpinout &&
127 test_sort_cmp directpinexpected directpinout
128 '
129
130 -test_expect_success "'ipfs pin ls -type=recursive' is correct" '
130 +test_expect_success "'ipfs pin ls --type=recursive' is correct" '
131 echo "$MBLOCKHASH" >rp_expected &&
132 echo "$HASH_WELCOME_DOCS" >>rp_expected &&
133 echo QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn >>rp_expected &&
134 ipfs refs -r "$HASH_WELCOME_DOCS" >>rp_expected &&
135 - ipfs pin ls -type=recursive >rp_actual &&
135 + ipfs pin ls --type=recursive >rp_actual &&
136 test_sort_cmp rp_expected rp_actual
137 '
138
139 -test_expect_success "'ipfs pin ls -type=all' is correct" '
139 +test_expect_success "'ipfs pin ls --type=all' is correct" '
140 cat directpinout >allpins &&
141 cat rp_actual >>allpins &&
142 cat indirectpins >>allpins &&
143 cat allpins | sort | uniq >> allpins_uniq &&
144 - ipfs pin ls -type=all >actual_allpins &&
144 + ipfs pin ls --type=all >actual_allpins &&
145 test_sort_cmp allpins_uniq actual_allpins
146 '
147
test/sharness/t0081-repo-pinning.sh
+1 -1
@@ -143,7 +143,7 @@ test_expect_success "added dir was NOT pinned indirectly" '
143 '
144
145 test_expect_success "nothing is pinned directly" '
146 - ipfs pin ls -type=direct >actual4 &&
146 + ipfs pin ls --type=direct >actual4 &&
147 test_must_be_empty actual4
148 '
149
test/sharness/t0100-name.sh
+2 -2
@@ -13,7 +13,7 @@ test_init_ipfs
13 # test publishing a hash
14
15 test_expect_success "'ipfs name publish' succeeds" '
16 - PEERID=`ipfs id -format="<id>"` &&
16 + PEERID=`ipfs id --format="<id>"` &&
17 ipfs name publish "$HASH_WELCOME_DOCS" >publish_out
18 '
19
@@ -34,7 +34,7 @@ test_expect_success "resolve output looks good" '
34 # now test with a path
35
36 test_expect_success "'ipfs name publish' succeeds" '
37 - PEERID=`ipfs id -format="<id>"` &&
37 + PEERID=`ipfs id --format="<id>"` &&
38 ipfs name publish "/ipfs/$HASH_WELCOME_DOCS/help" >publish_out
39 '
40