diff --git a/cmd/ipfs/main.go b/cmd/ipfs/main.go index 2ad90cfc1..73952676a 100644 --- a/cmd/ipfs/main.go +++ b/cmd/ipfs/main.go @@ -93,6 +93,12 @@ func main() { fmt.Fprintf(w, "Use 'ipfs %s --help' for information about this command\n", cmdPath) } + // Handle `ipfs help' + if len(os.Args) == 2 && os.Args[1] == "help" { + printHelp(false, os.Stdout) + os.Exit(0) + } + // parse the commandline into a command invocation parseErr := invoc.Parse(ctx, os.Args[1:]) @@ -110,13 +116,6 @@ func main() { } } - // here we handle the cases where - // - commands with no Run func are invoked directly. - // - the main command is invoked. - if invoc.cmd == nil || invoc.cmd.Run == nil { - printHelp(false, os.Stdout) - os.Exit(0) - } // ok now handle parse error (which means cli input was wrong, // e.g. incorrect number of args, or nonexistent subcommand) @@ -132,6 +131,14 @@ func main() { os.Exit(1) } + // here we handle the cases where + // - commands with no Run func are invoked directly. + // - the main command is invoked. + if invoc.cmd == nil || invoc.cmd.Run == nil { + printHelp(false, os.Stdout) + os.Exit(0) + } + // ok, finally, run the command invocation. intrh, ctx := invoc.SetupInterruptHandler(ctx) defer intrh.Close() diff --git a/commands/cli/helptext.go b/commands/cli/helptext.go index a3406c1c8..9d607c0fd 100644 --- a/commands/cli/helptext.go +++ b/commands/cli/helptext.go @@ -14,7 +14,8 @@ const ( requiredArg = "<%v>" optionalArg = "[<%v>]" variadicArg = "%v..." - optionFlag = "-%v" + shortFlag = "-%v" + longFlag = "--%v" optionType = "(%v)" whitespace = "\r\n\t " @@ -219,6 +220,14 @@ func argumentText(cmd *cmds.Command) []string { return lines } +func optionFlag(flag string) string { + if len(flag) == 1 { + return fmt.Sprintf(shortFlag, flag) + } else { + return fmt.Sprintf(longFlag, flag) + } +} + func optionText(cmd ...*cmds.Command) []string { // get a slice of the options we want to list out options := make([]cmds.Option, 0) @@ -241,7 +250,7 @@ func optionText(cmd ...*cmds.Command) []string { names := sortByLength(opt.Names()) if len(names) >= j+1 { - lines[i] += fmt.Sprintf(optionFlag, names[j]) + lines[i] += optionFlag(names[j]) } if len(names) > j+1 { lines[i] += ", " diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 6abdbf402..86835cbf4 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -2,7 +2,6 @@ package cli import ( "bytes" - "errors" "fmt" "os" "runtime" @@ -13,20 +12,12 @@ import ( u "github.com/ipfs/go-ipfs/util" ) -// ErrInvalidSubcmd signals when the parse error is not found -var ErrInvalidSubcmd = errors.New("subcommand not found") - // Parse parses the input commandline string (cmd, flags, and args). // returns the corresponding command Request object. func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *cmds.Command, []string, error) { - path, input, cmd := parsePath(input, root) - if len(path) == 0 { - return nil, nil, path, ErrInvalidSubcmd - } - - opts, stringVals, err := parseOptions(input) + path, opts, stringVals, cmd, err := parseOpts(input, root) if err != nil { - return nil, cmd, path, err + return nil, nil, path, err } optDefs, err := root.GetOptions(path) @@ -34,14 +25,6 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c return nil, cmd, path, err } - // check to make sure there aren't any undefined options - for k := range opts { - if _, found := optDefs[k]; !found { - err = fmt.Errorf("Unrecognized option: -%s", k) - return nil, cmd, path, err - } - } - req, err := cmds.NewRequest(path, opts, nil, nil, cmd, optDefs) if err != nil { return nil, cmd, path, err @@ -75,67 +58,142 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c return req, cmd, path, nil } -// parsePath separates the command path and the opts and args from a command string -// returns command path slice, rest slice, and the corresponding *cmd.Command -func parsePath(input []string, root *cmds.Command) ([]string, []string, *cmds.Command) { - cmd := root - path := make([]string, 0, len(input)) - input2 := make([]string, 0, len(input)) +// Parse a command line made up of sub-commands, short arguments, long arguments and positional arguments +func parseOpts(args []string, root *cmds.Command) ( + path []string, + opts map[string]interface{}, + stringVals []string, + cmd *cmds.Command, + err error, +) { + path = make([]string, 0, len(args)) + stringVals = make([]string, 0, len(args)) + optDefs := map[string]cmds.Option{} + opts = map[string]interface{}{} + cmd = root - for i, blob := range input { - if strings.HasPrefix(blob, "-") { - input2 = append(input2, blob) - continue + // parseFlag checks that a flag is valid and saves it into opts + // Returns true if the optional second argument is used + parseFlag := func(name string, arg *string, mustUse bool) (bool, error) { + if _, ok := opts[name]; ok { + return false, fmt.Errorf("Duplicate values for option '%s'", name) } - sub := cmd.Subcommand(blob) - if sub == nil { - input2 = append(input2, input[i:]...) - break + optDef, found := optDefs[name] + if !found { + err = fmt.Errorf("Unrecognized option '%s'", name) + return false, err } - cmd = sub - path = append(path, blob) - } - return path, input2, cmd -} - -// parseOptions parses the raw string values of the given options -// returns the parsed options as strings, along with the CLI args -func parseOptions(input []string) (map[string]interface{}, []string, error) { - opts := make(map[string]interface{}) - args := []string{} - - for i := 0; i < len(input); i++ { - blob := input[i] - - if strings.HasPrefix(blob, "-") { - name := blob[1:] - value := "" - - // support single and double dash - if strings.HasPrefix(name, "-") { - name = name[1:] + if optDef.Type() == cmds.Bool { + if mustUse { + return false, fmt.Errorf("Option '%s' takes no arguments, but was passed '%s'", name, *arg) } - - if strings.Contains(name, "=") { - split := strings.SplitN(name, "=", 2) - name = split[0] - value = split[1] - } - - if _, ok := opts[name]; ok { - return nil, nil, fmt.Errorf("Duplicate values for option '%s'", name) - } - - opts[name] = value - + opts[name] = "" + return false, nil } else { - args = append(args, blob) + if arg == nil { + return true, fmt.Errorf("Missing argument for option '%s'", name) + } + opts[name] = *arg + return true, nil } } - return opts, args, nil + optDefs, err = root.GetOptions(path) + if err != nil { + return + } + + consumed := false + for i, arg := range args { + switch { + case consumed: + // arg was already consumed by the preceding flag + consumed = false + continue + + case arg == "--": + // treat all remaining arguments as positional arguments + stringVals = append(stringVals, args[i+1:]...) + return + + case strings.HasPrefix(arg, "--"): + // arg is a long flag, with an optional argument specified + // using `=' or in args[i+1] + var slurped bool + var next *string + split := strings.SplitN(arg, "=", 2) + if len(split) == 2 { + slurped = false + arg = split[0] + next = &split[1] + } else { + slurped = true + if i+1 < len(args) { + next = &args[i+1] + } else { + next = nil + } + } + consumed, err = parseFlag(arg[2:], next, len(split) == 2) + if err != nil { + return + } + if !slurped { + consumed = false + } + + case strings.HasPrefix(arg, "-") && arg != "-": + // args is one or more flags in short form, followed by an optional argument + // all flags except the last one have type bool + for arg = arg[1:]; len(arg) != 0; arg = arg[1:] { + var rest *string + var slurped bool + mustUse := false + if len(arg) > 1 { + slurped = false + str := arg[1:] + if len(str) > 0 && str[0] == '=' { + str = str[1:] + mustUse = true + } + rest = &str + } else { + slurped = true + if i+1 < len(args) { + rest = &args[i+1] + } else { + rest = nil + } + } + var end bool + end, err = parseFlag(arg[0:1], rest, mustUse) + if err != nil { + return + } + if end { + consumed = slurped + break + } + } + + default: + // arg is a sub-command or a positional argument + sub := cmd.Subcommand(arg) + if sub != nil { + cmd = sub + path = append(path, arg) + optDefs, err = root.GetOptions(path) + if err != nil { + return + } + } else { + stringVals = append(stringVals, arg) + } + } + } + return } 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 // and the last arg definition is not variadic (or there are no definitions), return an error notVariadic := len(argDefs) == 0 || !argDefs[len(argDefs)-1].Variadic if notVariadic && numInputs > len(argDefs) { - return nil, nil, fmt.Errorf("Expected %v arguments, got %v", len(argDefs), numInputs) + return nil, nil, fmt.Errorf("Expected %v arguments, got %v: %v", len(argDefs), numInputs, inputs) } stringArgs := make([]string, 0, numInputs) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index be84d6d43..edbca7f76 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -1,7 +1,7 @@ package cli import ( - //"fmt" + "strings" "testing" "github.com/ipfs/go-ipfs/commands" @@ -11,43 +11,79 @@ func TestOptionParsing(t *testing.T) { subCmd := &commands.Command{} cmd := &commands.Command{ Options: []commands.Option{ - commands.StringOption("b", "some option"), + commands.StringOption("string", "s", "a string"), + commands.BoolOption("bool", "b", "a bool"), }, Subcommands: map[string]*commands.Command{ "test": subCmd, }, } - opts, input, err := parseOptions([]string{"--beep", "-boop=lol", "test2", "-c", "beep", "--foo=5"}) - /*for k, v := range opts { - fmt.Printf("%s: %s\n", k, v) - } - fmt.Printf("%s\n", input)*/ - if err != nil { - t.Error("Should have passed") - } - if len(opts) != 4 || opts["beep"] != "" || opts["boop"] != "lol" || opts["c"] != "" || opts["foo"] != "5" { - t.Errorf("Returned options were defferent than expected: %v", opts) - } - if len(input) != 2 || input[0] != "test2" || input[1] != "beep" { - t.Errorf("Returned input was different than expected: %v", input) + type kvs map[string]interface{} + type words []string + + sameWords := func(a words, b words) bool { + for i, w := range a { + if w != b[i] { + return false + } + } + return true } - _, _, err = parseOptions([]string{"-beep=1", "-boop=2", "-beep=3"}) - if err == nil { - t.Error("Should have failed (duplicate option name)") + sameKVs := func(a kvs, b kvs) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + if v != b[k] { + return false + } + } + return true } - path, args, sub := parsePath([]string{"test", "beep", "boop"}, cmd) - if len(path) != 1 || path[0] != "test" { - t.Errorf("Returned path was defferent than expected: %v", path) + testHelper := func(args string, expectedOpts kvs, expectedWords words, expectErr bool) { + _, opts, input, _, err := parseOpts(strings.Split(args, " "), cmd) + if expectErr { + if err == nil { + t.Errorf("Command line '%v' parsing should have failed", args) + } + } else if err != nil { + t.Errorf("Command line '%v' failed to parse: %v", args, err) + } else if !sameWords(input, expectedWords) || !sameKVs(opts, expectedOpts) { + t.Errorf("Command line '%v':\n parsed as %v %v\n instead of %v %v", + args, opts, input, expectedOpts, expectedWords) + } } - if len(args) != 2 || args[0] != "beep" || args[1] != "boop" { - t.Errorf("Returned args were different than expected: %v", args) + + testFail := func(args string) { + testHelper(args, kvs{}, words{}, true) } - if sub != subCmd { - t.Errorf("Returned command was different than expected") + + test := func(args string, expectedOpts kvs, expectedWords words) { + testHelper(args, expectedOpts, expectedWords, false) } + + test("-", kvs{}, words{"-"}) + testFail("-b -b") + test("beep boop", kvs{}, words{"beep", "boop"}) + test("test beep boop", kvs{}, words{"beep", "boop"}) + testFail("-s") + test("-s foo", kvs{"s": "foo"}, words{}) + test("-sfoo", kvs{"s": "foo"}, words{}) + test("-s=foo", kvs{"s": "foo"}, words{}) + test("-b", kvs{"b": ""}, words{}) + test("-bs foo", kvs{"b": "", "s": "foo"}, words{}) + test("-sb", kvs{"s": "b"}, words{}) + test("-b foo", kvs{"b": ""}, words{"foo"}) + test("--bool foo", kvs{"bool": ""}, words{"foo"}) + testFail("--bool=foo") + testFail("--string") + test("--string foo", kvs{"string": "foo"}, words{}) + test("--string=foo", kvs{"string": "foo"}, words{}) + test("-- -b", kvs{}, words{"-b"}) + test("foo -b", kvs{"b": ""}, words{"foo"}) } func TestArgumentParsing(t *testing.T) { diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index a59e5cd95..1717dfd91 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -40,7 +40,7 @@ Please run the ipfs migration tool before continuing. ` + migrationInstructions var ( - ErrNoRepo = errors.New("no ipfs repo found. please run: ipfs init") + ErrNoRepo = func (path string) error { return fmt.Errorf("no ipfs repo found in '%s'. please run: ipfs init ", path) } ErrNoVersion = errors.New("no version file found, please run 0-to-1 migration tool.\n" + migrationInstructions) ErrOldRepo = errors.New("ipfs repo found in old '~/.go-ipfs' location, please run migration tool.\n" + migrationInstructions) ) @@ -172,7 +172,7 @@ func checkInitialized(path string) error { if isInitializedUnsynced(alt) { return ErrOldRepo } - return ErrNoRepo + return ErrNoRepo(path) } return nil } diff --git a/test/sharness/lib/test-lib.sh b/test/sharness/lib/test-lib.sh index 1783ca31f..542167c6d 100644 --- a/test/sharness/lib/test-lib.sh +++ b/test/sharness/lib/test-lib.sh @@ -105,7 +105,7 @@ test_wait_open_tcp_port_10_sec() { # was setting really weird things and am not sure why. test_config_set() { - # grab flags (like -bool in "ipfs config -bool") + # grab flags (like --bool in "ipfs config --bool") test_cfg_flags="" # unset in case. test "$#" = 3 && { test_cfg_flags=$1; shift; } @@ -184,7 +184,7 @@ test_config_ipfs_gateway_writable() { test_config_ipfs_gateway_readonly $1 test_expect_success "prepare config -- gateway writable" ' - test_config_set -bool Gateway.Writable true || + test_config_set --bool Gateway.Writable true || test_fsh cat "\"$IPFS_PATH/config\"" ' } diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 8ab202bb7..10aed018d 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -7,7 +7,7 @@ test_description="Test config command" # we use a function so that we can run it both offline + online test_config_cmd_set() { - # flags (like -bool in "ipfs config -bool") + # flags (like --bool in "ipfs config --bool") cfg_flags="" # unset in case. test "$#" = 3 && { cfg_flags=$1; shift; } @@ -41,8 +41,8 @@ test_config_cmd() { test_config_cmd_set "beep" "boop" test_config_cmd_set "beep1" "boop2" test_config_cmd_set "beep1" "boop2" - test_config_cmd_set "-bool" "beep2" "true" - test_config_cmd_set "-bool" "beep2" "false" + test_config_cmd_set "--bool" "beep2" "true" + test_config_cmd_set "--bool" "beep2" "false" } diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh index 6c0b0cdb8..15b4ed448 100755 --- a/test/sharness/t0080-repo.sh +++ b/test/sharness/t0080-repo.sh @@ -17,7 +17,7 @@ test_expect_success "'ipfs add afile' succeeds" ' ' test_expect_success "added file was pinned" ' - ipfs pin ls -type=recursive >actual && + ipfs pin ls --type=recursive >actual && grep "$HASH" actual ' @@ -49,7 +49,7 @@ test_expect_success "file no longer pinned" ' echo "$HASH_WELCOME_DOCS" >expected2 && ipfs refs -r "$HASH_WELCOME_DOCS" >>expected2 && echo QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn >> expected2 && - ipfs pin ls -type=recursive >actual2 && + ipfs pin ls --type=recursive >actual2 && test_sort_cmp expected2 actual2 ' @@ -102,10 +102,10 @@ test_expect_success "adding multiblock random file succeeds" ' MBLOCKHASH=`ipfs add -q multiblock` ' -test_expect_success "'ipfs pin ls -type=indirect' is correct" ' +test_expect_success "'ipfs pin ls --type=indirect' is correct" ' ipfs refs "$MBLOCKHASH" >refsout && ipfs refs -r "$HASH_WELCOME_DOCS" >>refsout && - ipfs pin ls -type=indirect >indirectpins && + ipfs pin ls --type=indirect >indirectpins && test_sort_cmp refsout indirectpins ' @@ -121,27 +121,27 @@ test_expect_success "pin something directly" ' test_cmp expected10 actual10 ' -test_expect_success "'ipfs pin ls -type=direct' is correct" ' +test_expect_success "'ipfs pin ls --type=direct' is correct" ' echo "$DIRECTPIN" >directpinexpected && - ipfs pin ls -type=direct >directpinout && + ipfs pin ls --type=direct >directpinout && test_sort_cmp directpinexpected directpinout ' -test_expect_success "'ipfs pin ls -type=recursive' is correct" ' +test_expect_success "'ipfs pin ls --type=recursive' is correct" ' echo "$MBLOCKHASH" >rp_expected && echo "$HASH_WELCOME_DOCS" >>rp_expected && echo QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn >>rp_expected && ipfs refs -r "$HASH_WELCOME_DOCS" >>rp_expected && - ipfs pin ls -type=recursive >rp_actual && + ipfs pin ls --type=recursive >rp_actual && test_sort_cmp rp_expected rp_actual ' -test_expect_success "'ipfs pin ls -type=all' is correct" ' +test_expect_success "'ipfs pin ls --type=all' is correct" ' cat directpinout >allpins && cat rp_actual >>allpins && cat indirectpins >>allpins && cat allpins | sort | uniq >> allpins_uniq && - ipfs pin ls -type=all >actual_allpins && + ipfs pin ls --type=all >actual_allpins && test_sort_cmp allpins_uniq actual_allpins ' diff --git a/test/sharness/t0081-repo-pinning.sh b/test/sharness/t0081-repo-pinning.sh index 9c7bd2bf6..fa95da8ad 100755 --- a/test/sharness/t0081-repo-pinning.sh +++ b/test/sharness/t0081-repo-pinning.sh @@ -143,7 +143,7 @@ test_expect_success "added dir was NOT pinned indirectly" ' ' test_expect_success "nothing is pinned directly" ' - ipfs pin ls -type=direct >actual4 && + ipfs pin ls --type=direct >actual4 && test_must_be_empty actual4 ' diff --git a/test/sharness/t0100-name.sh b/test/sharness/t0100-name.sh index da995403d..07ef2f82f 100755 --- a/test/sharness/t0100-name.sh +++ b/test/sharness/t0100-name.sh @@ -13,7 +13,7 @@ test_init_ipfs # test publishing a hash test_expect_success "'ipfs name publish' succeeds" ' - PEERID=`ipfs id -format=""` && + PEERID=`ipfs id --format=""` && ipfs name publish "$HASH_WELCOME_DOCS" >publish_out ' @@ -34,7 +34,7 @@ test_expect_success "resolve output looks good" ' # now test with a path test_expect_success "'ipfs name publish' succeeds" ' - PEERID=`ipfs id -format=""` && + PEERID=`ipfs id --format=""` && ipfs name publish "/ipfs/$HASH_WELCOME_DOCS/help" >publish_out '