Some small fixes to the new options implementation.
Review Request #1236 — Created Oct. 28, 2014 and submitted
|johanoskarsson, patricklaw, zundel|
These all relate to options whose action is 'append', and address rare corner cases.
Switch some invocations of copy() to deepcopy(), to ensure e.g., that
default values of list type are not shared between the
parser we use to generate help and the parser we use to actually
Distinguish between an empty list value in config, and no value at all.
This ensures that "no value" doesn't trump a hard-coded default because
we mistake it for an explicit config value.
Don't special-case appends to RankedValue. Instead make copy()
return the underlying value. argparse is the only thing that calls copy()
on a RankedValue, and this will ensure that argparse always works with
raw values, both in the list case and (as before) in the single value case.
Nits: Remove an unused import, break up an overlong line.
Various manual tests of corner cases that could trigger these issues. CI running.
Just to repeat in my own words: before, argparse made a copy of the RankedValue instance using the default implementation of copy, and then called 'append()' on the copied element which was a RankedValue. It isn't really a list, so we hacked in the append() method.
Now, when argparse calls copy() it gets a copy of the list back instead, so when it calls append() it is holding a native list, and using the append() from it.