From 107409cee80562b57637a00e1bf72e5bffc24680 Mon Sep 17 00:00:00 2001 From: ForrestWeston Date: Fri, 9 Oct 2015 17:29:57 -0700 Subject: [PATCH 1/4] Added ability to specify true/false for bool opts License: MIT Signed-off-by: ForrestWeston --- commands/cli/parse.go | 25 ++++++++++++++++++++----- commands/cli/parse_test.go | 18 +++++++++++++----- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 2f4b2f5c6..f758429b8 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -85,13 +85,28 @@ func parseOpts(args []string, root *cmds.Command) ( err = fmt.Errorf("Unrecognized option '%s'", name) return false, err } - + // mustUse implies that you must use the argument given after the '=' + // eg. -r=true means you must take true into consideration + // mustUse == true in the above case + // eg. ipfs -r means disregard since there is no '=' + // mustUse == false in the above situation + //arg == nil implies the flag was specified without an argument if optDef.Type() == cmds.Bool { - if mustUse { - return false, fmt.Errorf("Option '%s' takes no arguments, but was passed '%s'", name, *arg) + if arg == nil || !mustUse { + opts[name] = true + return false, nil + } + argVal := strings.ToLower(*arg) + switch argVal { + case "true": + opts[name] = true + return true, nil + case "false": + opts[name] = false + return true, nil + default: + return true, fmt.Errorf("Option '%s' takes true/false arguments, but was passed '%s'", name, argVal) } - opts[name] = "" - return false, nil } else { if arg == nil { return true, fmt.Errorf("Missing argument for option '%s'", name) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index c6215299b..c02ac9f4d 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -106,17 +106,25 @@ func TestOptionParsing(t *testing.T) { 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("-b", kvs{"b": true}, words{}) + test("-bs foo", kvs{"b": true, "s": "foo"}, words{}) test("-sb", kvs{"s": "b"}, words{}) - test("-b foo", kvs{"b": ""}, words{"foo"}) - test("--bool foo", kvs{"bool": ""}, words{"foo"}) + test("-b foo", kvs{"b": true}, words{"foo"}) + test("--bool foo", kvs{"bool": true}, 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"}) + test("foo -b", kvs{"b": true}, words{"foo"}) + test("-b=false", kvs{"b": false}, words{}) + test("-b=true", kvs{"b": true}, words{}) + test("-b=false foo", kvs{"b": false}, words{"foo"}) + test("-b=true foo", kvs{"b": true}, words{"foo"}) + test("--bool=true foo", kvs{"bool": true}, words{"foo"}) + test("--bool=false foo", kvs{"bool": false}, words{"foo"}) + test("-b=FaLsE foo", kvs{"b": false}, words{"foo"}) + test("-b=TrUe foo", kvs{"b": true}, words{"foo"}) } func TestArgumentParsing(t *testing.T) { From 01239147b7bf8cae69b0a33b58c70b5b596ad3da Mon Sep 17 00:00:00 2001 From: ForrestWeston Date: Fri, 9 Oct 2015 17:35:23 -0700 Subject: [PATCH 2/4] changed pinning to be recursive by default pin add, pin rm, and pin ls will be recursive unless specified with '=false' eg. 'ipfs pin add -r=false ' tests for pinning have been updated/added License: MIT Signed-off-by: ForrestWeston --- core/commands/pin.go | 43 +++++++++++++++-------------- test/sharness/t0080-repo.sh | 20 ++++++++++---- test/sharness/t0081-repo-pinning.sh | 6 ++-- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/core/commands/pin.go b/core/commands/pin.go index d1beecefd..617fb81da 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -57,7 +57,7 @@ on disk. return } if !found { - recursive = false + recursive = true } added, err := corerepo.Pin(n, req.Context(), req.Arguments(), recursive) @@ -76,8 +76,8 @@ on disk. } var pintype string - rec, _, _ := res.Request().Option("recursive").Bool() - if rec { + rec, found, _ := res.Request().Option("recursive").Bool() + if rec || !found { pintype = "recursively" } else { pintype = "directly" @@ -94,9 +94,9 @@ on disk. var rmPinCmd = &cmds.Command{ Helptext: cmds.HelpText{ - Tagline: "Unpin an object from local storage", + Tagline: "Recursively unpin an object from local storage", ShortDescription: ` -Removes the pin from the given object allowing it to be garbage +Recursively removes the pin from the given object allowing it to be garbage collected if needed. `, }, @@ -122,7 +122,7 @@ collected if needed. return } if !found { - recursive = false // default + recursive = true // default } removed, err := corerepo.Unpin(n, req.Context(), req.Arguments(), recursive) @@ -153,26 +153,27 @@ var listPinCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "List objects pinned to local storage", ShortDescription: ` -Returns a list of hashes of objects being pinned. Objects that are indirectly -or recursively pinned are not included in the list. +Returns a list of objects that are pinned locally. +By default, only recursively pinned returned, but others may be shown via the '--type' flag. `, LongDescription: ` -Returns a list of hashes of objects being pinned. Objects that are indirectly -or recursively pinned are not included in the list. - -Use --type= to specify the type of pinned keys to list. Valid values are: - * "direct": pin that specific object. - * "recursive": pin that specific object, and indirectly pin all its decendants - * "indirect": pinned indirectly by an ancestor (like a refcount) - * "all" - -To see the ref count on indirect pins, pass the -count option flag. -Defaults to "direct". +Returns a list of objects that are pinned locally. +By default, only recursively pinned returned, but others may be shown via the '--type' flag. +Example: + $ echo "hello" | ipfs add -q + QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN + $ ipfs pin ls + QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN + # now remove the pin, and repin it directly + $ ipfs pin rm QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN + $ ipfs pin add -r=false QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN + $ ipfs pin ls --type=direct + QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN `, }, Options: []cmds.Option{ - cmds.StringOption("type", "t", "The type of pinned keys to list. Can be \"direct\", \"indirect\", \"recursive\", or \"all\". Defaults to \"direct\""), + cmds.StringOption("type", "t", "The type of pinned keys to list. Can be \"direct\", \"indirect\", \"recursive\", or \"all\". Defaults to \"recursive\""), cmds.BoolOption("count", "n", "Show refcount when listing indirect pins"), cmds.BoolOption("quiet", "q", "Write just hashes of objects"), }, @@ -189,7 +190,7 @@ Defaults to "direct". return } if !found { - typeStr = "direct" + typeStr = "recursive" } switch typeStr { diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh index 320bda08a..ea24ec02c 100755 --- a/test/sharness/t0080-repo.sh +++ b/test/sharness/t0080-repo.sh @@ -74,20 +74,28 @@ test_expect_success "file no longer pinned" ' test_sort_cmp expected2 actual2 ' -test_expect_success "recursively pin afile" ' +test_expect_success "recursively pin afile(default action)" ' HASH=`ipfs add -q afile` && + ipfs pin add "$HASH" +' + +test_expect_success "recursively pin rm afile (default action)" ' + ipfs pin rm "$HASH" +' + +test_expect_success "recursively pin afile" ' ipfs pin add -r "$HASH" ' test_expect_success "pinning directly should fail now" ' echo "Error: pin: $HASH already pinned recursively" >expected3 && - test_must_fail ipfs pin add "$HASH" 2>actual3 && + test_must_fail ipfs pin add -r=false "$HASH" 2>actual3 && test_cmp expected3 actual3 ' -test_expect_success "'ipfs pin rm ' should fail" ' +test_expect_success "'ipfs pin rm -r=false ' should fail" ' echo "Error: $HASH is pinned recursively" >expected4 && - test_must_fail ipfs pin rm "$HASH" 2>actual4 && + test_must_fail ipfs pin rm -r=false "$HASH" 2>actual4 && test_cmp expected4 actual4 ' @@ -95,7 +103,7 @@ test_expect_success "remove recursive pin, add direct" ' echo "unpinned $HASH" >expected5 && ipfs pin rm -r "$HASH" >actual5 && test_cmp expected5 actual5 && - ipfs pin add "$HASH" + ipfs pin add -r=false "$HASH" ' test_expect_success "remove direct pin" ' @@ -142,7 +150,7 @@ test_expect_success "pin something directly" ' test_cmp expected9 actual9 && echo "pinned $DIRECTPIN directly" >expected10 && - ipfs pin add "$DIRECTPIN" >actual10 && + ipfs pin add -r=false "$DIRECTPIN" >actual10 && test_cmp expected10 actual10 ' diff --git a/test/sharness/t0081-repo-pinning.sh b/test/sharness/t0081-repo-pinning.sh index 03988db24..1c062d79b 100755 --- a/test/sharness/t0081-repo-pinning.sh +++ b/test/sharness/t0081-repo-pinning.sh @@ -190,9 +190,9 @@ test_expect_success "none are pinned any more" ' ' test_expect_success "pin some directly and indirectly" ' - ipfs pin add "$HASH_DIR1" >actual7 && - ipfs pin add -r "$HASH_DIR2" >>actual7 && - ipfs pin add "$HASH_FILE1" >>actual7 && + ipfs pin add -r=false "$HASH_DIR1" >actual7 && + ipfs pin add -r=true "$HASH_DIR2" >>actual7 && + ipfs pin add -r=false "$HASH_FILE1" >>actual7 && echo "pinned $HASH_DIR1 directly" >expected7 && echo "pinned $HASH_DIR2 recursively" >>expected7 && echo "pinned $HASH_FILE1 directly" >>expected7 && From a8fc65fda014839a3de688208aa899109d22f422 Mon Sep 17 00:00:00 2001 From: ForrestWeston Date: Tue, 13 Oct 2015 17:09:43 -0700 Subject: [PATCH 3/4] Added tests from code-review License: MIT Signed-off-by: ForrestWeston --- commands/cli/parse_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index c02ac9f4d..5564b3b58 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -125,6 +125,10 @@ func TestOptionParsing(t *testing.T) { test("--bool=false foo", kvs{"bool": false}, words{"foo"}) test("-b=FaLsE foo", kvs{"b": false}, words{"foo"}) test("-b=TrUe foo", kvs{"b": true}, words{"foo"}) + test("-b true", kvs{"b": true}, words{"true"}) + test("-b false", kvs{"b": true}, words{"false"}) + test("-b --string foo bar", kvs{"b": true, "string": "foo"}, words{"bar"}) + test("-b=false --string bar", kvs{"b": false, "string": "bar"}, words{}) } func TestArgumentParsing(t *testing.T) { From 9916f8663bfa903ea6ebcfaf13d85af638c9e03c Mon Sep 17 00:00:00 2001 From: ForrestWeston Date: Wed, 14 Oct 2015 09:42:19 -0700 Subject: [PATCH 4/4] Updated Pin Remove Tagline and Description License: MIT Signed-off-by: ForrestWeston --- core/commands/pin.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/commands/pin.go b/core/commands/pin.go index 617fb81da..5aa87924c 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -94,10 +94,10 @@ on disk. var rmPinCmd = &cmds.Command{ Helptext: cmds.HelpText{ - Tagline: "Recursively unpin an object from local storage", + Tagline: "Removes the pinned object from local storage. (By default, recursively. Use -r=false for direct pins)", ShortDescription: ` -Recursively removes the pin from the given object allowing it to be garbage -collected if needed. +Removes the pin from the given object allowing it to be garbage +collected if needed. (By default, recursively. Use -r=false for direct pins) `, },