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

Switch from Docopt to Clap. #233

Merged
merged 1 commit into from
Nov 18, 2016
Merged

Switch from Docopt to Clap. #233

merged 1 commit into from
Nov 18, 2016

Conversation

BurntSushi
Copy link
Owner

There were two important reasons for the switch:

  1. Performance. Docopt does poorly when the argv becomes large, which is
    a reasonable common use case for search tools. (e.g., use with xargs)
  2. Better failure modes. Clap knows a lot more about how a particular
    argv might be invalid, and can therefore provide much clearer error
    messages.

While both were important, (1) made it urgent.

Note that since Clap requires at least Rust 1.11, this will in turn
increase the minimum Rust version supported by ripgrep from Rust 1.9 to
Rust 1.11. It is therefore a breaking change, so the soonest release of
ripgrep with Clap will have to be 0.3.

There is also at least one subtle breaking change in real usage.
Previous to this commit, this used to work:

rg -e -foo

Where this would cause ripgrep to search for the string -foo. Clap
currently has problems supporting this use case
(see: clap-rs/clap#742),
but it can be worked around by using this instead:

rg -e [-]foo

or even

rg [-]foo

and this still works:

rg -- -foo

This commit also adds Bash, Fish and PowerShell completion files to the
release, fixes a bug that prevented ripgrep from working on file
paths containing invalid UTF-8 and shows short descriptions in the
output of -h but longer descriptions in the output of --help.

Fixes #136, #189, #210, #230

@BurntSushi
Copy link
Owner Author

cc @kbknapp

@BurntSushi BurntSushi force-pushed the clap branch 3 times, most recently from 932e3a6 to d9bd654 Compare November 13, 2016 03:06
@kbknapp
Copy link
Contributor

kbknapp commented Nov 14, 2016

I'm on mobile right now so I still want to read through this in more detail but it looks good so far! And I really, really like some of the things you did that I haven't thought of doing!

I'll update here once clap-rs/clap#742 has been implemented.

@BurntSushi
Copy link
Owner Author

@kbknapp Thanks! I was hoping you'd take a peek. ^_^

There were two important reasons for the switch:

1. Performance. Docopt does poorly when the argv becomes large, which is
   a reasonable common use case for search tools. (e.g., use with xargs)
2. Better failure modes. Clap knows a lot more about how a particular
   argv might be invalid, and can therefore provide much clearer error
   messages.

While both were important, (1) made it urgent.

Note that since Clap requires at least Rust 1.11, this will in turn
increase the minimum Rust version supported by ripgrep from Rust 1.9 to
Rust 1.11. It is therefore a breaking change, so the soonest release of
ripgrep with Clap will have to be 0.3.

There is also at least one subtle breaking change in real usage.
Previous to this commit, this used to work:

    rg -e -foo

Where this would cause ripgrep to search for the string `-foo`. Clap
currently has problems supporting this use case
(see: clap-rs/clap#742),
but it can be worked around by using this instead:

    rg -e [-]foo

or even

    rg [-]foo

and this still works:

    rg -- -foo

This commit also adds Bash, Fish and PowerShell completion files to the
release, fixes a bug that prevented ripgrep from working on file
paths containing invalid UTF-8 and shows short descriptions in the
output of `-h` but longer descriptions in the output of `--help`.

Fixes #136, Fixes #189, Fixes #210, Fixes #230
@BurntSushi
Copy link
Owner Author

Woohoo! @kbknapp Thanks for clap! ^_^

@kbknapp
Copy link
Contributor

kbknapp commented Nov 21, 2016

@BurntSushi I saw you already noticed clap-rs/clap#755 but this should address a few of the items you mentioned. For the use of a hyphen in specific option and not command wide there are plenty of edge cases that don't have 100% solutions (i.e. many people may want many different things). So I've implemented what I think to be the 95% solution.

Namely, edge cases arise when one of two things happen,

  • The option in question accepts multiple values
  • The value the user wishes to supply partially or fully matches a valid flag

For the primary edge case example, assume -e PAT... where PAT can start with a hyphen. Also assume -v, -a, and -l are all valid short flags. Using -e valid_pattern -val can cause ambiguities. This can be solved by forcing -e to accept only value at a time Arg::number_of_values(1) and Arg::multiple(true). This allows -e PAT -e PAT but disallows -e PAT PAT.

So there are workarounds, I just wanted to ensure you're aware some testing may be required 😉

Also the ZSH completions should be fixed for you.

@BurntSushi
Copy link
Owner Author

@kbknapp Ah that sounds perfect! I think I've banned all uses of --flag val1 val2 anyway since it feels incredibly non-standard to me. :-) Thanks so much for the quick turnaround!

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.

slowdown with many files on the command line, compared to no arguments
2 participants