Skip to content

Parser config option to allow user control of -h #282

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rjp
Copy link

@rjp rjp commented Apr 2, 2025

Because I'm often wanting to use -h for "hostname", etc., the claiming of it by go-arg is annoying. This PR adds a config option named IgnoreShortHelp which prevents go-arg from claiming -h. --help is still left for go-arg to handle.

Because I'm often wanting to use `-h` for "hostname", etc., the claiming
of it by `go-arg` is annoying.  This PR adds a config option named
`IgnoreShortHelp` which prevents `go-arg` from claiming `-h`.  `--help`
is still left for `go-arg` to handle.
@alexflint
Copy link
Owner

Thanks for this @rjp. Would you be open to changing this so that the config option is

Help []string // a list of short or long command line options that cause go-arg to print usage information

where each element of the slice is expected to be a string like "--help", "-h", etc?

If config.Help is nil then it should default to []string{"--help", "-h"}. If config.Help is the empty slice (distinct from nil) then it should switch off the help option entirely.

In your case you would set it to []string{"--help"} and then there would be no short -h claimed by go-arg.

@rjp
Copy link
Author

rjp commented Apr 2, 2025

Yeah, good idea, I'll get on that later

Instead of `IgnoreShortHelp`, there's now `Help` which is a slice of
strings that will be treated as help options.  If the slice is `nil`,
it'll default to `[]string{"-h", "--help"}` to emulate the existing
behaviour.  Otherwise only options in the slice will activate "help".
parse.go Outdated
@@ -519,11 +518,12 @@ func cmdFromStruct(name string, dest path, t reflect.Type, envPrefix string) (*c
func (p *Parser) Parse(args []string) error {
err := p.process(args)
if err != nil {
if errors.Is(err, ErrHelp) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm mistaken, this won't do what the loop below previously did. The issue is that if you have, say, a program with a single flag:

var args struct {
  S string
}

And the user writes on the command line

./program --s --help

Then the program will now complain with something like "--s requires a value". But in this case we really want to print help output, not the error about --s.

I don't think the errors.Is check here will change that because the library will just bail when it encounters the first error (correct me if you already tested this and I'm remembering wrong).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seem to have tested everything but that. You're right, it doesn't trigger the help. I'll have a think.

Copy link
Owner

@alexflint alexflint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together @rjp. I'm afraid there is a bit more work to do because the help and usage information should reflect the help options configured in this way. Not sure if you have time/inclination to look at that - no problem if not, I should be able to look into it in the next few days.

Also for reason this PR is showing a diff versus the previous version of this feature. Any idea what's up with that? Again, no problem if you don't have time, I can pull the branch over and work on it at my end if needed.

rjp added 4 commits April 4, 2025 16:20
Correctly handles the `--s --help` test (should display help, not
complain about a missing argument for `--s`.)
+ Test that one long and one short option in `config.Help` works fine.
+ More will confuse things.
@rjp
Copy link
Author

rjp commented Apr 4, 2025

Thanks for putting this together @rjp. I'm afraid there is a bit more work to do because the help and usage information should reflect the help options configured in this way.

Latest push addresses this but will only work properly for one short and one long option in config.Help. Which I think is a reasonable restriction? If it is, might be worth enforcing that in NewParser and, maybe, moving from config.Help []string to config.ShortHelp string, config.LongHelp string which makes it obvious that you're only "allowed" one short + one long.

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

Successfully merging this pull request may close these issues.

2 participants