D6326: gendoc: group commands by category in man page and HTML help

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Wed May 1 14:51:13 EDT 2019


martinvonz requested changes to this revision.
martinvonz added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> gendoc.py:188
>  
> -    if True:
> -        for f in sorted(cmds):
> +    def helpcategory(cmd, h=h, cmdtable=cmdtable):
> +        """Given a canonical command name from `cmds` (above), retrieve its

Looks like just want `h` and `cmdtable` from the outer scope. They are available without this trick, so you can just delete the two arguments here.

> gendoc.py:195-197
> +        if helpcategory is None:
> +            return help.registrar.command.CATEGORY_NONE
> +        return helpcategory

Can be written `return helpcategory or help.registrar.command.CATEGORY_NONE`

> gendoc.py:203-209
> +        # Make a list of the commands in this category.
> +        # Rescanning the list of all commands for each category is quadratic;
> +        # this is justified because N is low, and repeating the lookup keeps
> +        # our data structure simpler.
> +        categorycmds = sorted([
> +            cmd for cmd in cmds
> +            if helpcategory(cmd) == category])

You could probably make it linear by creating a dict outside the loop like this:

  cmdsbycategory = {category: [] for category in help.CATEGORY_ORDER}
  for cmd in sorted(cmds):
      cmdsbycategory[helpcategory(cmd)].append(cmd)

That's barely longer, and it removes the need for the comment. I also think you can inline the `helpcategory()` function if you do that.

> gendoc.py:207-209
> +        categorycmds = sorted([
> +            cmd for cmd in cmds
> +            if helpcategory(cmd) == category])

Doesn't look like you need to sort here since it's going to be sorted on line 216 anyway.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6326

To: Sietse, #hg-reviewers, martinvonz
Cc: martinvonz, mercurial-devel


More information about the Mercurial-devel mailing list