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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 31 commits into from
Mar 11, 2017
Merged

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

merged 31 commits into from
Mar 11, 2017

Conversation

kbknapp
Copy link
Member

@kbknapp kbknapp commented Mar 9, 2017

No description provided.

@kbknapp kbknapp mentioned this pull request Mar 9, 2017
Closed
@kbknapp kbknapp force-pushed the issues-863,perf-imp2,883,882,871 branch from b256576 to bade4c8 Compare March 9, 2017 22:53
@coveralls
Copy link

coveralls commented Mar 10, 2017 •
edited
Loading

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 •
edited
Loading

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
@kbknapp
perf: changes internal use of VecMap to Vec for matched values of Args
24e3839 
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
@kbknapp
chore: tags release beta
4328376
@andete @kbknapp
fix(help): don't show ARGS when there are only hidden positional args
8adf353 
Also no need to check for Hidden inside for that already is filtered
on !Hidden.

Closes #882
@kbknapp
setting(InferSubcommands): adds a setting to allow one to infer short…
150756b 
…ened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities)

Closes #863
@kbknapp
perf: doesn't run arg_post_processing on multiple values anymore
6f638a5
@kbknapp
tests: adds some minor benches for adding args by ref or moving
6b58efa
@kbknapp
chore: rustfmt run
91d8280
@kbknapp
fix: now correctly shows subcommand as required in the usage string w…
97e8db2 
…hen AppSettings::SubcommandRequiredElseHelp is used

Close #883
@kbknapp
fix: doesn't include the various [ARGS] [FLAGS] or [OPTIONS] if the o…
539ad60 
…nly ones available are hidden

Closes #882
@kbknapp
fix: doesn't print the argument sections in the help message if all a…
d63d404 
…rgs in that section are hidden
@kbknapp
imp: when AppSettings::SubcommandsNegateReqs and ArgsNegateSubcommand…
c8ab24b 
…s are used, a new more accurate double line usage string is shown

Closes #871
@kbknapp
tests: adds tests for new dual usage strings with certain subcommand …
642db9b 
…settings
@kbknapp
refactor: moves validation code into it's own module
44ed8b6
@kbknapp
refactor: moves usage string generation code into it's own module
622b609
@kbknapp
tests: fixes failing dual usage string help test
33eb5a6
@kbknapp
chore: increase version
a2e31b2
@kbknapp
docs(README.md): removes the old updates from previous versions as it…
06e07ad 
…'s getting very long and hard to read
@kbknapp
fix: fixes a misspelled import for Windows
d484da0
@kbknapp
tests: fixes failing hidden args test
5c76350
@kbknapp
fix: fixes a failing build with no-default-features
b841f37
@kbknapp
fix: fixes false positive clean parse when the suggestions feature is…
d2b4c2c 
… disabled and InferSubcommands is enabled
@kbknapp kbknapp force-pushed the issues-863,perf-imp2,883,882,871 branch from f77692a to 9d4535e Compare March 10, 2017 13:24
@kbknapp kbknapp mentioned this pull request Mar 10, 2017
Merged
@kbknapp
docs(CHANGELOG.md): adds the details about ability to change the help…
e802a47 
… 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**.

@kbknapp kbknapp mentioned this pull request Mar 10, 2017
Closed
@coveralls
Copy link

coveralls commented Mar 11, 2017 •
edited
Loading

Coverage Status

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

kbknapp added 5 commits March 11, 2017 12:14
@kbknapp
api(Arg::last): adds the ability to mark a positional argument as 'la…
f966829 
…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
tests: adds tests for .last(true) args
3a10353
@kbknapp
chore: ups the codegen units for test builds
10f1ebc
@kbknapp
chore: updates the changelog and readme with .last(true) details
c53e273
@kbknapp
refactor: implements PartialEq for some of the base structs
96884aa
@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 •
edited
Loading

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 •
edited
Loading

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
Auto merge of #893 - kbknapp:issues-863,perf-imp2,883,882,871, r=kbknapp
814b126 
Issues 863,perf imp2,883,882,871
@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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kbknapp @coveralls @homu @andete