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

Preserve / improve consistency in CLI UI #8829

Closed
thaJeztah opened this Issue Oct 28, 2014 · 37 comments

Comments

Projects
None yet
10 participants
@thaJeztah
Copy link
Member

thaJeztah commented Oct 28, 2014

Summary:
There is no standardised "naming" for (sub)commands on the CLI (e.g. plural vs singular, create vs add), sometimes leading to inconsistencies.

Proposed solution:
A design document should be written, describing the (naming) conventions to follow for commands and the UX in general.


A number of new features are currently proposed via PRs for addition to Docker. These features add new commands to the CLI and I have noticed that the UI for those commands (and existing commands) is inconsistent.

While there still a chance to change those proposals before they are merged, I think it would be a good thing to review those PRs and reach consensus on what the UI should look like to be consistent.

I've created a (far from complete) overview of some existing and proposed commands;

Entity Action Command Notes
Container list docker ps
Device list - (not supported)
Host list docker hosts no sub command. see #8681
Image list docker images no sub command (ls)
Links list docker links list see #7468
Volume list docker volumes ls see #8484
Container delete docker rm (id)
Device delete docker device remove (id) see #8826
Host delete - (not supported?)
Image delete docker rmi (id)
Links delete docker links remove (id)
Volume delete docker volumes rm (id)
Container create docker create no "containers" before "create"
Device create docker device add note: singular "device", others are "plural"
Host create docker hosts create run is proposed as alternative
Link create docker links add
Volume create docker volumes add
  • The ls and list subcommands could be omitted dropped for volumes and links to be in line with the existing images command.
  • The subcommand for "delete" should either be "remove" or "rm" for all entities.
  • The subcommand for "create" could be renamed from "add" to "create" to be in line with the existing docker create command (not sure, I actually like "add")
  • All entities / top-level commands should be either plural or singular (rename device to devices?)

