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

fix(#1374): Remove all values from previous occurrences on self-override #1391

Closed
wants to merge 2 commits into from

Conversation

Elarnon
Copy link

@Elarnon Elarnon commented Dec 3, 2018

When an argument gets self-overriden, at most a single value gets kept
from the existing values list. This is incorrect when the argument has
multiple values per occurrence, and makes for "funny" bugs such as
--input a b --input c d being parsed as --input d.

This patch fixes the issue by keeping track of how many values were
already present when we started parsing each occurrence, and removing
all the values but the ones for the last occurrence when a self-override
occurs.

…f-override

When an argument gets self-overriden, at most a single value gets kept
from the existing values list.  This is incorrect when the argument has
multiple values per occurrence, and makes for "funny" bugs such as
`--input a b --input c d` being parsed as `--input d`.

This patch fixes the issue by keeping track of how many values were
already present when we started parsing each occurrence, and removing
all the values but the ones for the last occurrence when a self-override
occurs.
@kbknapp
Copy link
Member

kbknapp commented Oct 30, 2019

@Elarnon this was autoclosed when v3-master became master. If still interested please rebase onto master 😄

@Elarnon
Copy link
Author

Elarnon commented Oct 30, 2019

Given that in almost a year nobody looked at the patch or expressed that #1374 was also an issue for them, and that I am not using those features of clap currently, I will let this go. If someone else wants to rebase they are welcome to do so.

Thank you for checking in on this PR, and congrats on the progress on v3 -- it looks like the end of the road is near!

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