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

v3 Prep #1154

Merged
merged 15 commits into from
Jan 26, 2018
Merged

v3 Prep #1154

merged 15 commits into from
Jan 26, 2018

Conversation

kbknapp
Copy link
Member

@kbknapp kbknapp commented Jan 26, 2018

This PR sets the stage for v3.

The old v3-master became too far behind v2, and backporting all the bug fixes and features was a little too difficult with my limited time. So this is a fresh start. This time I'm going to feature freeze v2 until v3 is out.

It contains all the ground work to start implementing actual deprecations, breaking changes, and new features. See #1037 for details on whats left to do (...TONS right now).


This change is Reviewable

…p::write_help

Contains a *MINOR* breaking change. App::write_help now requires `&mut self` instead of `&self`.

This fixes a major bug where the help message is entirely incorrect.
More can be found at #808

I've decided to make this change because it was preventing further progress.

Anyone's code who breaks the fix is trivial:

```rust
// OLD BROKEN
let app = App::new("broken");
let mut out = io::stdout();
app.write_help(&mut out).expect("failed to write to stdout");

// NEW FIX
let mut app = App::new("broken");  // <-- let mut
let mut out = io::stdout();
app.write_help(&mut out).expect("failed to write to stdout");
```

Closes #808
This commit primarily changes to a lazy handling of POSIX overrides by
relying on github.com/bluss/ordermap instead of the old HashMap impl.
The ordermap allows us to keep track of which arguments arrived first,
and therefore determine which ones should be removed when an override
conflict is found.

This has the added benefit of we no longer have to do the bookkeeping to
keep track and override args as they come in, we can do it once at the
end.

Finally, ordermap allows fast Vec like iteration of the keys, which we
end up doing several times. Benching is still TBD once the v3 prep is
done, but this change should have a meaningful impact.
@mention-bot
Copy link

@kbknapp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kbknapp to be a potential reviewer.

@kbknapp kbknapp merged commit 7372359 into v3-master Jan 26, 2018
@kbknapp kbknapp deleted the v3-prep branch January 26, 2018 04:15
epage added a commit to epage/clap that referenced this pull request Dec 27, 2021
This allows two overriding args to interact with each other mid-line.
This was broken in 7673dfc / clap-rs#1154.  Before that, we duplicated the override
logic in severla places.

Now we plug into the start of a new arg which allows us to do this
incrementally without making the logic complex or inefficient, thanks to
prior refactors.

Fixes clap-rs#3217
epage added a commit to epage/clap that referenced this pull request Dec 27, 2021
This allows two overriding args to interact with each other mid-line.
This was broken in 7673dfc / clap-rs#1154.  Before that, we duplicated the override
logic in several places.

Now we plug into the start of a new arg which allows us to do this
incrementally without making the logic complex or inefficient, thanks to
prior refactors.

Fixes clap-rs#3217
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.

2 participants