Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StringSlice CLI should replace default values #95

Closed
Mzack9999 opened this issue Nov 4, 2022 · 3 comments · Fixed by #97 or #102
Closed

StringSlice CLI should replace default values #95

Mzack9999 opened this issue Nov 4, 2022 · 3 comments · Fixed by #97 or #102
Assignees
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@Mzack9999
Copy link
Member

Description

StringSlice flags by default append values passed via CLI to default values instead of replacing them.

How to reproduce

Definition:

flagSet.StringSliceVarP(&test, "test", "t", []string{"a"}, "Test", goflags.CommaSeparatedStringSliceOptions),

CLI Command:

go run . -t b

Current Result

["a","b"]

Wanted Result

["b"]
@Mzack9999 Mzack9999 added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Nov 4, 2022
@ehsandeep ehsandeep added the Priority: Medium This issue may be useful, and needs some attention. label Nov 4, 2022
@edoardottt
Copy link
Contributor

I've tried to test this and what I get is different. Since in this loop ((flagSet *FlagSet) StringSliceVarP):

for _, defaultItem := range defaultValue {
		_ = field.Set(defaultItem)
		values, _ := ToStringSlice(defaultItem, options)
		for _, value := range values {
			_ = field.Set(value)
		}
	}

the field is set twice, the default value will be always duplicated.

Examples:

  • Using examples/basic/main.go with
flagSet.StringSliceVarP(&testOptions.Email, "email", "e", []string{"A"}, "email of the user", goflags.CommaSeparatedStringSliceOptions),

and printing the result (fmt.Println(testOptions.Email)) I get:

go run examples/basic/main.go -e c
> ["A", "A", "c"]
  • Same if the default value is []string{"A,A,A"}, I get
go run examples/basic/main.go -e c
> ["A", "A", "A", "A", "A", "A", "c"]

@edoardottt
Copy link
Contributor

Moreover, I suspect that the problem is not only in StringSlice, but all the functions setting the default value, since also PortVarP is affected.

@Mzack9999
Copy link
Member Author

@edoardottt nice catch! I'm keeping the issue open as the command line passed value should also override any default pre-existing value:

func TestSetDefaultValue(t *testing.T) {
	var data StringSlice
	flagSet := NewFlagSet()
	flagSet.StringSliceVar(&data, "test", []string{"A,A,A"}, "Default value for a test flag example", CommaSeparatedStringSliceOptions)
	flagSet.CommandLine.Parse([]string{"-test", "item1"})
	log.Fatal(data)
        // actual: ["A", "A", "A", "item1"] => wanted: ["item1"]
}

@Mzack9999 Mzack9999 reopened this Nov 11, 2022
@Mzack9999 Mzack9999 linked a pull request Dec 14, 2022 that will close this issue
@Mzack9999 Mzack9999 self-assigned this Dec 14, 2022
@ehsandeep ehsandeep added the Status: Completed Nothing further to be done with this issue. Awaiting to be closed. label Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
3 participants