[PATCH REVIEW] help: Consistent use of minirst to format console and HTML help

[Matt Mackall]

On Sun, 2012-05-13 at 15:50 +0200, Olav Reinert wrote:
> # HG changeset patch
> # User Olav Reinert <seroton10 at gmail.com>
> # Date 1336916920 -7200
> # Node ID d1baea8535e408f5b7f0315273abe745bbc08257
> # Parent  76c744e0c5bbd0900beba49787b7e04ba862939b
> help: Consistent use of minirst to format console and HTML help
> 
> This patch changes the existing help system just enough to enable formatting of
> all help text to use minirst. It exposes several shortcomings of the reST
> parser and formatters which need to be fixed. Also, there are several
> opportunities for cleanup (such as moving many or all of the inner functions of
> helprst to help.py).
> 
> diff -r 76c744e0c5bb -r d1baea8535e4 mercurial/commands.py
> --- a/mercurial/commands.py	Sat May 12 13:20:26 2012 +0200
> +++ b/mercurial/commands.py	Sun May 13 15:48:40 2012 +0200
> @@ -3052,12 +3052,8 @@
>          displayer.show(ctx)
>      displayer.close()
>  
> - at command('help',
> -    [('e', 'extension', None, _('show only help for extensions')),
> -     ('c', 'command', None, _('show only help for commands'))],
> -    _('[-ec] [TOPIC]'))
> -def help_(ui, name=None, unknowncmd=False, full=True, **opts):
> -    """show help for a given topic or a help overview
> +def helprst(ui, name=None, unknowncmd=False, full=True, **opts):
> +    """generates RST-formatted help for a given topic or a help overview

Ok, patch one should be to move the RST generation to help.py with the
minimum number of changes. We don't keep utility functions in
commands.py as it's already pretty big.

>      With no arguments, print a list of commands with short help messages.
>  
> @@ -3105,19 +3101,15 @@
>          return rst
>  
>      # list all option lists
> -    def opttext(optlist, width):
> +    def opttext(optlist):
>          rst = ''
> -        if not optlist:
> -            return ''
> -
> -        for title, options in optlist:
> -            rst += '\n%s\n' % title
> -            if options:
> -                rst += "\n"
> -                rst += optrst(options)
> -                rst += '\n'
> -
> -        return '\n' + minirst.format(rst, width)
> +        if optlist:
> +            for title, options in optlist:
> +                rst += '\n%s\n' % title
> +                if options:
> +                    rst += '\n' + optrst(options) + '\n'
> +
> +        return rst

..or perhaps patch one is moving some of the RST formatting calls
around.

>      def addglobalopts(optlist, aliases):
>          if ui.quiet:
> @@ -3149,8 +3141,7 @@
>              # except block, nor can be used inside a lambda. python issue4617
>              prefix = inst.args[0]
>              select = lambda c: c.lstrip('^').startswith(prefix)
> -            helplist(select)
> -            return
> +            return helplist(select)
>  
>          # check if it's an invalid alias and display its error if it is
>          if getattr(entry[0], 'badalias', False):
> @@ -3158,20 +3149,21 @@
>                  entry[0](ui)
>              return
>  
> -        rst = ""
> +        rst = []

Moving rst from string append to list append is a third patch.

> +        #keep = ui.verbose and ['verbose'] or []
> +        #formatted, pruned = minirst.format(rst, textwidth, keep=keep)
> +        #ui.write(formatted)

We'll want a verbose arg to help.helprst().

> +def helphtml(ui, name):
> +    rst = helprst(ui, name)
> +    rst = '\n'.join(rst)
> +    ui.write(minirst.format(rst, style='html'))

This piece should live in hgweb. Patch four.

>  @command('identify|id',
> diff -r 76c744e0c5bb -r d1baea8535e4 mercurial/help.py
> --- a/mercurial/help.py	Sat May 12 13:20:26 2012 +0200
> +++ b/mercurial/help.py	Sun May 13 15:48:40 2012 +0200
> @@ -14,11 +14,9 @@
>      '''return a text listing of the given extensions'''
>      if not exts:
>          return ''
> -    maxlength = max(len(e) for e in exts)
> -    result = '\n%s\n\n' % header
> +    result = [ '\n%s\n' % header ]
>      for name, desc in sorted(exts.iteritems()):
> -        result += '%s%-*s %s\n' % (' ' * indent, maxlength + 2,
> -                                   ':%s:' % name, desc)
> +        result.append('%s:%s: %s' % (' ' * indent, name, desc))
>      return result
>  
>  def extshelp():
> diff -r 76c744e0c5bb -r d1baea8535e4 mercurial/hgweb/webcommands.py
> --- a/mercurial/hgweb/webcommands.py	Sat May 12 13:20:26 2012 +0200
> +++ b/mercurial/hgweb/webcommands.py	Sun May 13 15:48:40 2012 +0200
> @@ -850,7 +850,7 @@
>      u = webutil.wsgiui()
>      u.pushbuffer()
>      try:
> -        commands.help_(u, topicname)
> +        commands.helphtml(u, topicname)

Having hgweb import commands was never a good idea, we should try to fix
that here. All the hgweb bits can be patch five.

-- 
Mathematics is the supreme nostalgia of our time.