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

Issues 863,perf imp2,883,882,871 #893

Merged
merged 31 commits into from
Mar 11, 2017
Merged

Conversation

kbknapp
Copy link
Member

@kbknapp kbknapp commented Mar 9, 2017

No description provided.

@coveralls
Copy link

coveralls commented Mar 10, 2017

Coverage Status

Coverage increased (+0.2%) to 91.792% when pulling 55b1001 on issues-863,perf-imp2,883,882,871 into cb5c9f7 on master.

@coveralls
Copy link

coveralls commented Mar 10, 2017

Coverage Status

Coverage decreased (-0.02%) to 91.535% when pulling b77ee4d on issues-863,perf-imp2,883,882,871 into cb5c9f7 on master.

@kbknapp kbknapp force-pushed the issues-863,perf-imp2,883,882,871 branch from 6277e8e to ec9b694 Compare March 10, 2017 03:45
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 91.788% when pulling ec9b694 on issues-863,perf-imp2,883,882,871 into cb5c9f7 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 91.788% when pulling ec9b694 on issues-863,perf-imp2,883,882,871 into cb5c9f7 on master.

@kbknapp kbknapp force-pushed the issues-863,perf-imp2,883,882,871 branch from ec9b694 to f77692a Compare March 10, 2017 13:15
@homu
Copy link
Contributor

homu commented Mar 10, 2017

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

kbknapp and others added 21 commits March 10, 2017 08:22
This makes a very big difference for CLIs that parse a large number of
values (think ripgrep over a large directory).

This commit improved the ripgrep parsing of ~2,000 values, simulating
giving ripgrep a bunch of files. Parsing when from ~1,200,000 ns to
~400,000 ns! This was conducted a i7-5600U 2.6GHz
Also no need to check for Hidden inside for that already is filtered
on !Hidden.

Closes #882
…ened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities)

Closes #863
…hen AppSettings::SubcommandRequiredElseHelp is used

Close #883
…s are used, a new more accurate double line usage string is shown

Closes #871
… message for the auto-generated help/version
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e802a47 on issues-863,perf-imp2,883,882,871 into ** on master**.

2 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e802a47 on issues-863,perf-imp2,883,882,871 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e802a47 on issues-863,perf-imp2,883,882,871 into ** on master**.

@coveralls
Copy link

coveralls commented Mar 11, 2017

Coverage Status

Coverage decreased (-0.01%) to 91.791% when pulling 96aad7d on issues-863,perf-imp2,883,882,871 into 0e1e6dc on master.

…st' which means it should be used with `--` syntax and can be accessed early

Marking a positional argument `.last(true)` will allow accessing this argument earlier if the `--` syntax is used (i.e. skipping other positional args)
and also change the usage string to one of the following:

* `$ prog [arg1] [-- <last_arg>]`
* `$ prog [arg1] -- <last_arg>` (if the arg marked `.last(true)` is also marked `.required(true)`)

Closes #888
@kbknapp kbknapp force-pushed the issues-863,perf-imp2,883,882,871 branch from 96aad7d to 96884aa Compare March 11, 2017 17:38
@coveralls
Copy link

coveralls commented Mar 11, 2017

Coverage Status

Coverage decreased (-0.003%) to 91.8% when pulling 96884aa on issues-863,perf-imp2,883,882,871 into 0e1e6dc on master.

@kbknapp
Copy link
Member Author

kbknapp commented Mar 11, 2017

Cc @BurntSushi

This PR adds some substantial performance gains for parsing lots of args (I.e. from things like shell globs). Parsing ~2,000 args went from taking between 1-1.2M ns to ~300k ns on my slow laptop.

Cc @fnichol
This PR adds the ability to infer shortened subcommands automatically, which is something I saw habitat doing manually. I would say test it out before going full force with it since it's new 😉 It only works when there's no ambiguity but if there are valid values that could also be an unambiguous shortened subcommand I would lean to doing it manually in those rare circumstances.

@homu r+3

@homu
Copy link
Contributor

homu commented Mar 11, 2017

📌 Commit 96884aa has been approved by kbknapp

homu added a commit that referenced this pull request Mar 11, 2017
@homu
Copy link
Contributor

homu commented Mar 11, 2017

⚡ Test exempted - status

@homu homu merged commit 96884aa into master Mar 11, 2017
@kbknapp kbknapp deleted the issues-863,perf-imp2,883,882,871 branch March 12, 2017 15:45
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.

4 participants