Merge pull request #1823 from ForrestWeston/recurPin

Pin commands default to recursive

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 <file> means disregard <file> 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)

commands/cli/parse_test.go

@@ -106,17 +106,29 @@ 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"})
+	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) {

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,10 +94,10 @@ on disk.
 
 var rmPinCmd = &cmds.Command{
 	Helptext: cmds.HelpText{
-		Tagline: "Unpin an object from local storage",
+		Tagline: "Removes the pinned object from local storage. (By default, recursively. Use -r=false for direct pins)",
 		ShortDescription: `
 Removes the pin from the given object allowing it to be garbage
-collected if needed.
+collected if needed. (By default, recursively. Use -r=false for direct pins)
 `,
 	},
 
@@ -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=<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 {

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 <hash>' should fail" '
+test_expect_success "'ipfs pin rm -r=false <hash>' 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
 '
 

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 &&