Remove all values from previous occurrences on self-override #1376
Conversation
…f-override When an argument gets self-overriden, at most a single value[1] gets removed from the existing values list. This is incorrect when the argument has multiple values per occurrence, and makes for "funny" bugs such as `--input --input a b` being parsed as `--input b` and `--input a b c --input d` being parsed as `--input c 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. [1]: Actually at most two values due to clap handling the overrides twice, which is a separate bug fixed in v3.
|
Looks like |
|
Looks like |
|
@kbknapp do you need anything else from me on this? |
|
Hi @kbknapp, friendly ping on this -- what can I do to help this PR go forward? Should I port it to the v3-master branch instead? |
|
Sorry, I've been out of it recently. I'll double check through this today after I get home from work and ping you. I've been doing all my work on v3-master recently and tried to essentially put blinders on myself with the 2.x branch so I can actually get v3 out :) If this PR doesn't require any additional work, and you'd like to also port it to the v3-master branch that would be appreciated. |
|
No worries! I think this is very confusing behavior so I'd really like it to be fixed for v3 actually. I initially intended to create a follow-up PR porting the fix to v3 but I'll make a backport PR to v2 instead. I'll port the PR to v3 tonight or tomorrow. |
|
Ported to v3 in #1391 |
When an argument gets self-overriden, at most a single value[1] gets removed from the existing values list. This is incorrect when the argument has multiple values per occurrence, and makes for "funny" bugs such as
--input --input a bbeing parsed as--input band--input a b c --input dbeing parsed as--input c 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.
Fixes #1374
[1]: Actually at most two values due to clap handling the overrides twice, which is a separate bug fixed in v3.