Add support for --no-* versions of boolean flags directly in mflag itself by tianon · Pull Request #5764 · moby/moby

tianon · 2014-05-13T04:12:55Z

So, this is building upon #5760 just to better illustrate what I was trying to describe in #5545.

I don't necessarily expect it to be merged, but figured it wouldn't hurt to actually implement it just to show exactly what it is that I mean.

Basically, for boolean flags, this deprecates the --flag=true|false syntax (while still supporting it, but only for non---no-* versions to keep it simple and easy).

Here's some example output:

$ ./bundles/0.11.1-dev/dynbinary/docker --help
Usage of ./bundles/0.11.1-dev/dynbinary/docker:
  --api-enable-cors, --no-api-enable-cors (=false)    Enable CORS headers in the remote API
  -b, --bridge=""                                     Attach containers to a pre-existing network bridge
                                                        use 'none' to disable container networking
  --bip=""                                            Use this CIDR notation address for the network bridge's IP, not compatible with -b
  -d, --daemon, --no-daemon (=false)                  Enable daemon mode
  -D, --debug, --no-debug (=false)                    Enable debug mode
  --dns=[]                                            Force docker to use specific DNS servers
  --dns-search=[]                                     Force Docker to use specific DNS search domains
  -e, --exec-driver="native"                          Force the docker runtime to use a specific exec driver
  -g, --graph="/var/lib/docker"                       Path to use as the root of the docker runtime
  -G, --group="docker"                                Group to assign the unix socket specified by -H when running in daemon mode
                                                        use '' (the empty string) to disable setting of a group
  -H, --host=[]                                       The socket(s) to bind to in daemon mode
                                                        specified using one or more tcp://host:port, unix:///path/to/socket, fd://* or fd://socketfd.
  --icc, --no-icc (=true)                             Enable inter-container communication
  --ip="0.0.0.0"                                      Default IP address to use when binding container ports
  --ip-forward, --no-ip-forward (=true)               Enable net.ipv4.ip_forward
  --iptables, --no-iptables (=true)                   Enable Docker's addition of iptables rules
  --mtu=0                                             Set the containers network MTU
                                                        if no value is provided: default to the default route MTU or 1500 if no default route is available
  -p, --pidfile="/var/run/docker.pid"                 Path to use for daemon PID file
  -r, --restart, --no-restart (=true)                 Restart previously running containers
  -s, --storage-driver=""                             Force the docker runtime to use a specific storage driver
  --selinux-enabled, --no-selinux-enabled (=false)    Enable selinux support
  --tls, --no-tls (=false)                            Use TLS; implied by tls-verify flags
  --tlscacert="/home/tianon/.docker/ca.pem"           Trust only remotes providing a certificate signed by the CA given here
  --tlscert="/home/tianon/.docker/cert.pem"           Path to TLS certificate file
  --tlskey="/home/tianon/.docker/key.pem"             Path to TLS key file
  --tlsverify, --no-tlsverify (=false)                Use TLS and verify the remote (daemon: verify client, client: verify daemon)
  -v, --version, --no-version (=false)                Print version information and quit
$ ./bundles/0.11.1-dev/dynbinary/docker run --help

Usage: docker run [OPTIONS] IMAGE [COMMAND] [ARG...]

Run a command in a new container

  -a, --attach=[]                                 Attach to stdin, stdout or stderr.
  -c, --cpu-shares=0                              CPU shares (relative weight)
  --cidfile=""                                    Write the container ID to the file
  -d, --detach, --no-detach (=false)              Detached mode: Run container in the background, print new container id
  --dns=[]                                        Set custom dns servers
  --dns-search=[]                                 Set custom dns search domains
  -e, --env=[]                                    Set environment variables
  --entrypoint=""                                 Overwrite the default entrypoint of the image
  --env-file=[]                                   Read in a line delimited file of ENV variables
  --expose=[]                                     Expose a port from the container without publishing it to your host
  -h, --hostname=""                               Container host name
  -i, --interactive, --no-interactive (=false)    Keep stdin open even if not attached
  --link=[]                                       Add link to another container (name:alias)
  --lxc-conf=[]                                   (lxc exec-driver only) Add custom lxc options --lxc-conf="lxc.cgroup.cpuset.cpus = 0,1"
  -m, --memory=""                                 Memory limit (format: <number><optional unit>, where unit = b, k, m or g)
  --name=""                                       Assign a name to the container
  --net="bridge"                                  Set the Network mode for the container
                                                    'bridge': creates a new network stack for the container on the docker bridge
                                                    'none': no networking for this container
                                                    'container:<name|id>': reuses another container network stack
                                                    'host': use the host network stack inside the contaner
  -p, --publish=[]                                Publish a container's port to the host
                                                    format: ip:hostPort:containerPort | ip::containerPort | hostPort:containerPort
                                                    (use 'docker port' to see the actual mapping)
  -P, --publish-all, --no-publish-all (=false)    Publish all exposed ports to the host interfaces
  --privileged, --no-privileged (=false)          Give extended privileges to this container
  --rm, --no-rm (=false)                          Automatically remove the container when it exits (incompatible with -d)
  --sig-proxy, --no-sig-proxy (=true)             Proxify all received signal to the process (even in non-tty mode)
  -t, --tty, --no-tty (=false)                    Allocate a pseudo-tty
  -u, --user=""                                   Username or UID
  -v, --volume=[]                                 Bind mount a volume (e.g. from the host: -v /host:/container, from docker: -v /container)
  --volumes-from=[]                               Mount volumes from the specified container(s)
  -w, --workdir=""                                Working directory inside the container

