pwclient handling of argparse options

[Brian Norris]

On Mon, Oct 19, 2015 at 01:56:35PM -0400, Mike Frysinger wrote:
> On 19 Oct 2015 09:57, Bernhard Reutner-Fischer wrote:
> > On October 17, 2015 4:25:54 AM GMT+02:00, Mike Frysinger wrote:
> > >On 16 Oct 2015 16:39, Brian Norris wrote:
> > >>          commit_str = args.get('c')
> > >
> > >if you feel like cleaning things up, i find the args behavior weird and
> > >non-standard.  it does:
> > >	args = action_parser.parse_args()
> > >	args = dict(vars(args))
> > >	action = args.get('subcmd')
> > >
> > >normally argparse code does:
> > >	args = action_parser.parse_args()
> > >	action = args.subcmd

I took a quick look at what it takes to switch. A few questions.

> > >i'm not sure why this dict/get style was picked.  maybe Bernhard can
> > >shed some light here.
> > 
> > At this point all we need is a dict and I found it easier and shorter to type.
> 
> i'm not sure how this is shorter:
> 	args.get('subcmd')
> than this:
> 	args.subcmd
> 
> it's also more dangerous: get will not throw an error if the key
> doesn't exist.  so if you did something like:
> 	args.get('submcd')
> you might not notice -- python would return None in the default case
> you didn't typo and everytime you typod.  but if you did:
> 	args.submcd
> then python would an exception as soon as that code is run.  since
> argparse has a default=None, args.subcmd would return None if the
> user hadn't specified anything.

Those benefits all seem nice to me.

> > I do remember to have gone through hoops to handle --help and --hash
> > at the same time AND allowing for recursive printing of all help (I
> > considered that a feature back then) but I do not remember if or why
> > my attempt to use a conflict_handler didn't work out. Could be there
> > was a bug in argparse back then or maybe I just goofed it.
> 
> Brian has posted some CLs to clean that up by using some more of the
> argparse infrastructure.

Right, so that part is fixed AFAICT. I wrote that stuff so long ago, I
don't actually remember the details about the conflict_handler now :)

Anyway, if we stop converting to a dict, then we have to be a little
more careful on how we treat various arguments when implementing common
code. e.g., we can't do this, since not all subcommands have the
"archived" flag:

	archived_str = args.a

	...

	if archived_str:
		# do something

Maybe I'm not being creative (or pythonic) enough to figure out what the
elegant solution would be that doesn't involve accidentally referencing
attributes that only exist on some subcommands. Do we just go with
hasattr()? Or getattr()? (But then we're still back to the same problem
of typos...)

Brian