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

Parse options without names vector #81

Merged
merged 1 commit into from
May 17, 2019

Conversation

jridgewell
Copy link
Contributor

The only time the names vector has more than one Name in it is when parsing contiguous short options (-abc). We would continue putting Names into the vector until we hit a short option that takes an argument. At that point, we break, so we can process the argument.

So there's no point to reprocess the head of that vector, we already know they don't take arguments. Only the last one matters.

Copy link
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up @jridgewell

@KodrAus
Copy link
Contributor

KodrAus commented May 17, 2019

Looks like we've got some merge conflicts to sort out. Are you happy to take a look at these @jridgewell?

The only time the names vector has more than one `Name` in it is when parsing contiguous short options (`-abc`). We would continue putting `Name`s into the vector until we hit a short option that takes an argument. At that point, we break, so we can process the argument.

So there's no point to reprocess the head of that vector, we already know they don't take arguments. Only the last one matters.
@jridgewell
Copy link
Contributor Author

Rebased. Should be good to merge now.

@KodrAus KodrAus merged commit 4a7bb77 into rust-lang:master May 17, 2019
@jridgewell jridgewell deleted the parse_with_vec branch May 17, 2019 03:07
alexcrichton added a commit to alexcrichton/getopts that referenced this pull request Aug 19, 2019
This commit reverts three recent PRs to the `getopts` crate. One of the
main consumers of `getopts` is `rustc`, which isn't allowed to have
breaking changes. In rust-lang#82, however, a breaking change was landed. It
looks like rust-lang#83 builds on this change, and while rust-lang#81 seems unrelated the
diffs were somewhat tangled.
@alexcrichton alexcrichton mentioned this pull request Aug 19, 2019
alexcrichton added a commit that referenced this pull request Aug 19, 2019
alexcrichton added a commit that referenced this pull request Aug 19, 2019
Fixes a git mistake with #84
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