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

Multiple CLI cleanups #419

Merged
merged 9 commits into from May 10, 2016
Merged

Multiple CLI cleanups #419

merged 9 commits into from May 10, 2016

Conversation

brson
Copy link
Contributor

@brson brson commented May 8, 2016

This adds help text for most subcommands and cleans them up in various ways.

The most important thing this does is try to resolve the bikesheddy problem of command synonyms that has been bugging me. That is, should the command be 'install' or 'update'? Should all install-like commands be the same verb or not?

From the commit, this is what I'm going for:

This reorganizes the verbs used in rustup commands. The concept here
is that every subcommand uses the most appropriate 'add/remove' verb
pair for itself, but implements hidden alternate commands using
the obvious other verbs.

So now the problematic verbs are:

  • toolchain install / toolchain uninstall
  • target add / target remove
  • override set / override unset

Additional (hidden) synonyms are:

  • toolchain update
  • toolchain add / toolchain remove
  • target add / target remove

This leaves the top level rustup update as the only non-hidden
'update' command, which has the unique focus of updating everything.

@aturon Importantly, this means that in the README I'm showing rustup toolchain install $toolchain for the installation command, and rustup update for doing updates. I like emphasizing the 'toolchain' in early examples of install command because it's an important concept. This is slightly different than the blog post you have in your hands where I'm demonstrating the (shorter but non-existant) rustup install command, which I have not implemented here.

@brson
Copy link
Contributor Author

brson commented May 8, 2016

I didn't make it clear but the hidden synonyms aren't just compromise, but some are needed because I changed some of the 'preferred' verbs.

@alexcrichton
Copy link
Member

I wonder if it'd be worth to also have a dedicated rustup install synonym for rustup toolchain install? That would perhaps provide a nice easy vector for newbies and would also align with other systems like nvm/rvm/etc?

@kbknapp
Copy link
Contributor

kbknapp commented May 8, 2016

clap-rs/clap#469 may be applicable too. It's something I hadn't prioritized, but would be a very quick addition.

@bors
Copy link
Contributor

bors commented May 9, 2016

☔ The latest upstream changes (presumably #425) made this pull request unmergeable. Please resolve the merge conflicts.

brson added 7 commits May 9, 2016 23:19
These are just the fully names of the `rustup install`
and `rustup update` commands.
This emphasizes `rustup toolchain install` for installing
toolchains, reinforcing the 'toolchain' nomenclature;
leaving `rustup update` for updating everything.
'show', 'update', 'default', 'toolchain', 'target', etc.
This reorganizes the verbs used in rustup commands. The concept here
is that every subcommand uses the most appropriate 'add/remove' verb
pair for itself, but implements hidden alternate commands using
the obvious other verbs.

So now the problematic verbs are:

* `toolchain install` / `toolchain uninstall`
* `target add` / `target remove`
* `override set` / `override unset`

Additional (hidden) synonyms are:

* `toolchain update`
* `toolchain add` / `toolchain remove`
* `target add` / `target remove`

This leaves the top level `rustup update` as the only non-hidden
'update' command, which has the unique focus of updating everything.
@brson
Copy link
Contributor Author

brson commented May 9, 2016

@alexcrichton I added rustup install <toolchain> as a hidden synonym.

@alexcrichton
Copy link
Member

Sounds good to me! Maybe some of the README could be updated as well? Other than r=me though, this all sounds good to me

@brson
Copy link
Contributor Author

brson commented May 9, 2016

Well I made rustup install a hidden synonym so it works, but not with the intent of promoting it in for use, though I do agree that rustup install nightly is nice and sweet, and familiar to rbenv users. Have to think about it a bit more.

@alexcrichton
Copy link
Member

Ah ok, either way sounds good to me

@brson
Copy link
Contributor Author

brson commented May 10, 2016

I went ahead and put the sweet commands in the README (but still not in --help).

@brson brson merged commit c8f6b09 into rust-lang:master May 10, 2016
@bors bors mentioned this pull request May 10, 2016
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.

None yet

4 participants