From eeeb88a667141380b843d92eb4478553b2256f17 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 16 May 2022 14:30:45 +0200 Subject: shell completion --format: only show exported fields go templates only support exported fields, so the completion logic must filter the private fields out. Signed-off-by: Paul Holzinger --- cmd/podman/common/completion.go | 5 +++++ cmd/podman/common/completion_test.go | 7 +++++++ 2 files changed, 12 insertions(+) (limited to 'cmd') diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index e07e28dab..0db9f7351 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -1054,6 +1054,11 @@ func getStructFields(f reflect.Value, prefix string) []string { // loop over all field names for j := 0; j < f.NumField(); j++ { field := f.Type().Field(j) + // check if struct field is not exported, templates only use exported fields + // PkgPath is always empty for exported fields + if field.PkgPath != "" { + continue + } fname := field.Name suffix := "}}" kind := field.Type.Kind() diff --git a/cmd/podman/common/completion_test.go b/cmd/podman/common/completion_test.go index 13f45a662..c9e189961 100644 --- a/cmd/podman/common/completion_test.go +++ b/cmd/podman/common/completion_test.go @@ -31,6 +31,12 @@ func (c *Car) Color() string { return "" } +// This is for reflect testing required. +// nolint:unused +func (c Car) internal() int { + return 0 +} + func TestAutocompleteFormat(t *testing.T) { testStruct := struct { Name string @@ -38,6 +44,7 @@ func TestAutocompleteFormat(t *testing.T) { Car *Car Car2 *Car *Anonymous + private int }{} testStruct.Car = &Car{} -- cgit v1.2.3-54-g00ecf From 11ff5ffd3b0acba02991b0879a1b8493ea0f3bc9 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 16 May 2022 15:10:58 +0200 Subject: shell completion --format: only show usable methods In a template you cann call function that are defined on a type, however this is only useful if they return one value. If it returns more than one the template cannot know what value it has to display. Signed-off-by: Paul Holzinger --- cmd/podman/common/completion.go | 7 ++++++- cmd/podman/common/completion_test.go | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'cmd') diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 0db9f7351..3c614c0e1 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -1084,7 +1084,12 @@ func getStructFields(f reflect.Value, prefix string) []string { func getMethodNames(f reflect.Value, prefix string) []string { suggestions := make([]string, 0, f.NumMethod()) for j := 0; j < f.NumMethod(); j++ { - fname := f.Type().Method(j).Name + method := f.Type().Method(j) + // in a template we can only run functions with one return value + if method.Func.Type().NumOut() != 1 { + continue + } + fname := method.Name if strings.HasPrefix(fname, prefix) { // add method name with closing braces suggestions = append(suggestions, fname+"}}") diff --git a/cmd/podman/common/completion_test.go b/cmd/podman/common/completion_test.go index c9e189961..63b99501f 100644 --- a/cmd/podman/common/completion_test.go +++ b/cmd/podman/common/completion_test.go @@ -37,6 +37,10 @@ func (c Car) internal() int { return 0 } +func (c Car) TwoOut() (string, string) { + return "", "" +} + func TestAutocompleteFormat(t *testing.T) { testStruct := struct { Name string -- cgit v1.2.3-54-g00ecf From ecd6edb19186b43c064a09b0a824732ff5f5242e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 16 May 2022 16:50:07 +0200 Subject: shell completion --format: fix embedded struct handling When a struct is embeeded it is possible that we end up with same names but different types, this results in incorrect completions. The go template logic always preferes the actual field/method name before the one from the embedded one. Thefore the completion logic should do the same. First get all method/fields names from the struct and then only add the field names from the embedded struct when they are not already present in the list. Signed-off-by: Paul Holzinger --- cmd/podman/common/completion.go | 43 ++++++++++++++++++++++++++++-------- cmd/podman/common/completion_test.go | 21 +++++++++++++----- 2 files changed, 50 insertions(+), 14 deletions(-) (limited to 'cmd') diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 3c614c0e1..4fec549c1 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -960,6 +960,19 @@ func AutocompleteNetworkFlag(cmd *cobra.Command, args []string, toComplete strin return append(networks, suggestions...), dir } +type formatSuggestion struct { + fieldname string + suffix string +} + +func convertFormatSuggestions(suggestions []formatSuggestion) []string { + completions := make([]string, 0, len(suggestions)) + for _, f := range suggestions { + completions = append(completions, f.fieldname+f.suffix) + } + return completions +} + // AutocompleteFormat - Autocomplete json or a given struct to use for a go template. // The input can be nil, In this case only json will be autocompleted. // This function will only work for structs other types are not supported. @@ -999,7 +1012,7 @@ func AutocompleteFormat(o interface{}) func(cmd *cobra.Command, args []string, t toCompArr := strings.Split(toComplete, ".") toCompArr[len(toCompArr)-1] = "" toComplete = strings.Join(toCompArr, ".") - return prefixSlice(toComplete, suggestions), cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp + return prefixSlice(toComplete, convertFormatSuggestions(suggestions)), cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp } val := getActualStructType(f) @@ -1038,8 +1051,8 @@ func getActualStructType(f reflect.Value) *reflect.Value { } // getStructFields reads all struct field names and method names and returns them. -func getStructFields(f reflect.Value, prefix string) []string { - var suggestions []string +func getStructFields(f reflect.Value, prefix string) []formatSuggestion { + var suggestions []formatSuggestion if f.IsValid() { suggestions = append(suggestions, getMethodNames(f, prefix)...) } @@ -1051,6 +1064,7 @@ func getStructFields(f reflect.Value, prefix string) []string { } f = *val + var anonymous []formatSuggestion // loop over all field names for j := 0; j < f.NumField(); j++ { field := f.Type().Field(j) @@ -1072,17 +1086,28 @@ func getStructFields(f reflect.Value, prefix string) []string { } // if field is anonymous add the child fields as well if field.Anonymous { - suggestions = append(suggestions, getStructFields(f.Field(j), prefix)...) - } else if strings.HasPrefix(fname, prefix) { + anonymous = append(anonymous, getStructFields(f.Field(j), prefix)...) + } + if strings.HasPrefix(fname, prefix) { // add field name with suffix - suggestions = append(suggestions, fname+suffix) + suggestions = append(suggestions, formatSuggestion{fieldname: fname, suffix: suffix}) + } + } +outer: + for _, ano := range anonymous { + // we should only add anonymous child fields if they are not already present. + for _, sug := range suggestions { + if ano.fieldname == sug.fieldname { + continue outer + } } + suggestions = append(suggestions, ano) } return suggestions } -func getMethodNames(f reflect.Value, prefix string) []string { - suggestions := make([]string, 0, f.NumMethod()) +func getMethodNames(f reflect.Value, prefix string) []formatSuggestion { + suggestions := make([]formatSuggestion, 0, f.NumMethod()) for j := 0; j < f.NumMethod(); j++ { method := f.Type().Method(j) // in a template we can only run functions with one return value @@ -1092,7 +1117,7 @@ func getMethodNames(f reflect.Value, prefix string) []string { fname := method.Name if strings.HasPrefix(fname, prefix) { // add method name with closing braces - suggestions = append(suggestions, fname+"}}") + suggestions = append(suggestions, formatSuggestion{fieldname: fname, suffix: "}}"}) } } return suggestions diff --git a/cmd/podman/common/completion_test.go b/cmd/podman/common/completion_test.go index 63b99501f..34ec9b6a5 100644 --- a/cmd/podman/common/completion_test.go +++ b/cmd/podman/common/completion_test.go @@ -19,6 +19,17 @@ type Car struct { type Anonymous struct { Hello string + // The name should match the testStruct Name below. This is used to make + // sure the logic uses the actual struct fields before the embedded ones. + Name struct { + Suffix string + Prefix string + } +} + +// The name should match the testStruct Age name below. +func (a Anonymous) Age() int { + return 0 } func (c Car) Type() string { @@ -87,17 +98,17 @@ func TestAutocompleteFormat(t *testing.T) { { "invalid completion", "{{ ..", - nil, + []string{}, }, { "fist level struct field name", "{{.", - []string{"{{.Name}}", "{{.Age}}", "{{.Car.", "{{.Car2.", "{{.Hello}}"}, + []string{"{{.Name}}", "{{.Age}}", "{{.Car.", "{{.Car2.", "{{.Anonymous.", "{{.Hello}}"}, }, { "fist level struct field name", "{{ .", - []string{"{{ .Name}}", "{{ .Age}}", "{{ .Car.", "{{ .Car2.", "{{ .Hello}}"}, + []string{"{{ .Name}}", "{{ .Age}}", "{{ .Car.", "{{ .Car2.", "{{ .Anonymous.", "{{ .Hello}}"}, }, { "fist level struct field name", @@ -137,12 +148,12 @@ func TestAutocompleteFormat(t *testing.T) { { "invalid field name", "{{ .Ca.B", - nil, + []string{}, }, { "map key names don't work", "{{ .Car.Extras.", - nil, + []string{}, }, { "two variables struct field name", -- cgit v1.2.3-54-g00ecf From 3d8a1f91731b7935ae5239023d588a028dcd51e8 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 16 May 2022 20:38:45 +0200 Subject: shell completion --format: support maps and functions Currently we only support structs in a template string like this: `{{.var1.test.` -> this meams that test must be a struct field on var1. Now with this var1 and test could also be either a map or function which returns a struct. A actual example: `podman container inspect --format {{.NetworkSettings.Networks.netname.` Now we can complete the struct fileds after netname. Note that this cannot complete map keys since they are empty by default, so it is impossible to get them in the completion logic. Also this fixes a panic with embeeded nil structs Fixes #14223 Signed-off-by: Paul Holzinger --- cmd/podman/common/completion.go | 76 ++++++++++++++++++++++++------------ cmd/podman/common/completion_test.go | 40 +++++++++++++++---- 2 files changed, 85 insertions(+), 31 deletions(-) (limited to 'cmd') diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 4fec549c1..32dd896c0 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -975,7 +975,7 @@ func convertFormatSuggestions(suggestions []formatSuggestion) []string { // AutocompleteFormat - Autocomplete json or a given struct to use for a go template. // The input can be nil, In this case only json will be autocompleted. -// This function will only work for structs other types are not supported. +// This function will only work for pointer to structs other types are not supported. // When "{{." is typed the field and method names of the given struct will be completed. // This also works recursive for nested structs. func AutocompleteFormat(o interface{}) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { @@ -1004,6 +1004,12 @@ func AutocompleteFormat(o interface{}) func(cmd *cobra.Command, args []string, t // split this into it struct field names fields := strings.Split(field[len(field)-1], ".") f := reflect.ValueOf(o) + if f.Kind() != reflect.Ptr { + // We panic here to make sure that all callers pass the value by reference. + // If someone passes a by value then all podman commands will panic since + // this function is run at init time. + panic("AutocompleteFormat: passed value must be a pointer to a struct") + } for i := 1; i < len(fields); i++ { // last field get all names to suggest if i == len(fields)-1 { @@ -1015,39 +1021,56 @@ func AutocompleteFormat(o interface{}) func(cmd *cobra.Command, args []string, t return prefixSlice(toComplete, convertFormatSuggestions(suggestions)), cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp } - val := getActualStructType(f) - if val == nil { - // no struct return nothing to complete + // first follow pointer and create element when it is nil + f = actualReflectValue(f) + switch f.Kind() { + case reflect.Struct: + for j := 0; j < f.NumField(); j++ { + field := f.Type().Field(j) + // ok this is a bit weird but when we have an embedded nil struct + // calling FieldByName on a name which is present on this struct will panic + // Therefore we have to init them (non nil ptr), https://github.com/containers/podman/issues/14223 + if field.Anonymous && f.Field(j).Type().Kind() == reflect.Ptr { + f.Field(j).Set(reflect.New(f.Field(j).Type().Elem())) + } + } + // set the next struct field + f = f.FieldByName(fields[i]) + case reflect.Map: + rtype := f.Type().Elem() + if rtype.Kind() == reflect.Ptr { + rtype = rtype.Elem() + } + f = reflect.New(rtype) + case reflect.Func: + if f.Type().NumOut() != 1 { + // unsupported type return nothing + return nil, cobra.ShellCompDirectiveNoFileComp + } + f = reflect.New(f.Type().Out(0)) + default: + // unsupported type return nothing return nil, cobra.ShellCompDirectiveNoFileComp } - f = *val - - // set the next struct field - f = f.FieldByName(fields[i]) } return nil, cobra.ShellCompDirectiveNoFileComp } } -// getActualStructType take the value and check if it is a struct, +// actualReflectValue takes the value, // if it is pointer it will dereference it and when it is nil, -// it will create a new value from it to get the actual struct -// returns nil when type is not a struct -func getActualStructType(f reflect.Value) *reflect.Value { +// it will create a new value from it +func actualReflectValue(f reflect.Value) reflect.Value { // follow the pointer first if f.Kind() == reflect.Ptr { // if the pointer is nil we create a new value from the elements type - // this allows us to follow nil pointers and get the actual struct fields + // this allows us to follow nil pointers and get the actual type if f.IsNil() { f = reflect.New(f.Type().Elem()) } f = f.Elem() } - // we only support structs - if f.Kind() != reflect.Struct { - return nil - } - return &f + return f } // getStructFields reads all struct field names and method names and returns them. @@ -1057,12 +1080,11 @@ func getStructFields(f reflect.Value, prefix string) []formatSuggestion { suggestions = append(suggestions, getMethodNames(f, prefix)...) } - val := getActualStructType(f) - if val == nil { - // no struct return nothing to complete + f = actualReflectValue(f) + // we only support structs + if f.Kind() != reflect.Struct { return suggestions } - f = *val var anonymous []formatSuggestion // loop over all field names @@ -1081,7 +1103,7 @@ func getStructFields(f reflect.Value, prefix string) []formatSuggestion { kind = field.Type.Elem().Kind() } // when we have a nested struct do not append braces instead append a dot - if kind == reflect.Struct { + if kind == reflect.Struct || kind == reflect.Map { suffix = "." } // if field is anonymous add the child fields as well @@ -1114,10 +1136,16 @@ func getMethodNames(f reflect.Value, prefix string) []formatSuggestion { if method.Func.Type().NumOut() != 1 { continue } + // when we have a nested struct do not append braces instead append a dot + kind := method.Func.Type().Out(0).Kind() + suffix := "}}" + if kind == reflect.Struct || kind == reflect.Map { + suffix = "." + } fname := method.Name if strings.HasPrefix(fname, prefix) { // add method name with closing braces - suggestions = append(suggestions, formatSuggestion{fieldname: fname, suffix: "}}"}) + suggestions = append(suggestions, formatSuggestion{fieldname: fname, suffix: suffix}) } } return suggestions diff --git a/cmd/podman/common/completion_test.go b/cmd/podman/common/completion_test.go index 34ec9b6a5..ae23b02e2 100644 --- a/cmd/podman/common/completion_test.go +++ b/cmd/podman/common/completion_test.go @@ -14,7 +14,14 @@ type Car struct { HP *int Displacement int } - Extras map[string]string + Extras map[string]Extra + // also ensure it will work with pointers + Extras2 map[string]*Extra +} + +type Extra struct { + Name1 string + Name2 string } type Anonymous struct { @@ -52,6 +59,10 @@ func (c Car) TwoOut() (string, string) { return "", "" } +func (c Car) Struct() Car { + return Car{} +} + func TestAutocompleteFormat(t *testing.T) { testStruct := struct { Name string @@ -63,7 +74,6 @@ func TestAutocompleteFormat(t *testing.T) { }{} testStruct.Car = &Car{} - testStruct.Car.Extras = map[string]string{"test": "1"} tests := []struct { name string @@ -118,7 +128,7 @@ func TestAutocompleteFormat(t *testing.T) { { "second level struct field name", "{{ .Car.", - []string{"{{ .Car.Color}}", "{{ .Car.Type}}", "{{ .Car.Brand}}", "{{ .Car.Stats.", "{{ .Car.Extras}}"}, + []string{"{{ .Car.Color}}", "{{ .Car.Struct.", "{{ .Car.Type}}", "{{ .Car.Brand}}", "{{ .Car.Stats.", "{{ .Car.Extras.", "{{ .Car.Extras2."}, }, { "second level struct field name", @@ -128,7 +138,7 @@ func TestAutocompleteFormat(t *testing.T) { { "second level nil struct field name", "{{ .Car2.", - []string{"{{ .Car2.Color}}", "{{ .Car2.Type}}", "{{ .Car2.Brand}}", "{{ .Car2.Stats.", "{{ .Car2.Extras}}"}, + []string{"{{ .Car2.Color}}", "{{ .Car2.Struct.", "{{ .Car2.Type}}", "{{ .Car2.Brand}}", "{{ .Car2.Stats.", "{{ .Car2.Extras.", "{{ .Car2.Extras2."}, }, { "three level struct field name", @@ -155,21 +165,37 @@ func TestAutocompleteFormat(t *testing.T) { "{{ .Car.Extras.", []string{}, }, + { + "map values work", + "{{ .Car.Extras.somekey.", + []string{"{{ .Car.Extras.somekey.Name1}}", "{{ .Car.Extras.somekey.Name2}}"}, + }, + { + "map values work with ptr", + "{{ .Car.Extras2.somekey.", + []string{"{{ .Car.Extras2.somekey.Name1}}", "{{ .Car.Extras2.somekey.Name2}}"}, + }, { "two variables struct field name", "{{ .Car.Brand }} {{ .Car.", - []string{"{{ .Car.Brand }} {{ .Car.Color}}", "{{ .Car.Brand }} {{ .Car.Type}}", "{{ .Car.Brand }} {{ .Car.Brand}}", - "{{ .Car.Brand }} {{ .Car.Stats.", "{{ .Car.Brand }} {{ .Car.Extras}}"}, + []string{"{{ .Car.Brand }} {{ .Car.Color}}", "{{ .Car.Brand }} {{ .Car.Struct.", "{{ .Car.Brand }} {{ .Car.Type}}", + "{{ .Car.Brand }} {{ .Car.Brand}}", "{{ .Car.Brand }} {{ .Car.Stats.", "{{ .Car.Brand }} {{ .Car.Extras.", + "{{ .Car.Brand }} {{ .Car.Extras2."}, }, { "only dot without variable", ".", nil, }, + { + "access embedded nil struct field", + "{{.Hello.", + []string{}, + }, } for _, test := range tests { - completion, directive := common.AutocompleteFormat(testStruct)(nil, nil, test.toComplete) + completion, directive := common.AutocompleteFormat(&testStruct)(nil, nil, test.toComplete) // directive should always be greater than ShellCompDirectiveNoFileComp assert.GreaterOrEqual(t, directive, cobra.ShellCompDirectiveNoFileComp, "unexpected ShellCompDirective") assert.Equal(t, test.expected, completion, test.name) -- cgit v1.2.3-54-g00ecf From 3b0844f990c6b7de708eeec1f8dfdc7e7d4611bd Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 16 May 2022 21:01:52 +0200 Subject: shell completion --format: add help to function with args From a template users POV it is not importent when they use a struct field or method. They only notice the difference when the function requires arguments. So lets be nice and let the user know that this method requires arguments via the help text. This is how it now looks like when the completion descriptions are enabled on bash: ``` $ bin/podman ps --format {{.Created.A {{.Created.AddDate (This is a function and requires 3 arguments) {{.Created.After (This is a function and requires 1 argument) {{.Created.Add (This is a function and requires 1 argument) {{.Created.AppendFormat (This is a function and requires 2 arguments) ``` Signed-off-by: Paul Holzinger --- cmd/podman/common/completion.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'cmd') diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 32dd896c0..ce26c1678 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "reflect" + "strconv" "strings" "github.com/containers/common/libnetwork/types" @@ -1142,6 +1143,20 @@ func getMethodNames(f reflect.Value, prefix string) []formatSuggestion { if kind == reflect.Struct || kind == reflect.Map { suffix = "." } + // From a template users POV it is not importent when the use a struct field or method. + // They only notice the difference when the function requires arguments. + // So lets be nice and let the user know that this method requires arguments via the help text. + // Note since this is actually a method on a type the first argument is always fix so we should skip it. + num := method.Func.Type().NumIn() - 1 + if num > 0 { + // everything after tab will the completion scripts show as help when enabled + // overwrite the suffix because it expects the args + suffix = "\tThis is a function and requires " + strconv.Itoa(num) + " argument" + if num > 1 { + // add plural s + suffix += "s" + } + } fname := method.Name if strings.HasPrefix(fname, prefix) { // add method name with closing braces -- cgit v1.2.3-54-g00ecf