Again, this mostly just here to clarify my thoughts, so I won't be sad if it doesn't get reviewed and tweaked if it's unpopular. :)

Docker-DCO-1.1-Signed-off-by: Victor Vieux <[email protected]> (github: vieux)

jamtur01 · 2014-05-13T04:44:20Z

Seems reasonable to me.

cyphar · 2014-05-13T07:25:21Z

Kind of like -Wno-* gcc flags? Makes sense to me.

cyphar · 2014-05-13T07:26:02Z

Typo: Don't you mean Use a?

/me puts on American voice and chants "USA, USA, USA". mmmm, maybe thats just a tad aussie :)

Heh, this was stolen directly from @vieux, since I've included his PR here to make these changes simpler. Also, he hasn't cleared any of this in theory, let alone practice, so I'm not too keen on doing too much more work on it unless it gets reviewed and general acceptance. :)

yes totally my fault!

SvenDowideit · 2014-05-13T08:58:55Z

yes, I was thinking about this way too,

small nit: -D, --debug, --no-debug (=false) what was the default? --no-debug=false?

clearly we'd need to tweak the doc output to say that --no-debug is the default, and never mention =true/false ever again :)

the main thing i care deeply about is the double-negative thing, it bit us and our users so many times in TWiki/Foswiki its scary. nodocument=false

As to the PR itself, whats easier to use and get right --no-interactive or --interactive=false?

I can grep for '--interactive' or \-interactive either way. I assume that the API will have a field called interactive.

nope, still no strong feelings either way :)

thaJeztah · 2014-05-13T19:35:54Z

--selinux-enabled, --no-selinux-enabled are really awkward, some ideas;

--selinux / --no-selinux to be in line with the other flags
--enable-selinux / --disable-selinux

Will be a breaking change, but thought I should mention it now that these are discussed

Docker-DCO-1.1-Signed-off-by: Victor Vieux <[email protected]> (github: vieux)

cyphar · 2014-05-14T01:38:31Z

@thaJeztah +1. Double negative arguments are very confusing. I vote for 1, since it makes the arguments consistent.

thaJeztah · 2014-05-14T06:34:45Z

@cyphar Wasn't 100% convinced myself on just --selinux as it's kinda non-descriptive.

note api-enable-cors may need some attention as well

…self Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)

Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)

tianon · 2014-05-14T19:53:45Z

Thanks for the typo fix @vieux ❤️ (I've rebased to include it)

Other reviewers - please see @vieux's original in #5760 before getting too deep in this one. The real meat here is specifically in 24beb29 (and then a3f1a2d is just removing the hacks to explicitly add #no-* flags since the mflag package supports them directly in the previous commit).

crosbymichael · 2014-08-01T23:43:23Z

-1

crosbymichael · 2014-08-01T23:44:09Z

Lets not do this and blow up the cli with a bunch of flags for some opposite case.

tianon · 2014-08-01T23:44:28Z

Fair enough. :P

no --no- flags

80689a1

Docker-DCO-1.1-Signed-off-by: Victor Vieux <[email protected]> (github: vieux)

tianon added the /project/doc label May 13, 2014

cyphar reviewed May 13, 2014
View reviewed changes

update man

efc64b8

Docker-DCO-1.1-Signed-off-by: Victor Vieux <[email protected]> (github: vieux)

tianon mentioned this pull request May 14, 2014

Invert flags #5760

Closed

tianon added 2 commits May 14, 2014 13:49

Add support for --no-* versions of boolean flags directly in mflag it…

24beb29

…self Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)

Update commands.go to remove the "#no-*" hacks

a3f1a2d

Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)

vieux closed this Aug 1, 2014

tianon deleted the flags-no-prefix branch August 1, 2014 23:44

thaJeztah mentioned this pull request Nov 29, 2014

Docs: Remove defaults for (boolean)flags/options that expect no value #9406

Closed

thaJeztah mentioned this pull request Dec 20, 2015

Docker help isn't as helpful anymore w.r.t. options #18797

Open

MHBauer mentioned this pull request Apr 27, 2016

docker ps -h: --no-trunc incorrectly documented #5545

Closed

Conversation

tianon commented May 13, 2014

Uh oh!

jamtur01 commented May 13, 2014

Uh oh!

cyphar commented May 13, 2014

Uh oh!

cyphar May 13, 2014

Choose a reason for hiding this comment

Uh oh!

SvenDowideit May 13, 2014

Choose a reason for hiding this comment

Uh oh!

tianon May 13, 2014

Choose a reason for hiding this comment

Uh oh!

vieux May 13, 2014

Choose a reason for hiding this comment

Uh oh!

SvenDowideit commented May 13, 2014

Uh oh!

thaJeztah commented May 13, 2014

Uh oh!

cyphar commented May 14, 2014

Uh oh!

thaJeztah commented May 14, 2014

Uh oh!

tianon commented May 14, 2014

Uh oh!

crosbymichael commented Aug 1, 2014

Uh oh!

crosbymichael commented Aug 1, 2014

Uh oh!

tianon commented Aug 1, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants