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.

Sign up for GitHub

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

Document boolean flags by presence #283

Merged
merged 1 commit into from
Jul 27, 2019
Merged

Document boolean flags by presence #283

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions conventions/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ In this case `foo` is positional, and refers to the service to create.
example, `--image=img.repo/asdf` in Knative Serving sets
`spec.template.containers[0].image`

* Flags that control a boolean behavior (eg. generate a name or not) are
specified by their presence. The default happens when the flag is not present,
and adding the flag marks the user's desire for the non-default thing. When
the flag *disables* a default behavior which is to do something, it should
start with `no` (eg. `--no-generate-name` when the default is to generate a
name).

Copy link
Contributor

@rhuss rhuss Jul 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, both variations (with and without --no) should be present in this case, regardless of the default value (using the variant that corresponds to the default value should be possible, too). In this example, e.g. --generate-name and --no-generate-name should be both available all the time (corresponding --generate-name=true and --generate-name=false).

The reason why I prefer this variant is:

  • There is no ambiguity what should be used as boolean value ("true", "yes" and "on" are commonly used as such values), where "true" is probably mostly accustomed for programmers (maybe not so much users of kn ?)
  • It's shorter without specifying the value
  • It stands in the tradition of GNU getopts (sometimes you see the variation --with- and --no- for true and false)
  • Many boolean options are modelled like this in Unix (they are called 'flags' in this case. Eg.: ls -l)
  • We already use that style e.g. for kn --help (or would we like to switch to kn --help=true ?). Other examples: kn service create --force or kn service create --async would have change to kn service create --force=true and kn service create --async=true (or maybe in the latter case --sync=false)
  • For consistencies reasons we would always have to require options with values then, which might lead to an unnecessarily bloated UI (because you can't use simple flags anymore).

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, both variations (with and without --no) should be present in this case, regardless of the default value (using the variant that corresponds to the default value should be possible, too).

I have the same feeling.

Copy link

Choose a reason for hiding this comment

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

just noticed your statement here:

We already use that style e.g. for kn --help (or would we like to switch to kn --help=true ?). Other examples: kn service create --force or kn service create --async would have change to kn service create --force=true and kn service create --async=true (or maybe in the latter case --sync=false)

--help continues to work as cobra defaults it to mean true. so full form --flag=false is only necessary when turning things off.

Copy link
Contributor

@rhuss rhuss Jul 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@cppforlife Have you tried kn --help=false ? My bad, it works as expected (no arg == help). You can use kn service list --help=false, too.

#### Output

Commands that output information should support `--output` with a shorthand of
Expand Down