Now that the "links" (#7468), "volumes" (#8484), "hosts" (#8681) and "devices" (#8826) changes are still a proposal, I think the creators those proposals should discuss a consistent UI before they're implemented to keep things consistent.

For the existing commands I understand that most frequently used commands are deliberately left short for convenience, perhaps "long" versions (docker images rm?) could be added to make the UI more consistent.

(This is a follow up on a comment I placed here #8484 (comment))

update
Added the "hosts" proposal to the list and sorted the list a bit

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 28, 2014

/ping @shykes I think this is something for you 😄

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 29, 2014

Perhaps I should "spam" the creators of the linked issues as well (@erikh @cpuguy83 @bfirsh @imain).

To clarify, all are excellent proposals, just trying to keep a consistent UI here.

@bfirsh

This comment has been minimized.

Copy link
Contributor

bfirsh commented Oct 29, 2014

👍 This is really important. Thanks for aggregating these.

I think subcommands should follow the same verbs as the top-level container commands – create and rm, basically.

The one I've invented for hosts is list. I'm not particularly tied to it. ls also works, and is consistent with rm. Though I like that omitting the subcommand implies list. This maps well with how docker images already works. I expect at some point that'll turn into subcommands too and we'll have docker images rm, etc (with docker rmi being an alias to that).

This is part of my role as DX, so I'm happy to chase after the people writing these new features.

/cc @aanand

@bfirsh

This comment has been minimized.

Copy link
Contributor

bfirsh commented Oct 29, 2014

Though I'm not entirely sure I like how we're mixing short and long (create / rm). Perhaps we could have both: the long versions could be the primary ones (create, list, remove), then the short versions would also work as aliases (ls, rm).

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 29, 2014

@bfirsh thanks for picking this up, I didn't know who to "ping" 😄

I think having both short (ls) and long (list) subcommands makes sense; as is The suggestion to add full versions for the shortcut commands, like rmi.

Finding the "right" terms for all variations will need a bit of experimenting, for example, "create" or "add" might not make sense for all "entities".

Additionally, there's currently no "create" or "add" for Images; unless we regard "commit" as such.

The list is just a start, if you have suggestions for data to add, just let me know.

On a side note; @cpuguy83 pointed me to the remote API, because the same will apply there as for the CLI; Are you responsible for that as well, or should someone else be involved for that?

@bfirsh

This comment has been minimized.

Copy link
Contributor

bfirsh commented Oct 30, 2014

I propose this as the standard set of commands against objects:

Command Action
docker objects List
docker objects ls List
docker objects create Create
docker objects rm Remove
docker objects inspect Detail

If we want to also have the full versions of the commands, we can support list and remove. I'm not 100% sold on whether we need that.

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 30, 2014

I don't think the full versions of the commands are required, perhaps better not, to not clutter the interface.

Just to re-iterate / confirm; the commands are all plural? I know some people prefer singular in some cases, but for consistency, it will be plural.

Do we need the same list for the API?

Also, do we need something for update / modify?

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Oct 30, 2014

This is my preference for CRUD actions

Action Verb URL
List objectsGET/objects
Inspect objectGET/objects/id
Create objectPOST/objects
Delete objectDELETE/objects/id
Update objectPATCH/objects/id
Download objectGET/objects/id/export

Note that this is not the case for today's API, though it is exactly what I have setup for the API for the volumes command.

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 30, 2014

@cpuguy83 Yes, for the API that's the right approach.

Only thing I think should be different is the "Download"; from a RESTful view, I think the right approach would be to use the Accept: header to determin the output-format. As a fallback, we'd be able to use a file-extension. The table will then look like;

Action Verb URL Accept Extension
List objects GET /objects(.ext) application/json .json
Inspect object GET /objects/id(.ext) application/json .json
Download object GET /objects/id(.ext) application/x-gtar (*) .tar.gz / .tgz
Create object POST /objects(.ext) application/json .json
Delete object DELETE /objects/id(.ext) application/json .json
Update object PATCH /objects/id(.ext) application/json .json

(*) Looked this one up on Wikipedia, I'm not sure if there's an official mime-type registered for tar.gz

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 30, 2014

Note; there's already a ticket on inconsistencies of the API: #7671

@imain

This comment has been minimized.

Copy link
Contributor

imain commented Oct 30, 2014

We could certainly do docker devices add/rm. I don't like the idea of using 'create' because we're really not creating anything. We are just adding an existing device to the container. I think this is why we have different uses with add and create. I don't see them as inconsistent but as a consiquence of the english language. It's interesting that 'remove' is ok for both cases.

Actually create/destroy and add/remove would be more logical pairs.

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 30, 2014

@imain I see your point, hence my earlier comment;

Finding the "right" terms for all variations will need a bit of experimenting

I personally have no objection against expanding the set of possible subcommands on the CLI (I don't think this problem affects the remote API), as long as "we" try to keep them consistent and they don't confuse the user (thinking of the number of tickets related to save/load/export/import user errors).

The main reason for opening this ticket is exactly this; to get this discussion started and get everybody's point of view so that a usable and consistent set of commands can be established.

Thanks for "joining in", much appreciated!

@bfirsh

This comment has been minimized.

Copy link
Contributor

bfirsh commented Oct 31, 2014

To be clear: the main reasoning of this is for consistency. We don't necessarily need to use exactly the right language. We're using create and rm because docker create and docker rm exist.

We want users not to have to think when trying to remove a device. If they already use docker rm, they won't have to think or look up anything to type docker devices rm. They won't think about whether it's the correct use of the language, they'll just type what they're already used to typing.

I agree there is a balance here to make between consistency and using a term that correctly describes what is going on, but I'd argue that keeping the CRUD verbs consistent is important, though. E.g. devices create could be thought of as creating a link between a device and a container rather than creating the device itself.

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 31, 2014

could be thought of as creating a link between a device and a container rather than creating the device itself

Yes, that might work, still tricky (should it be called device-links, or be part of the 'links' or 'volumes' top command?)

Perhaps some other thought; if a command arises that needs to have both a create and an add subcommand ("create the device", then "add" / "attach" it). From that perspective, it could be useful to define both create and add.

@imain

This comment has been minimized.

Copy link
Contributor

imain commented Nov 4, 2014

So do we have any consensus or direction from this discussion? Should I update the device add/rm proposal to use 'devices add' and 'devices rm'? That seem reasonable?

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Nov 4, 2014

I'm +1 on that, but the final word is with @bfirsh because I'm not a maintainer 😄

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Nov 4, 2014

@imain are devices an object that docker will be managing or are they resources managed for singular container instances that you are adding a way to modify for an existing container?

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Nov 4, 2014

@imain hold on; I was probably sleeping while writing my previous comment. I'm +1 on plural devices, but the add vs create debate wasn't finished yet.

I don't want to hold up your PR too long, so hope that Ben is able to shed his light one more time (see my comment before on this subject) :)

/ping @bfirsh

@bfirsh

This comment has been minimized.

Copy link
Contributor

bfirsh commented Nov 4, 2014

@imain Hmmm. It's a tough one.

My concern is that people will try to guess what the command is for something by basing it off something they've already typed. So somebody who has already used docker hosts create will try to recall what the command is to make a new device, and they'll think docker devices create. Likewise, if they've used docker devices add, they'll type docker hosts add. They're such similar verbs that it'll be difficult to remember which applies to which command.

I wonder if we can choose a verb that is sufficiently different from "create" that you'll remember it is for the "devices" command and not mix it up with another command. docker devices connect, perhaps. "Connecting" a device is a real-world metaphor, so it'll be easy to remember.

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Nov 4, 2014

attach and detach

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Nov 4, 2014

Yes attach and detach came up in my mind as well, but was worried its functionality would "conflict" (in user expectation) with docker attach.

Other option; mount / u(n)mount, but I don't know if that makes sense for a device (and users might try to use it for volumes)

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Nov 7, 2014

Related; proposal for registry API V2. Might be interesting to check this when discussing consistency wrt the remote API; #9015

@imain

This comment has been minimized.

Copy link
Contributor

imain commented Nov 19, 2014

@cpuguy83, they are resources managed for a container that I'm adding a way to modify for an existing container.

mount/unmount won't work as that is already overloaded. We only make the device node, we don't mount the device.

Attach/detach works for me. What we are really doing is adding permissions to access the device and creating the device node in /dev. I'm not sure that is really attaching but it is similar.

Allow/disallow? :) heh.

You know, we already have links and volumes add, I don't see the issue with device add..

@jessfraz

This comment has been minimized.

Copy link
Contributor

jessfraz commented Feb 26, 2015

I'm trying to think of what an actionable item from this is, perhaps some sort of design doc

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Feb 26, 2015

@jfrazelle added; #8829 (comment). @bfirsh are you still "in charge" of UX-design? I can start a PR, but it could be difficult for me if design needs to be discussed further / decided on.

@jessfraz

This comment has been minimized.

Copy link
Contributor

jessfraz commented Feb 26, 2015

Actually sorry I was thinking about this last night, I think all the information here is super valuable so I was thinking we can just keep it w maybe label content and cli :)

@ewindisch

This comment has been minimized.

Copy link
Contributor

ewindisch commented May 27, 2015

Bump? I can't believe this is still open. @jfrazelle whatever the output is, it seems to be blocking for other issues, see: #8826. It's frustrating to have good PRs blocked on bikeshedding.

@duglin

This comment has been minimized.

Copy link
Contributor

duglin commented May 27, 2015

Trying to a little bit of it slowly, here: #13509

@dnephin

This comment has been minimized.

Copy link
Member

dnephin commented Jun 28, 2016

We can create aliases (or hidden aliases) for the old names and move the commands to new names. That is quite easy to do with cobra. I have a prototype of this on a branch:

Ex: docker ps, docker container ls (with alaises for docker container list and docker container ps) would all be the same commands.

@vdemeester

This comment has been minimized.

Copy link
Member

vdemeester commented Jun 28, 2016

@dnephin That would be really cool 👼 😻

@dnephin

This comment has been minimized.

Copy link
Member

dnephin commented Aug 25, 2016

PR up #26025

@SwitchedToGitlab

This comment has been minimized.

Copy link

SwitchedToGitlab commented Feb 5, 2019

Way to go guys, you took the one thing that Docker did well and absolutely ruined it.

the main reasoning of this is for consistency.

If you wanted to consistency, you should have focused on consistency for the thousands of professionals that use this daily instead of ripping the rug out from underneath them randomly. This entire issue thread just has me shaking my head at just how flippant and reckless Docker is with their user experience. This is sad, shameful and not the way a project should be managed.

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Feb 5, 2019

@SwitchedToGitlab To what are you referring? There was no change to existing commands in this change. Also note this was merged over 2 years ago.

@SwitchedToGitlab

This comment has been minimized.

Copy link

SwitchedToGitlab commented Feb 6, 2019

@SwitchedToGitlab

This comment has been minimized.

Copy link

SwitchedToGitlab commented Feb 6, 2019

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Feb 6, 2019

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Feb 6, 2019

Also interested in the issue that was caused; the change at the time;

  • kept all existing commands untouched (but added an environment variable to hide some of them if desired)
  • added new aliases for the existing commands (eg now docker container run as alias for docker run)
  • new commands going forward following the new pattern (docker <object> <verb>)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.