peterbourgon / ff

Flags-first package for configuration
Apache License 2.0
1.34k stars 59 forks source link

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

Closed abhinav closed 1 year ago

abhinav commented 1 year ago

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:

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:

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?

peterbourgon commented 1 year ago

Guess this is close-able?

abhinav commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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!