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

Clap #5152

Merged
merged 69 commits into from
Mar 13, 2018
Merged

Clap #5152

merged 69 commits into from
Mar 13, 2018

Conversation

matklad
Copy link
Member

@matklad matklad commented Mar 8, 2018

Reopening of #5129

So, looks like all tests are 🍏 on my machine!

I definitely want to refactor it some more, and also manually checked that we haven't regressed any help messages, but all the major parts are in place already.

pub mod update;
pub mod verify_project;
pub mod version;
pub mod yank;
Copy link
Member Author

@matklad matklad Mar 12, 2018

Choose a reason for hiding this comment

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

This file contains some copy-paste. We can combat it resurrecting original for_each_subcommand macro, though I prefer not to do this, because X Macros are hard to undestand to new contributors and to stupid IDEs :)

We could also coalesce builtin_exec and builtin into a single function probably, but I don't think it'll buy us much, and it was simpler to implement builtin_exec using multiple cursors :)

@matklad
Copy link
Member Author

matklad commented Mar 12, 2018

For style I personally still like the command-per-file rather than one-giant-file

Done! I think all the comments should be addressed now? Hope there aren't much quite bugs left :)

@alexcrichton
Copy link
Member

@bors: r+

🎊

@bors
Copy link
Contributor

bors commented Mar 13, 2018

📌 Commit 6b9c063 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 13, 2018

⌛ Testing commit 6b9c063 with merge 3cfb23b...

bors added a commit that referenced this pull request Mar 13, 2018
Clap

Reopening of #5129

So, looks like all tests are 🍏 on my machine!

I definitely want to refactor it some more, and also manually checked that we haven't regressed any help messages, but all the major parts are in place already.
@bors
Copy link
Contributor

bors commented Mar 13, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 3cfb23b to master...

@bors bors merged commit 6b9c063 into rust-lang:master Mar 13, 2018
infinity0 added a commit to infinity0/cargo that referenced this pull request Mar 14, 2018
infinity0 added a commit to infinity0/cargo that referenced this pull request Mar 15, 2018
@matklad matklad deleted the clap branch March 16, 2018 10:45
matklad added a commit to matklad/rust that referenced this pull request Mar 16, 2018
This notably includes

* rust-lang/cargo#5152
* rust-lang/cargo#5188

The first one switches cargo from docopt to clap (
we also update to the latest calp in this repository),
the second one should help us to unify feature flags
for Cargo itself and RLS, and build Cargo libray only
once.
dwijnand added a commit to dwijnand/cargo that referenced this pull request Jul 23, 2018
From reviewing the history this looks like a copy-paste error while
porting to clap (rust-lang#5152): this is
fetch's after_help info
bors added a commit that referenced this pull request Jul 23, 2018
Drop after_help in generate-lockfile

From reviewing the history this looks like a copy-paste error while
porting to clap (#5152): this is
fetch's after_help info

Fixes #5692
@ehuss ehuss added this to the 1.26.0 milestone Feb 6, 2022
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.

6 participants