Skip to content

Commit

Permalink
WIP: flagcodec: enable flag normalization
Browse files Browse the repository at this point in the history 
Signed-off-by: Francesco Romani <[email protected]>
  • Loading branch information
@ffromani
ffromani committed Nov 29, 2023
1 parent b5639b3 commit d994faa
Show file tree
Hide file tree
Showing 2 changed files with 177 additions and 18 deletions.
62 changes: 55 additions & 7 deletions pkg/flagcodec/flagcodec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ type Val struct {
}

type Flags struct {
command string
args map[string]Val
keys []string
command string
args map[string]Val
keys []string
processFlagName func(string) string
}

// ParseArgvKeyValue parses a clean (trimmed) argv whose components
Expand All @@ -53,7 +54,7 @@ type Flags struct {
// "--opt", "foo"
// The value of argv[0], whatever it is, is taken at command.
func ParseArgvKeyValue(args []string) *Flags {
return ParseArgvKeyValueWithCommand("", args)
return ParseArgvKeyValueWithOptions("", args)
}

// ParseArgvKeyValueWithCommand parses a clean (trimmed) argv whose components
Expand All @@ -64,11 +65,30 @@ func ParseArgvKeyValue(args []string) *Flags {
// "--opt=foo"
// AND NOT
// "--opt", "foo"
// The command is supplied explicitely as parameter.
// The command is supplied explicitly as parameter.
func ParseArgvKeyValueWithCommand(command string, args []string) *Flags {
return ParseArgvKeyValueWithOptions(command, args)
}

type Option func(*Flags) *Flags

// ParseArgvKeyValueWithOptions parses a clean (trimmed) argv whose components
// are either toggles or key=value pairs. IOW, this is a restricted and easier
// to parse flavor of argv on which option and value are guaranteed to
// be in the same item.
// IOW, we expect
// "--opt=foo"
// AND NOT
// "--opt", "foo"
// The command is supplied explicitly as parameter.
func ParseArgvKeyValueWithOptions(command string, args []string, opts ...Option) *Flags {
ret := &Flags{
command: command,
args: make(map[string]Val),
command: command,
args: make(map[string]Val),
processFlagName: func(v string) string { return v },
}
for _, opt := range opts {
opt(ret)
}
for _, arg := range args {
fields := strings.SplitN(arg, "=", 2)
Expand All @@ -81,6 +101,30 @@ func ParseArgvKeyValueWithCommand(command string, args []string) *Flags {
return ret
}

func normalizeFlagName(v string) string {
if len(v) == 3 && v[0] == '-' && v[1] == '-' {
// single char, double dash flag (ugly?), fix it
return v[1:]
}
// everything else pass through silently
return v
}

// WithFlagNormalization optionally enables flag normalization.
// The canonical representation of flags in this package is:
// * single-dash for one-char flags (-v, -h)
// * double-dash for multi-char flags (--foo, --long-option)
// pflag allows one-char to have one or two dashes. For flagcodec
// these were different options. When normalization is enabled,
// though, all flag names are processed to adhere to the canonical
// representation, so flagcodec will treat `--v` and `-v` to
// be the same flag. Since this is possibly breaking change,
// this treatment is opt-in.
func WithFlagNormalization(fl *Flags) *Flags {
fl.processFlagName = normalizeFlagName
return fl
}

func (fl *Flags) recordFlag(name string) {
if _, ok := fl.args[name]; !ok {
fl.keys = append(fl.keys, name)
Expand All @@ -99,13 +143,15 @@ func (fl *Flags) forgetFlag(name string) {
}

func (fl *Flags) SetToggle(name string) {
name = fl.processFlagName(name)
fl.recordFlag(name)
fl.args[name] = Val{
Kind: FlagToggle,
}
}

func (fl *Flags) SetOption(name, data string) {
name = fl.processFlagName(name)
fl.recordFlag(name)
fl.args[name] = Val{
Kind: FlagOption,
Expand All @@ -114,6 +160,7 @@ func (fl *Flags) SetOption(name, data string) {
}

func (fl *Flags) Delete(name string) {
name = fl.processFlagName(name)
fl.forgetFlag(name)
delete(fl.args, name)
}
Expand All @@ -139,6 +186,7 @@ func (fl *Flags) Argv() []string {
}

func (fl *Flags) GetFlag(name string) (Val, bool) {
name = fl.processFlagName(name)
if val, ok := fl.args[name]; ok {
return val, ok
}
Expand Down
133 changes: 122 additions & 11 deletions pkg/flagcodec/flagcodec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,25 @@ func TestParseStringRoundTrip(t *testing.T) {
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fl := ParseArgvKeyValue(tc.argv)
got := fl.Argv()
if !reflect.DeepEqual(tc.expected, got) {
t.Errorf("expected %v got %v", tc.expected, got)
for _, normFlag := range []bool{false, true} {
for _, tc := range testCases {
name := tc.name
if normFlag {
name += "-norm-flag"
}
})
t.Run(name, func(t *testing.T) {
var fl *Flags
if normFlag {
fl = ParseArgvKeyValueWithOptions("", tc.argv, WithFlagNormalization)
} else {
fl = ParseArgvKeyValue(tc.argv)
}
got := fl.Argv()
if !reflect.DeepEqual(tc.expected, got) {
t.Errorf("expected %v got %v", tc.expected, got)
}
})
}
}
}

Expand Down Expand Up @@ -117,9 +128,80 @@ func TestParseStringRoundTripWithCommand(t *testing.T) {
},
}

for _, normFlag := range []bool{false, true} {
for _, tc := range testCases {
name := tc.name
if normFlag {
name += "-norm-flag"
}
t.Run(name, func(t *testing.T) {
var fl *Flags
if normFlag {
fl = ParseArgvKeyValueWithOptions(tc.command, tc.args, WithFlagNormalization)
} else {
fl = ParseArgvKeyValueWithCommand(tc.command, tc.args)
}
got := fl.Argv()
if !reflect.DeepEqual(tc.expected, got) {
t.Errorf("expected %v got %v", tc.expected, got)
}
})
}
}
}

func TestAddFlags(t *testing.T) {
type testOpt struct {
name string
value string
}

type testCase struct {
name string
command string
args []string
options []testOpt
expected []string
}

testCases := []testCase{
{
name: "add-mixed",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
options: []testOpt{
{
name: "--hostname",
value: "host.test.net",
},
{
name: "--v",
value: "2",
},
},
expected: []string{
"/bin/resource-topology-exporter",
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
"--hostname=host.test.net",
"--v=2",
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fl := ParseArgvKeyValueWithCommand(tc.command, tc.args)
for _, opt := range tc.options {
fl.SetOption(opt.name, opt.value)
}
got := fl.Argv()
if !reflect.DeepEqual(tc.expected, got) {
t.Errorf("expected %v got %v", tc.expected, got)
Expand All @@ -128,7 +210,7 @@ func TestParseStringRoundTripWithCommand(t *testing.T) {
}
}

func TestAddFlags(t *testing.T) {
func TestAddFlagsNormalized(t *testing.T) {
type testOpt struct {
name string
value string
Expand All @@ -144,7 +226,7 @@ func TestAddFlags(t *testing.T) {

testCases := []testCase{
{
name: "add-mixed",
name: "add-mixed-two-dashes-single-letter",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
Expand All @@ -169,14 +251,43 @@ func TestAddFlags(t *testing.T) {
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
"--hostname=host.test.net",
"--v=2",
"-v=2",
},
},
{
name: "add-mixed-single-dashe-single-letter",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
options: []testOpt{
{
name: "--hostname",
value: "host.test.net",
},
{
name: "-v",
value: "2",
},
},
expected: []string{
"/bin/resource-topology-exporter",
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
"--hostname=host.test.net",
"-v=2",
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fl := ParseArgvKeyValueWithCommand(tc.command, tc.args)
fl := ParseArgvKeyValueWithOptions(tc.command, tc.args, WithFlagNormalization)
for _, opt := range tc.options {
fl.SetOption(opt.name, opt.value)
}
Expand Down

0 comments on commit d994faa

Please sign in to comment.