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

ffcli.DefaultUsageFunc: Match output of FlagSet.PrintDefaults #105

Closed
abhinav opened this issue May 3, 2023 · 6 comments
Closed

ffcli.DefaultUsageFunc: Match output of FlagSet.PrintDefaults #105

abhinav opened this issue May 3, 2023 · 6 comments

Comments

@abhinav
Copy link
Contributor

abhinav commented May 3, 2023

This issue is to discuss a possible change in behavior in advance of a PR.
Currently, -h on a ffcli.Command prints a FLAGS section like the following:

  -b=false  bool
  -d 0s     time.Duration
  -f 0      float64
  -i 0      int
  -s ...    string
  -x ...    collection of strings (repeatable)

Things to note:

  • Flags use the placeholder "..." instead of "string", "int", or something more helpful.
  • Flags are always on the same line as the flag usage. Usage text has to shift right if a flag with a different length is added.
  • Defaults are printed in the help message instead of being put at the end of the usage text, which adds another case when usage text has to shift right.
  • Usage text shifting right adds uncertainty to how much horizontal space is available to a help message for a flag.
  • Doesn't handle newlines in the flag usage text; it'll wrap to the next line instead of being indented to match.

FlagSet.PrintDefaults, on the other hand, prints:

  -b        bool
  -d duration
            time.Duration
  -f float
            float64
  -i int
            int
  -s string
            string
  -x value
            collection of strings (repeatable)

Only short boolean flags are on the same line -- if they fit in that space. All other usage text is on the next line and indented.

Not visible in this example:

  • non-zero defaults are added at the end in the form (default: foo)

  • newlines in usage text are adjusted to match indentation of the rest of the text

  • if the help text wraps a word with backticks, for example:

    path to input `file`
    

    then that word is treated as the placeholder in the help:

    -f file
              path to input file
    

It's not terribly difficult to change DefaultUsageFunc to match this behavior, but it is a change in behavior.
Are you open to such a change or would you prefer that users write a custom UsageFunc for this?

abhinav added a commit to abhinav/ff that referenced this issue May 3, 2023
(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.
abhinav added a commit to abhinav/ff that referenced this issue May 6, 2023
(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 pushed a commit that referenced this issue May 9, 2023
(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:

    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 #105, if you're open to it,
would change more of the output.
@peterbourgon
Copy link
Owner

Guess this is close-able?

@abhinav
Copy link
Contributor Author

abhinav commented May 9, 2023

Note quite ready to close. #106 was an incremental less-controversial change.

I wanted your opinion before I made the changes proposed in this issue.
In short, there are a fair number of differences between the output of ffcli.DefaultUsageFunc and FlagSet.PrintDefaults.
I'm happy to make DefaultUsageFunc match FlagSet.PrintDefaults, but I want to know if you're okay with that change.

@peterbourgon
Copy link
Owner

peterbourgon commented May 9, 2023

I'm happy to make changes like the one that was merged, but I definitely don't want ff.DefaultUsageFunc to match flag.FlagSet.PrintDefaults — I consider the latter to be pretty user-unfriendly, overall.

edit: Sorry, I failed to parse the OP. No, I think it's important to keep each flag on a single line, basically.

@abhinav
Copy link
Contributor Author

abhinav commented May 9, 2023

That's a completely reasonable position to hold.
Just for clarity, I'll enumerate the differences between DefaultUsageFunc and PrintDefaults:

  • flag and help are on separate lines if flag is longer than a couple characters
  • help can be multi-line and its indentation is adjusted accordingly
  • flag value in help (-foo bar) is the type of the value (-foo sting) or the name specified with back-ticks (per ffcli.DefaultUsageFunc: Support flag help placeholders #106)
  • default value is at the end of the help text (e.g. -f file path to output file (default: "foo"))

There are a couple independent changes that could be made here:

  1. Split flag and help across two lines if flag is long — you've indicated this is undesirable
  2. Support multi-line flag help message — I suspect this would be acceptable
  3. Move default value to the end of the help text; this will have the effect of only printing flag type (-f string) or back-ticked value name (-f file) instead of default value in help (-f out.txt)

I'm willing to make all or a subset of these changes depending on what you like or dislike about flag.PrintDefaults.
I'm also happy to close this issue if you feel that further alignment with flag.PrintDefaults is a poorer user-experience.

Lastly, another alternative is: DefaultUsageFunc retains its current behavior, and a second alternative is added (ffcli.StdUsageFunc?) that matches the behavior of flag.PrintDefaults. If we do this, it will likely make sense for a ffcli.Command with a nil usage func to inherit it from its parent so that a program specifies the override once only.

@peterbourgon
Copy link
Owner

Split flag and help across two lines if flag is long — you've indicated this is undesirable

👍

Support multi-line flag help message — I suspect this would be acceptable

I'd say this is out of scope for DefaultUsageFunc, and something you should solve with your own implementation. (IMO multi-line flag help messages are a code smell, so to speak, and should be fixed, though I understand reasonable people may disagree.)

Move default value to the end of the help text; this will have the effect of only printing flag type (-f string) or back-ticked value name (-f file) instead of default value in help (-f out.txt)

I don't have strong opinions here.

@abhinav
Copy link
Contributor Author

abhinav commented May 9, 2023

Move default value to the end of the help text; this will have the effect of only printing flag type (-f string) or back-ticked >> value name (-f file) instead of default value in help (-f out.txt)

I don't have strong opinions here.

Likewise.

Honestly, the only one I considered actionable is multi-line help, but I'm okay with calling that not ffcli's problem.
Command.Usage provides the ability to plug in arbitrary behavior so DefaultUsageFunc doesn't have to do it all.
I will close the issue since I don't see anything else actionable.

Thanks for the discussion!

@abhinav abhinav closed this as completed May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants