Clean up bool flags a little (#362)

pkg/kn/flags/bool.go

@@ -60,45 +60,44 @@ func ReconcileBoolFlags(f *pflag.FlagSet) error {
 		// Walk the "no-" versions of the flags. Make sure we didn't set
 		// both, and set the positive value to the opposite of the "no-"
 		// value if it exists.
-		if strings.HasPrefix(flag.Name, "no-") {
+		if strings.HasPrefix(flag.Name, negPrefix) {
 			positiveName := flag.Name[len(negPrefix):]
 			positive := f.Lookup(positiveName)
+			// Non-paired flag, or wrong types
+			if positive == nil || positive.Value.Type() != "bool" || flag.Value.Type() != "bool" {
+				return
+			}
 			if flag.Changed {
 				if positive.Changed {
 					err = fmt.Errorf("only one of --%s and --%s may be specified",
 						flag.Name, positiveName)
 					return
 				}
-				var noValue bool
+				err = checkExplicitFalse(flag, positiveName)
-				noValue, err = strconv.ParseBool(flag.Value.String())
 				if err != nil {
 					return
 				}
-				if !noValue {
+				err = positive.Value.Set("false")
-					err = fmt.Errorf("use --%s instead of providing false to --%s",
+			} else {
-						positiveName, flag.Name)
+				err = checkExplicitFalse(positive, flag.Name)
-					if err != nil {
-						return
-					}
-				}
-				err = positive.Value.Set(strconv.FormatBool(!noValue))
-			} else if positive.Changed {
-				// For the positive version, just check it wasn't set to the
-				// confusing "false" value.
-				var yesValue bool
-				yesValue, err = strconv.ParseBool(positive.Value.String())
-				if err != nil {
-					return
-				}
-				if !yesValue {
-					err = fmt.Errorf("use --%s instead of providing false to --%s",
-						flag.Name, positiveName)
-					if err != nil {
-						return
-					}
-				}
 			}
 		}
 	})
 	return err
 }
+
+// checkExplicitFalse returns an error if the flag was explicitly set to false.
+func checkExplicitFalse(f *pflag.Flag, betterFlag string) error {
+	if !f.Changed {
+		return nil
+	}
+	val, err := strconv.ParseBool(f.Value.String())
+	if err != nil {
+		return err
+	}
+	if !val {
+		return fmt.Errorf("use --%s instead of providing \"%s\" to --%s",
+			betterFlag, f.Value.String(), f.Name)
+	}
+	return nil
+}

pkg/kn/flags/bool_test.go

@@ -40,8 +40,8 @@ func TestBooleanPair(t *testing.T) {
 		{"foo", true, []string{"--foo", "--no-foo"}, false, "only one of"},
 		{"foo", true, []string{"--no-foo", "--foo"}, false, "only one of"},
 		// Disallow confusing "false" value.
-		{"foo", true, []string{"--foo=false"}, false, "use --no-foo instead of providing false to --foo"},
+		{"foo", true, []string{"--foo=false"}, false, "use --no-foo instead of providing \"false\" to --foo"},
-		{"foo", true, []string{"--no-foo=false"}, false, "use --foo instead of providing false to --no-foo"},
+		{"foo", true, []string{"--no-foo=false"}, false, "use --foo instead of providing \"false\" to --no-foo"},
 
 		// Ensure tests still pass if positive sorts after no- alphabetically.
 		{"zoo", true, []string{}, true, ""},
@@ -53,8 +53,8 @@ func TestBooleanPair(t *testing.T) {
 		{"zoo", true, []string{"--zoo", "--no-zoo"}, false, "only one of"},
 		{"zoo", true, []string{"--no-zoo", "--zoo"}, false, "only one of"},
 		// Disallow confusing "false" value.
-		{"zoo", true, []string{"--zoo=false"}, false, "use --no-zoo instead of providing false to --zoo"},
+		{"zoo", true, []string{"--zoo=false"}, false, "use --no-zoo instead of providing \"false\" to --zoo"},
-		{"zoo", true, []string{"--no-zoo=false"}, false, "use --zoo instead of providing false to --no-zoo"},
+		{"zoo", true, []string{"--no-zoo=false"}, false, "use --zoo instead of providing \"false\" to --no-zoo"},
 	}
 	for _, c := range cases {
 		f := &pflag.FlagSet{}