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

[Sean Farley]

On Wed, Jul 10, 2013 at 1:53 PM, Matt Mackall <mpm at selenic.com> wrote:
> 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.

Ah, yes, I see now.

>> > 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

This was definitely in the category of
"didn't-think-through-other-combinations" on my part.

>> > 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.

Derp, parsing error on my part. And also not using the method correctly.

>> > 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.

As mentioned in IRC, my concern was forcing to load ipdb every time
dispatch.py is loaded. Kevin mentioned that we could perhaps move the
import inside the debug function?