D6326: gendoc: group commands by category in man page and HTML help
This revision was automatically updated to reflect the committed changes. Closed by commit rHG3816e361e3d8: gendoc: group commands by category in man page and HTML help (authored by Sietse, committed by ). CHANGED PRIOR TO COMMIT https://phab.mercurial-scm.org/D6326?vs=14998=15012#toc REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6326?vs=14998=15012 REVISION DETAIL https://phab.mercurial-scm.org/D6326 AFFECTED FILES doc/gendoc.py CHANGE DETAILS diff --git a/doc/gendoc.py b/doc/gendoc.py --- a/doc/gendoc.py +++ b/doc/gendoc.py @@ -185,8 +185,33 @@ h[f] = c cmds = h.keys() -if True: -for f in sorted(cmds): +def helpcategory(cmd): +"""Given a canonical command name from `cmds` (above), retrieve its +help category. If helpcategory is None, default to CATEGORY_NONE. +""" +fullname = h[cmd] +details = cmdtable[fullname] +helpcategory = details[0].helpcategory +return helpcategory or help.registrar.command.CATEGORY_NONE + +# Print the help for each command. We present the commands grouped by +# category, and we use help.CATEGORY_ORDER as a guide for a helpful order +# in which to present the categories. +cmdsbycategory = {category: [] for category in help.CATEGORY_ORDER} +for cmd in cmds: +cmdsbycategory[helpcategory(cmd)].append(cmd) + +for category in help.CATEGORY_ORDER: +categorycmds = cmdsbycategory[category] +if not categorycmds: +# Skip empty categories +continue +# Print a section header for the category. +# For now, the category header is at the same level as the headers for +# the commands in the category; this is fixed in the next commit. +ui.write(sectionfunc(help.CATEGORY_NAMES[category])) +# Print each command in the category +for f in sorted(categorycmds): if f.startswith(b"debug"): continue d = get_cmd(h[f], cmdtable) To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
martinvonz added inline comments. INLINE COMMENTS > gendoc.py:207 > +if not categorycmds: > +continue # Skip empty categories > +# Print a section header for the category. `test-check-code.t` didn't like this ("gratuitous whitespace after Python keyword"), so I moved the comment to separate line above. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6326 To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
Sietse marked 2 inline comments as done. Sietse added inline comments. INLINE COMMENTS > martinvonz wrote in gendoc.py:188 > Interesting examples. > > I found the `commithook()` quite confusing. I've sent > https://phab.mercurial-scm.org/D6336 to remove the arguments from it. > > As you said, the `def one`argument is overridden in one place, so I don't > think that's a good comparison. > > The `def doit` actually uses the `remote=remote` as a trick for creating the > variable in local scope. > > Even if there are a few more valid cases, the vast majority seems to be just > accessing the variables from the closure. Fair enough. I've removed the outer-scope variables from the function's signature, so now the function closes over them instead. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6326 To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
Sietse updated this revision to Diff 14998. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6326?vs=14989=14998 REVISION DETAIL https://phab.mercurial-scm.org/D6326 AFFECTED FILES doc/gendoc.py CHANGE DETAILS diff --git a/doc/gendoc.py b/doc/gendoc.py --- a/doc/gendoc.py +++ b/doc/gendoc.py @@ -185,8 +185,32 @@ h[f] = c cmds = h.keys() -if True: -for f in sorted(cmds): +def helpcategory(cmd): +"""Given a canonical command name from `cmds` (above), retrieve its +help category. If helpcategory is None, default to CATEGORY_NONE. +""" +fullname = h[cmd] +details = cmdtable[fullname] +helpcategory = details[0].helpcategory +return helpcategory or help.registrar.command.CATEGORY_NONE + +# Print the help for each command. We present the commands grouped by +# category, and we use help.CATEGORY_ORDER as a guide for a helpful order +# in which to present the categories. +cmdsbycategory = {category: [] for category in help.CATEGORY_ORDER} +for cmd in cmds: +cmdsbycategory[helpcategory(cmd)].append(cmd) + +for category in help.CATEGORY_ORDER: +categorycmds = cmdsbycategory[category] +if not categorycmds: +continue # Skip empty categories +# Print a section header for the category. +# For now, the category header is at the same level as the headers for +# the commands in the category; this is fixed in the next commit. +ui.write(sectionfunc(help.CATEGORY_NAMES[category])) +# Print each command in the category +for f in sorted(categorycmds): if f.startswith(b"debug"): continue d = get_cmd(h[f], cmdtable) To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
martinvonz added inline comments. INLINE COMMENTS > Sietse wrote in gendoc.py:188 > Passing `h=h, cmdtable=cmdtable` as default args is to make it obvious to the > reader which data the function depends on. I realize it can also silently > close over the variables in the outer scope, but I think this helps > legibility. > > Other places in the Mercurial codebase that define an inner function, and > make a captured value obvious by passing it as a default argument: > > - `def commithook` inside `def commit` in `localrepo.py` > - `def one` inside `def _showcompatlist` in `templateutil.py` (edge case, as > the default arg is actually overridden once) > - `def doit` inside `def debugdiscovery` in `debugcommands.py > > I'd say leave this as is, but I'll let you make the call and follow that. Interesting examples. I found the `commithook()` quite confusing. I've sent https://phab.mercurial-scm.org/D6336 to remove the arguments from it. As you said, the `def one`argument is overridden in one place, so I don't think that's a good comparison. The `def doit` actually uses the `remote=remote` as a trick for creating the variable in local scope. Even if there are a few more valid cases, the vast majority seems to be just accessing the variables from the closure. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6326 To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
Sietse marked 2 inline comments as done. Sietse added inline comments. INLINE COMMENTS > martinvonz wrote in gendoc.py:188 > 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. I've marked your comment about capturing/passing `h` and `cmdtable` "Done" in the sense of "replied to". I'm not sure how Phabricator's flow works -- "Done" seemed most likely to keep the flow going, but I do remain open to any reply by you (which I will then not bikeshed, because this is ultimately trivial :-P) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6326 To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
Sietse marked 3 inline comments as done. Sietse added inline comments. INLINE COMMENTS > martinvonz wrote in gendoc.py:188 > 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. Passing `h=h, cmdtable=cmdtable` as default args is to make it obvious to the reader which data the function depends on. I realize it can also silently close over the variables in the outer scope, but I think this helps legibility. Other places in the Mercurial codebase that define an inner function, and make a captured value obvious by passing it as a default argument: - `def commithook` inside `def commit` in `localrepo.py` - `def one` inside `def _showcompatlist` in `templateutil.py` (edge case, as the default arg is actually overridden once) - `def doit` inside `def debugdiscovery` in `debugcommands.py I'd say leave this as is, but I'll let you make the call and follow that. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6326 To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
Sietse updated this revision to Diff 14989. Sietse edited the summary of this revision. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6326?vs=14959=14989 REVISION DETAIL https://phab.mercurial-scm.org/D6326 AFFECTED FILES doc/gendoc.py CHANGE DETAILS diff --git a/doc/gendoc.py b/doc/gendoc.py --- a/doc/gendoc.py +++ b/doc/gendoc.py @@ -185,8 +185,32 @@ h[f] = c cmds = h.keys() -if True: -for f in sorted(cmds): +def helpcategory(cmd, h=h, cmdtable=cmdtable): +"""Given a canonical command name from `cmds` (above), retrieve its +help category. If helpcategory is None, default to CATEGORY_NONE +""" +fullname = h[cmd] +details = cmdtable[fullname] +helpcategory = details[0].helpcategory +return helpcategory or help.registrar.command.CATEGORY_NONE + +# Print the help for each command. We present the commands grouped by +# category, and we use help.CATEGORY_ORDER as a guide for a helpful order +# in which to present the categories. +cmdsbycategory = {category: [] for category in help.CATEGORY_ORDER} +for cmd in cmds: +cmdsbycategory[helpcategory(cmd)].append(cmd) + +for category in help.CATEGORY_ORDER: +categorycmds = cmdsbycategory[category] +if not categorycmds: +continue # Skip empty categories +# Print a section header for the category. +# For now, the category header is at the same level as the headers for +# the commands in the category; this is fixed in the next commit. +ui.write(sectionfunc(help.CATEGORY_NAMES[category])) +# Print each command in the category +for f in sorted(categorycmds): if f.startswith(b"debug"): continue d = get_cmd(h[f], cmdtable) To: Sietse, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6326: gendoc: group commands by category in man page and HTML help
Sietse created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY Make Mercurial's man page and HTML help group commands by category, and present the categories in a helpful order. `hg help` already does this; this patch uses the same metadata. This patch uses the same header level for command categories, and for commands. A subsequent patch will push the command headers down one level. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6326 AFFECTED FILES doc/gendoc.py CHANGE DETAILS diff --git a/doc/gendoc.py b/doc/gendoc.py --- a/doc/gendoc.py +++ b/doc/gendoc.py @@ -185,8 +185,35 @@ h[f] = c cmds = h.keys() -if True: -for f in sorted(cmds): +def helpcategory(cmd, h=h, cmdtable=cmdtable): +"""Given a canonical command name from `cmds` (above), retrieve its +help category. If helpcategory is None, default to CATEGORY_NONE +""" +fullname = h[cmd] +details = cmdtable[fullname] +helpcategory = details[0].helpcategory +if helpcategory is None: +return help.registrar.command.CATEGORY_NONE +return helpcategory + +# Print the help for each command. We present the commands grouped by +# category, and we use help.CATEGORY_ORDER as a guide for a helpful order +# in which to present the categories. +for category in help.CATEGORY_ORDER: +# 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]) +# Print a section header for the category. Skip empty categories. For +# now, the category header is at the same level as the headers for the +# commands in the category; this is fixed in the next commit. +if categorycmds: +ui.write(sectionfunc(help.CATEGORY_NAMES[category])) +# Print each command in the category +for f in sorted(categorycmds): if f.startswith(b"debug"): continue d = get_cmd(h[f], cmdtable) To: Sietse, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel