From 4e0ca860e9f9828dcebb54877ecdb29757619027 Mon Sep 17 00:00:00 2001 From: Lars Gierth Date: Wed, 20 May 2015 04:23:43 +0200 Subject: [PATCH 1/5] parse_test: test unwanted stdin --- commands/cli/parse_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 3685c9f48c7..fe2c863c303 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -268,4 +268,7 @@ func TestArgumentParsing(t *testing.T) { fstdin = fileToSimulateStdin(t, "stdin1") test([]string{"stdinenablednotvariadic2args", "value1"}, fstdin, []string{"value1", "stdin1"}) test([]string{"stdinenablednotvariadic2args", "value1", "value2"}, fstdin, []string{"value1", "value2"}) + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"noarg"}, fstdin, []string{}) } From 2eea1b05b7c307516f26c90b8ee3b1b86c7c52fb Mon Sep 17 00:00:00 2001 From: Lars Gierth Date: Wed, 20 May 2015 04:24:31 +0200 Subject: [PATCH 2/5] parse: fix handling of unwanted stdin There can be non-terminal (i.e. non-interactive) sessions that are *not* a pipe, for example: ssh user@host ipfs version In this case, it looks like we should read from stdin. Parsing stdin is accomplished by deliberately triggering the parsing loop once. We didn't previously check whether there is an ArgDef to support that loop iteration. --- commands/cli/parse.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 92be53fa669..4ff7103a0b4 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -219,9 +219,11 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi } } - // count number of values provided by user + // count number of values provided by user. + // if there is at least one ArgDef, we can safely trigger the inputs loop + // below to parse stdin. numInputs := len(inputs) - if stdin != nil { + if argDef := getArgDef(0, argDefs); argDef != nil && stdin != nil { numInputs += 1 } From 8d6bfec890c9687eaebcb7ae43cc1b9b2e9f9d7f Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 20 May 2015 21:28:41 +0200 Subject: [PATCH 3/5] parse: improve stdin fix License: MIT Signed-off-by: Christian Couder --- commands/cli/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 4ff7103a0b4..11003335ef3 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -223,7 +223,7 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi // if there is at least one ArgDef, we can safely trigger the inputs loop // below to parse stdin. numInputs := len(inputs) - if argDef := getArgDef(0, argDefs); argDef != nil && stdin != nil { + if len(argDefs) > 0 && stdin != nil { numInputs += 1 } From be3b7e13e3a2edbb730ffef2240f50f754073b5a Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 20 May 2015 22:47:14 +0200 Subject: [PATCH 4/5] parse_test: test command with optional arg License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index fe2c863c303..2c42ddcc452 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -144,6 +144,12 @@ func TestArgumentParsing(t *testing.T) { commands.StringArg("b", false, true, "another arg"), }, }, + "optionalsecond": { + Arguments: []commands.Argument{ + commands.StringArg("a", true, false, "some arg"), + commands.StringArg("b", false, false, "another arg"), + }, + }, "reversedoptional": { Arguments: []commands.Argument{ commands.StringArg("a", false, false, "some arg"), @@ -213,6 +219,12 @@ func TestArgumentParsing(t *testing.T) { test([]string{"optional", "value!"}, nil, []string{"value!"}) test([]string{"optional"}, nil, []string{}) + test([]string{"optional", "value1", "value2"}, nil, []string{"value1", "value2"}) + + test([]string{"optionalsecond", "value!"}, nil, []string{"value!"}) + test([]string{"optionalsecond", "value1", "value2"}, nil, []string{"value1", "value2"}) + testFail([]string{"optionalsecond"}, "didn't provide any args, 1 required") + testFail([]string{"optionalsecond", "value1", "value2", "value3"}, "provided too many args, takes 2 maximum") test([]string{"reversedoptional", "value1", "value2"}, nil, []string{"value1", "value2"}) test([]string{"reversedoptional", "value!"}, nil, []string{"value!"}) From 23681727e04e652b7f67f8bfa94065209a8b63f3 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 21 May 2015 00:30:08 +0200 Subject: [PATCH 5/5] parse: fix parsing optional arg with stdin License: MIT Signed-off-by: Christian Couder --- commands/cli/parse.go | 2 +- commands/cli/parse_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 11003335ef3..0b99fd49981 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -223,7 +223,7 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi // if there is at least one ArgDef, we can safely trigger the inputs loop // below to parse stdin. numInputs := len(inputs) - if len(argDefs) > 0 && stdin != nil { + if len(argDefs) > 0 && argDefs[len(argDefs)-1].SupportsStdin && stdin != nil { numInputs += 1 } diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 2c42ddcc452..c6215299b73 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -283,4 +283,7 @@ func TestArgumentParsing(t *testing.T) { fstdin = fileToSimulateStdin(t, "stdin1") test([]string{"noarg"}, fstdin, []string{}) + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"optionalsecond", "value1", "value2"}, fstdin, []string{"value1", "value2"}) }