-
Notifications
You must be signed in to change notification settings - Fork 363
implement CopyToGoFlagSet #330
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
Conversation
d350035 to
7849f50
Compare
| usage += " (DEPRECATED: " + flag.Deprecated + ")" | ||
| } | ||
|
|
||
| switch value := flag.Value.(type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit fragile; how do we ensure it keeps working when new value types are added to the project (as has been done a few times since this PR was opened, e.g. #348, and will probably be done again, e.g. #359)? Is there a better way to register these things with flag without having to know the exact type of the thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This list only needs to include types which have a counterpart in the standard flag package. Everything else is handled through the generic default case. In other words, the list is complete until the standard package gets extended, and even then it's not breaking.
|
I'll resolve the merge conflict soonish... a bit busy right now (Kubernetes code freeze is looming). 😅 Thanks for taking a look at pflag and checking issues/PRs! |
7849f50 to
1de06c0
Compare
This is useful for programs which want to define some flags with pflag (for example, in external packages) but still need to use Go flag command line parsing to preserve backward compatibility with previous releases, in particular support for single-dash flags. Without this in pflag, such tools have to resort to copying via the public API, which leads to less useful help messages (type of basic values will be unknown).
1de06c0 to
44aa4aa
Compare
|
I rebased (conflict with other new method added at the same place, so easy to resolve), please merge now. |
|
I know I missed 1.0.7 (sorry for the delay!), but perhaps a 1.0.8 can follow soon? 🙏 |
|
Yeah, I don't see why not. Only caveat is that while I'm on parental leave (until Feb next year) my mental space to sit down and do proper work on this project (or anything, really 😅) is sporadic and irregular, so I'm not going to make any promises on a timeline. |
This is useful for programs which want to define some flags with pflag (for
example, in external packages) but still need to use Go flag command line
parsing to preserve backward compatibility with previous releases.
Without this in pflag, such tools have to resort to copying via the public
API, which leads to less useful help messages (type of basic values
will be unknown).
Fixes: #329