search

spf13 / pflag

Drop-in replacement for Go's flag package, implementing POSIX/GNU-style --flags.
BSD 3-Clause "New" or "Revised" License
2.43k stars 348 forks source link

Generate a clearer panic message #381

Open roshanavand opened 1 year ago

roshanavand commented 1 year ago

This aims to fix this issue

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

marckhouzam commented 1 year ago

@roshanavand would you mind specifying the output before this change and after this change?

roshanavand commented 1 year ago

@marckhouzam Not at all. Assuming we want to add a new flag named myFlag with shorthand h to a flagset named myFlagSet: Before the message would read: unable to redefine 'h' shorthand in "myFlagSet" flagset: it's already used for "myFlag" flag Now it reads: unable to define 'h' shorthand for "help" flag: it's already used for "myFlag" flag

Considering the linked issue, the custom flag is set first, after that the help flag creation is initiated and results to that panic.

roshanavand commented 1 year ago

Especially in case of using h shorthand, the message is more vague for the user as they might not know that help with the same shorthand is being initiated later and causing that error. But I believe the new message is clearer in all the situations.