[Bugfix] Remove Options Duplications #293
Conversation
| @@ -224,6 +226,12 @@ func (c ConfigurationPrefixes) Parse(args ...string) (*Configuration, []Configur | |||
| return nil, nil, fmt.Errorf("option --%s is essential to the starters behavior and cannot be overwritten", targ) | |||
| } | |||
|
|
|||
| if _, ok := flags[arg]; ok { | |||
There was a problem hiding this comment.
It seems that this can be simplified to
if _, ok := flags[arg]; !ok {
flags[arg] = true
...
}
break
but this does not change the logic, it looks a bit easier to read, IMHO.
There was a problem hiding this comment.
It took me a while to understand what is going on here. It seems that you want to select only those args, which are in ConfigurationPrefixes, and only allow each one once. I cannot really judge if this is correct behaviour. What about multiple options like
--all.log.level=requests=trace --all.log.level=communication=trace
are these still allowed?
There was a problem hiding this comment.
-
usually, linter prefers if you exit if as fast as possible - in this case we check if option is defined and then exit. If no, proceed. It is better to keep smaller piece of code inside block.
-
In this case we only generate list of flags passed to Cobra, where each one can be parsed multiple times by Cobra itself. So your case is totally valid, you will get
[]string{"--log.level=requests=trace", "--log.level=communication=trace"}after parsint it by Cobra. Problem was only when you tried to add flag twice - it just ran into panic.
No description provided.