[PATCH 1 of 1 RFC] dispatch: run "hg --help", "hg --version", and "hg" through runcommand()

[Brodie Rao]

On Fri, May 18, 2012 at 2:26 PM, Matt Mackall <mpm at selenic.com> wrote:
> On Sun, 2012-05-13 at 22:08 +0200, Brodie Rao wrote:
>> # HG changeset patch
>> # User Brodie Rao <brodie at sf.io>
>> # Date 1336924194 -7200
>> # Node ID cdfd3ed7d957ff3b4aecf029313100b5a494e2cc
>> # Parent  d947e1da12597090d71c3d97ec224b7aa5513330
>> dispatch: run "hg --help", "hg --version", and "hg" through runcommand()
>>
>> This fires pre- and post- command hooks and allows pager to work.
>>
>> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
>> --- a/mercurial/dispatch.py
>> +++ b/mercurial/dispatch.py
>> @@ -645,15 +645,22 @@ def _dispatch(req):
>>          for ui_ in uis:
>>              ui_.setconfig('web', 'cacerts', '')
>>
>> -    if options['version']:
>> -        return commands.version_(ui)
>> -    if options['help']:
>> -        return commands.help_(ui, cmd)
>> -    elif not cmd:
>> -        return commands.help_(ui, 'shortlist')
>> -
>>      repo = None
>>      cmdpats = args[:]
>> +
>> +    if options['version']:
>> +        d = lambda: commands.version_(ui)
>> +        return runcommand(lui, repo, 'version', fullargs, ui, options, d,
>> +                          cmdpats, cmdoptions)
>> +    if options['help']:
>> +        d = lambda: commands.help_(ui, cmd, **cmdoptions)
>> +        return runcommand(lui, repo, 'help', fullargs, ui, options, d,
>> +                          cmdpats, cmdoptions)
>> +    elif not cmd:
>> +        d = lambda: commands.help_(ui, 'shortlist', **cmdoptions)
>> +        return runcommand(lui, repo, 'help', fullargs, ui, options, d,
>> +                          cmdpats, cmdoptions)
>
> This is a bit hairy. I went through a few iterations and came up with:
>
> diff -r 9acb5cd19162 -r 803431bdeaea mercurial/dispatch.py
> --- a/mercurial/dispatch.py     Thu May 17 15:52:14 2012 -0500
> +++ b/mercurial/dispatch.py     Fri May 18 15:56:59 2012 -0500
> @@ -684,11 +684,14 @@
>             ui_.setconfig('web', 'cacerts', '')
>
>     if options['version']:
> -        return commands.version_(ui)
> +        cmd, args, cmdoptions = 'version', [], {}
> +        func = cmdutil.findcmd(cmd, commands.table, True)[1][0]
>     if options['help']:
> -        return commands.help_(ui, cmd)
> +        cmd, args, cmdoptions = 'help', [cmd], {}
> +        func = cmdutil.findcmd(cmd, commands.table, True)[1][0]
>     elif not cmd:
> -        return commands.help_(ui, 'shortlist')
> +        cmd, args, cmdoptions = 'help', ['shortlist'], {}
> +        func = cmdutil.findcmd(cmd, commands.table, True)[1][0]
>
>     repo = None
>     cmdpats = args[:]
>
> But sadly, this only does half the job. We've got five other places in
> dispatch that are calling commands.help_ directly, and those are pretty
> nasty as they're deep in error-handling paths.
>
> So I'm beginning to think we should find a tidier way to handle this
> special case. Perhaps add a gethelp function that all of dispatch uses
> and pager can wrap. But then we have to deal with cases like:
>
>  * command was supposed to be paged, but had too few args, help not
> supposed to be paged
>  * command had --pager false, too few args, help invoked without access
> to global args
>
> Ugh!

I don't think we need to worry about the other callers. I think the
pager should get invoked when a) you specifically ask for help[1], and
b) when the help output is in its full/long form. I think it's fine
for dispatch to bypass its own machinery when presenting help in
exceptional cases.

[1]: Going by those rules, my patch shouldn't page "hg", but it does
at the moment.