[PATCH 01 of 14 RFC] dispatch: add ability to specify ipdb as a debugger

[Matt Mackall]

On Tue, 2013-07-09 at 17:33 -0500, Sean Farley wrote:
> mpm at selenic.com writes:
> 
> > On Tue, 2013-07-09 at 16:54 -0500, Sean Farley wrote:
> >> # HG changeset patch
> >> # User Sean Farley <sean.michael.farley at gmail.com>
> >> # Date 1372876982 18000
> >> #      Wed Jul 03 13:43:02 2013 -0500
> >> # Node ID 2bda9c94e65cff8c6968d82cfc07ec6049b194c0
> >> # Parent  648d1974b3f328947ee6cf2d00c66815a33cd208
> >> dispatch: add ability to specify ipdb as a debugger
> >> 
> >> Question: is there a better way to test a module's availability with
> >> demandimport?
> >> 
> >> diff --git a/mercurial/commands.py b/mercurial/commands.py
> >> --- a/mercurial/commands.py
> >> +++ b/mercurial/commands.py
> >> @@ -37,11 +37,11 @@
> >>      ('v', 'verbose', None, _('enable additional output')),
> >>      ('', 'config', [],
> >>       _('set/override config option (use \'section.name=value\')'),
> >>       _('CONFIG')),
> >>      ('', 'debug', None, _('enable debugging output')),
> >> -    ('', 'debugger', None, _('start debugger')),
> >> +    ('', 'debugger', 'pdb', _('start debugger')),
> >
> > Ok, so we're changing --debugger to accept an arg..
> >
> >> +    if '--debugger' in req.args:
> >> +        debugger = 'pdb'
> >> +    debugarg = _earlygetopt(['--debugger'], req.args)
> >
> > And then doing some special voodoo to allow it to not accept an arg?
> 
> Hmm, I thought that was how args were retrieved in dispatch.py? What is
> usable in dispatch.py at this point in the code for getting an optional
> argument?

It's voodoo because there's no support anywhere for optional args.

> > This is unfortunate. We've carefully avoided having optional args until
> > now because they're stupidly complicated. And we can't have a mandatory
> > arg to --debugger either.
> 
> Ah, I didn't realize there were no optional args.

Optional args make parsing ambiguous and require the parsers to have
intimate knowledge of what sorts of arguments are acceptable.

As a contrived example, imagine that someone had an ipdb alias. Now this
is ambiguous:

hg --debugger ipdb foo

> > Btw, I don't see you using debugarg[1], so you've almost certainly got a
> > bug here.
> 
> In the next line?
> 
> +    if debugarg:
> +        debugger = debugarg[0]
> 
> Or was the "[1]" supposed to refer to something I missed?

debugarg[1] returns the unused arguments.. so you can use them
elsewhere. If you're not using debugarg[1] somewhere, it probably means
you're using the _original_ argument list somewhere, still containing
pdb/ipdb. 

> > This might need to be an environment variable or (if possible) an hgrc
> > setting.
> 
> Aha, that might indeed be a better idea. The question still remains on
> how to check for an existence of a module with-in this demandimport
> funny business?

Reference anything contained in the module.

-- 
Mathematics is the supreme nostalgia of our time.