-
Notifications
You must be signed in to change notification settings - Fork 59
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
ffcli.DefaultUsageFunc: Support flag help placeholders #106
Conversation
I'm open to allowing users to specify placeholder text other than |
// | ||
// Otherwise, it's an educated guess for a placeholder, | ||
// or an empty string if one couldn't be determined. | ||
argname, usage := flag.UnquoteUsage(f) |
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 appears to be the major change, is there some prior art that uses UnquoteUsage
in this way?
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.
Apparently flag.FlagSet lol 😮💨
RE: Prior art: this is how the std flag package does it. This is documented mostly in flag.PrintDefaults:
This PR isn't suggesting a completely custom behavior; just to carry over something that the std flag package already provides. I'll agree that this feature isn't super obviously documented in std flag, but users of flag who know about it will likely expect it from ffcli as well. (That's what brought me here! 😄 ) About whether this is the best way to encode this information: So right now, this is just suggesting matching the behavior of what flag.PrintDefaults already provides. |
Oh, cool! Somehow I didn't know about that. |
(This is an arguably uncontroversial part of the proposal in peterbourgon#105. Its's a new feature, not a significant change in behavior.) Currently, ffcli.DefaultUsageFunc prints "..." for any flag that does not have a default value specified. This produces less-than-effective help from DefaultUsageFunc. This change retains the behavior of printing the default value as-is, but if a default value is not provided, it allows users to provide placeholder text by wrapping a word inside the help text for a flag in backticks. For example, given the following: fset.String("c", "" /* default */, "path to `config` file") We'll get: -c config path to config file This matches the behavior of FlagSet.PrintDefaults, and indeed it relies on the same flag.UnquoteUsage machinery for this. This also has the nice side-effect of making a reasonable guess at an alternative placeholder text instead of "...". For example: fset.Int("n", "" /* default */, "number of items") // Before: -n ... number of items // Now: -n int number of items Note that as implemented right now, the user supplied placeholder will be used only if a non-zero default value was not supplied. This was an attempt to retain as much of the existing behavior. The proposal in peterbourgon#105, if you're open to it, would change more of the output.
@peterbourgon Obviously no rush, but FYI, the suggested change has been made. |
Thanks for the poke. |
(This is an arguably uncontroversial part of the proposal in #105.
Its's a new feature, not a significant change in behavior.)
Currently, ffcli.DefaultUsageFunc prints "..." for any flag
that does not have a default value specified.
This produces less-than-effective help from DefaultUsageFunc.
This change retains the behavior of printing the default value as-is,
but if a default value is not provided,
it allows users to provide placeholder text
by wrapping a word inside the help text for a flag in backticks.
For example, given the following:
We'll get:
This matches the behavior of FlagSet.PrintDefaults,
and indeed it relies on the same flag.UnquoteUsage machinery for this.
This also has the nice side-effect of making a reasonable guess
at an alternative placeholder text instead of "...". For example:
Note that as implemented right now, the user supplied placeholder will
be used only if a non-zero default value was not supplied.
This was an attempt to retain as much of the existing behavior.
The proposal in #105, if you're open to it,
would change more of the output.