Bool flags in the paired --foo and --no-foo format

pkg/kn/core/root.go

@@ -29,6 +29,7 @@ import (
 	"github.com/knative/client/pkg/kn/commands/revision"
 	"github.com/knative/client/pkg/kn/commands/route"
 	"github.com/knative/client/pkg/kn/commands/service"
+	"github.com/knative/client/pkg/kn/flags"
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -115,6 +116,10 @@ func NewKnCommand(params ...commands.KnParams) *cobra.Command {
 
 		// Prevents Cobra from dealing with errors as we deal with them in main.go
 		SilenceErrors: true,
+
+		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+			return flags.ReconcileBoolFlags(cmd.Flags())
+		},
 	}
 	if p.Output != nil {
 		rootCmd.SetOutput(p.Output)
@@ -123,7 +128,7 @@ func NewKnCommand(params ...commands.KnParams) *cobra.Command {
 	// Persistent flags
 	rootCmd.PersistentFlags().StringVar(&commands.CfgFile, "config", "", "kn config file (default is $HOME/.kn/config.yaml)")
 	rootCmd.PersistentFlags().StringVar(&p.KubeCfgPath, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")
-	rootCmd.PersistentFlags().BoolVar(&p.LogHttp, "log-http", false, "log http traffic")
+	flags.AddBothBoolFlags(rootCmd.PersistentFlags(), &p.LogHttp, "log-http", "", false, "log http traffic")
 
 	plugin.AddPluginFlags(rootCmd)
 	plugin.BindPluginsFlagToViper(rootCmd)

pkg/kn/flags/bool.go

@@ -0,0 +1,79 @@
+// Copyright © 2018 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package flags
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+
+	"github.com/spf13/pflag"
+)
+
+var negPrefix = "no-"
+
+// AddBothBoolFlags adds the given flag in both `--foo` and `--no-foo` variants.
+// If you do this, make sure you call ReconcileBoolFlags later to catch errors and
+// set the relationship between the flag values.
+func AddBothBoolFlags(f *pflag.FlagSet, p *bool, name, short string, value bool, usage string) {
+
+	negativeName := negPrefix + name
+
+	f.BoolVarP(p, name, short, value, usage)
+	f.Bool(negativeName, !value, "do not "+usage)
+
+	if value {
+		err := f.MarkHidden(name)
+		if err != nil {
+			panic(err)
+		}
+	} else {
+		err := f.MarkHidden(negativeName)
+		if err != nil {
+			panic(err)
+		}
+	}
+}
+
+// ReconcileBoolFlags sets the value of the all the "--foo" flags based on
+// "--no-foo" if provided, and returns an error if both were provided.
+func ReconcileBoolFlags(f *pflag.FlagSet) error {
+	var err error
+	f.Visit(func(flag *pflag.Flag) {
+		// Return early from our comprehension
+		if err != nil {
+			return
+		}
+		// 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-") {
+			positiveName := flag.Name[len(negPrefix):len(flag.Name)]
+			positive := f.Lookup(positiveName)
+			if positive.Changed {
+				err = fmt.Errorf("only one of --%s and --%s may be specified",
+					flag.Name, positiveName)
+				return
+			}
+			var noValue bool
+			noValue, err = strconv.ParseBool(flag.Value.String())
+			if err != nil {
+				return
+			}
+			err = positive.Value.Set(strconv.FormatBool(!noValue))
+		}
+	})
+	return err
+}

pkg/kn/flags/bool_test.go

@@ -0,0 +1,55 @@
+// Copyright © 2019 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package flags
+
+import (
+	"testing"
+
+	"github.com/spf13/pflag"
+	"gotest.tools/assert"
+	"gotest.tools/assert/cmp"
+)
+
+type boolPairTestCase struct {
+	name            string
+	defaultVal      bool
+	flags           []string
+	expectedResult  bool
+	expectedErrText string
+}
+
+func TestBooleanPair(t *testing.T) {
+	cases := []*boolPairTestCase{
+		{"foo", true, []string{}, true, ""},
+		{"foo", true, []string{"--foo"}, true, ""},
+		{"foo", true, []string{"--no-foo"}, false, ""},
+		{"foo", false, []string{"--foo"}, true, ""},
+		{"foo", false, []string{}, false, ""},
+		{"foo", false, []string{"--no-foo"}, false, ""},
+		{"foo", true, []string{"--foo", "--no-foo"}, false, "only one of"},
+	}
+	for _, c := range cases {
+		f := &pflag.FlagSet{}
+		var result bool
+		AddBothBoolFlags(f, &result, c.name, "", c.defaultVal, "set "+c.name)
+		f.Parse(c.flags)
+		err := ReconcileBoolFlags(f)
+		if c.expectedErrText != "" {
+			assert.ErrorContains(t, err, c.expectedErrText)
+		} else {
+			assert.Assert(t, cmp.Equal(result, c.expectedResult))
+		}
+	}
+}