version.py: Use argparse instead of getopt.

version.py: Use argparse instead of getopt.

Rewrite the argument handling code using argparse. It allows us to get
rid of a lot of manual parsing as well as the whole help message.

In addition, it fixes a few other bugs present in the previous
implementation:

- Passing 3 or more arguments that are not values for parameters would
  cause an infinite loop.
- The "--template" option was not allowed (only its short form "-t"
  was).

R=mark@chromium.org, brettw@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277450

[Mark Mentovai, 2014-06-12 13:49:44]

https://codereview.chromium.org/336513002/diff/1/build/util/version.py
File build/util/version.py (right):

https://codereview.chromium.org/336513002/diff/1/build/util/version.py#newcode10
build/util/version.py:10: import argparse
The CL description still mentions optparse.

https://codereview.chromium.org/336513002/diff/1/build/util/version.py#newcod...
build/util/version.py:104: parser.add_argument('-f', '--file', action='append',
default=[],
[] is already the default when action is “append”, but you can leave this here
if you really want to be explicit about it.

https://codereview.chromium.org/336513002/diff/1/build/util/version.py#newcod...
build/util/version.py:112: parser.add_argument('-e', '--eval', default=None,
The script used to accept multiple -e arguments, but the action here is not
explicitly set and takes the default, “store”, and…

https://codereview.chromium.org/336513002/diff/1/build/util/version.py#newcod...
build/util/version.py:122: if options.eval is not None:
…this condition is no longer ever false. It should turn into a loop, anyway.

[Raphael Kubo da Costa (rakuco), 2014-06-16 13:21:16]

Sorry for the brain farts, I hadn't realized -e was accumulating before.

Patch v2 should address all points you've raised, with the exception of the
"default=[]" part -- it turns out if I leave it out the default for "append"
actions is also None.

[Mark Mentovai, 2014-06-16 13:28:24]

LGTM

https://codereview.chromium.org/336513002/diff/20001/build/util/version.py
File build/util/version.py (right):

https://codereview.chromium.org/336513002/diff/20001/build/util/version.py#ne...
build/util/version.py:104: parser.add_argument('-f', '--file', action='append',
default=[],
I don’t know what you saw with the “default” parameter here, but here’s what I
see:

mark@cougar bash$ python
Python 2.7.5 (default, Mar  9 2014, 22:15:05) 
[GCC 4.2.1 Compatible Apple LLVM 5.0 (clang-500.0.68)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import argparse
>>> p = argparse.ArgumentParser()
>>> p.add_argument('-a', action='append')
_AppendAction(option_strings=['-a'], dest='a', nargs=None, const=None,
default=None, type=None, choices=None, help=None, metavar=None)
>>> o = p.parse_args(['-a', 'one', '-a', 'two'])
>>> o.a
['one', 'two']

It doesn’t really matter, having the explicit default=[] is fine, but it’s
curious if the interface here changed.

[Raphael Kubo da Costa (rakuco), 2014-06-16 13:41:43]

https://codereview.chromium.org/336513002/diff/20001/build/util/version.py
File build/util/version.py (right):

https://codereview.chromium.org/336513002/diff/20001/build/util/version.py#ne...
build/util/version.py:104: parser.add_argument('-f', '--file', action='append',
default=[],
On 2014/06/16 13:28:23, Mark Mentovai wrote:
> I don’t know what you saw with the “default” parameter here, but here’s what I
> see:
> [...]

In this example the default value is not used at all. What I have in mind is

>>> import argparse
>>> p = argparse.ArgumentParser()
>>> p.add_argument('-a', action='append')
_AppendAction(option_strings=['-a'], dest='a', nargs=None, const=None,
default=None, type=None, choices=None, help=None, metavar=None)
>>> print p.parse_args([]).a
None

vs

>>> import argparse
>>> p = argparse.ArgumentParser()
>>> p.add_argument('-a', action='append', default=[])
_AppendAction(option_strings=['-a'], dest='a', nargs=None, const=None,
default=[], type=None, choices=None, help=None, metavar=None)
>>> print p.parse_args([]).a
[]

[Raphael Kubo da Costa (rakuco), 2014-06-16 13:42:12]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[commit-bot: I haz the power, 2014-06-16 13:43:25]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/raphael.kubo.da.costa@intel.com/336513...

[Mark Mentovai, 2014-06-16 14:12:44]

Aha, right. LGTM.

[commit-bot: I haz the power, 2014-06-16 15:41:44]

Change committed as 277450