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

Show boolean value type default in flag formatting. #317

Closed
wants to merge 1 commit into from
Closed

Show boolean value type default in flag formatting. #317

wants to merge 1 commit into from

Conversation

geekodour
Copy link

@geekodour geekodour commented May 3, 2020

Fixes: #243

Unsure if contradicts with kingpins design to have the implicit --no-name flags.

cc: @autarch @alecthomas

Signed-off-by: Hrishikesh Barman hrishikeshbman@gmail.com

Signed-off-by: Hrishikesh Barman <hrishikeshbman@gmail.com>
@autarch
Copy link
Contributor

autarch commented May 3, 2020

Thanks for working on this, @geekodour.

My thinking is that this isn't the right way to do this. Showing output like "--foo=false" seems quite confusing. If someone actually passes that they'll get an error:

package main

import (
	"fmt"

	"github.com/alecthomas/kingpin"
)

var boolFlag = kingpin.Flag("foo", "Foo").Default("true").Bool()

func main() {
	kingpin.Parse()
	fmt.Printf("%v\n", *boolFlag)
}
$> go run ./foo/main.go --foo=false
main: error: unexpected false, try --help
exit status 1

I think the original suggestion of showing --foo or --no-foo depending on the flag's default value was the right approach. It'd also be nice to change the help output to clarify this, so we'd get something like this:

--no-foo    Disable the foo feature. The default is to enable this feature.
--bar       Enable the bar feature. The default is to disable this feature.

The exact wording of that needs some work, since enable/disable may not work for every flag.

@alecthomas
Copy link
Owner

Yes, thanks for the PR, but I agree with @autarch. Whatever the help displays should be usable by the user.

@alecthomas alecthomas closed this May 14, 2020
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.

Detect default true values for boolean flags and display the disable syntax
3 participants