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

2019-05-04 Thread Sietse (Sietse Brouwer)
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

2019-05-04 Thread martinvonz (Martin von Zweigbergk)
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

2019-05-04 Thread Sietse (Sietse Brouwer)
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

2019-05-04 Thread Sietse (Sietse Brouwer)
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

2019-05-03 Thread martinvonz (Martin von Zweigbergk)
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

2019-05-03 Thread Sietse (Sietse Brouwer)
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

2019-05-03 Thread Sietse (Sietse Brouwer)
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

2019-05-03 Thread Sietse (Sietse Brouwer)
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

2019-05-01 Thread martinvonz (Martin von Zweigbergk)
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

2019-04-28 Thread Sietse (Sietse Brouwer)
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