This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6327

gendoc: guarantee that all commands were processed
ClosedPublic

Authored by Sietse on Apr 28 2019, 5:17 PM.

Download Raw Diff

Details

Reviewers

martinvonz

Group Reviewers

hg-reviewers

Commits

rHG037a97d62625: gendoc: guarantee that all commands were processed

Summary

The new logic renders the commands belonging to each category in turn.
Commands with an unregistered category are at risk of getting skipped
because their category is not in the list. By comparing the list of all
commands to a log of processed commands, we can detect commands with
unregistered categories and fail with an error message.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

Sietse created this revision.Apr 28 2019, 5:17 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptApr 28 2019, 5:17 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

Sietse added a child revision: D6328: minirst: support subsubsubsubsections (header level 5) with marker ''''.Apr 28 2019, 5:17 PM

Sietse retitled this revision from gendoc: guarantee that all commands were processed. to gendoc: guarantee that all commands were processed.Apr 29 2019, 7:08 AM

I think this patch will be simpler if you create the dict I suggested on D6326. You could then put the check for debug commands in that initial loop. You could also put the check just after that loop (any categories found that are not in help.CATEGORY_ORDER would be unregistered).

This revision now requires changes to proceed.May 1 2019, 3:01 PM

Sietse retitled this revision from gendoc: guarantee that all commands were processed to gendoc: guarantee that all commands were processed..May 3 2019, 9:36 AM

Sietse retitled this revision from gendoc: guarantee that all commands were processed. to gendoc: guarantee that all commands were processed.

Sietse updated this revision to Diff 14990.

martinvonz added inline comments.May 3 2019, 12:06 PM

doc/gendoc.py
198–207	No need to use exceptions here (`KeyError` could potentially be raised by something we didn't expect and then we'd misdiagnose it here). I think it's clearer like this: category = helpcategory(cmd) if category in cmdsbycategory: raise ... cmdsbycategory[category].append(cmd)
200	I think a previous version of this patch ignored uncategorized debug commands. Will this version error out?

Sietse updated this revision to Diff 14999.May 4 2019, 10:36 AM

Sietse marked 2 inline comments as done.May 4 2019, 12:02 PM

Sietse added inline comments.

doc/gendoc.py
200	It doesn't error out, I checked before uploading. Debug commands have CATEGORY_NONE; see also below. The purpose of this patch is to give confidence that no commands got 'lost' as we moved from 'process every command' (which guarantees every commands gets processed) to 'process every registered category' (where commands can get skipped if their category isn't registered). The previous version of this patch compared an expected list of commands to a log of processed commands. Because the main command-rendering loop skipped debug commands, I manually removed them from the expected list, too. That's why it ignored debug commands. The current version of this patch checks at command-sorting time that every command is part of a registered category. Commands with no category (such as debug commands) get sorted into `CATEGORY_NONE`, which is a known category displayed as "Uncategorized commands". We've guaranteed that the command-rendering loop will see every command, and what it skips or keeps is up to the rendering logic.

martinvonz added inline comments.May 4 2019, 12:31 PM

doc/gendoc.py
200	The previous version of this patch compared an expected list of commands to a log of processed commands. Because the main command-rendering loop skipped debug commands, I manually removed them from the expected list, too. That's why it ignored debug commands. Ah, that makes sense.

Sietse marked 3 inline comments as done.May 4 2019, 3:58 PM

Closed by commit rHG037a97d62625: gendoc: guarantee that all commands were processed (authored by Sietse). · Explain WhyMay 5 2019, 1:12 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			doc/gendoc.py (17 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	14960		Apr 28 2019, 5:17 PM	★	★
Diff 2	14990		May 3 2019, 9:36 AM	★	★
Diff 3	14999		May 4 2019, 10:36 AM	★	★
Diff 4	15013	rHG037a97d62625e4e81f7ba31dafaa12138b5e505e	May 3 2019, 9:37 AM	★	★

Commit	Parents	Author	Summary	Date
		Sietse Brouwer		May 3 2019, 9:37 AM

Status	Author	Revision
Closed	Sietse	D6329 gendoc: nest command headers under category headers
Closed	Sietse	D6328 minirst: support subsubsubsubsections (header level 5) with marker ''''
Closed	Sietse	D6327 gendoc: guarantee that all commands were processed
Closed	Sietse	D6326 gendoc: group commands by category in man page and HTML help
Closed	Sietse	D6325 gendoc: indent loop to make next patch more legible
Closed	Sietse	D6324 help: register the 'gpg' command category and give it a description

Diff 14990

doc/gendoc.py

	"""Given a canonical command name from `cmds` (above), retrieve its			"""Given a canonical command name from `cmds` (above), retrieve its
	help category. If helpcategory is None, default to CATEGORY_NONE			help category. If helpcategory is None, default to CATEGORY_NONE
	"""			"""
	fullname = h[cmd]			fullname = h[cmd]
	details = cmdtable[fullname]			details = cmdtable[fullname]
	helpcategory = details[0].helpcategory			helpcategory = details[0].helpcategory
	return helpcategory or help.registrar.command.CATEGORY_NONE			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}			cmdsbycategory = {category: [] for category in help.CATEGORY_ORDER}
	for cmd in cmds:			for cmd in cmds:
				try:
	cmdsbycategory[helpcategory(cmd)].append(cmd)			cmdsbycategory[helpcategory(cmd)].append(cmd)
				martinvonzUnsubmitted Done I think a previous version of this patch ignored uncategorized debug commands. Will this version error out? martinvonz: I think a previous version of this patch ignored uncategorized debug commands. Will this…
				SietseAuthorUnsubmitted Done It doesn't error out, I checked before uploading. Debug commands have CATEGORY_NONE; see also below. The purpose of this patch is to give confidence that no commands got 'lost' as we moved from 'process every command' (which guarantees every commands gets processed) to 'process every registered category' (where commands can get skipped if their category isn't registered). The previous version of this patch compared an expected list of commands to a log of processed commands. Because the main command-rendering loop skipped debug commands, I manually removed them from the expected list, too. That's why it ignored debug commands. The current version of this patch checks at command-sorting time that every command is part of a registered category. Commands with no category (such as debug commands) get sorted into `CATEGORY_NONE`, which is a known category displayed as "Uncategorized commands". We've guaranteed that the command-rendering loop will see every command, and what it skips or keeps is up to the rendering logic. Sietse: It doesn't error out, I checked before uploading. Debug commands have CATEGORY_NONE; see also…
				martinvonzUnsubmitted Done The previous version of this patch compared an expected list of commands to a log of processed commands. Because the main command-rendering loop skipped debug commands, I manually removed them from the expected list, too. That's why it ignored debug commands. Ah, that makes sense. martinvonz: > The previous version of this patch compared an expected list of commands to a log of…
				except KeyError:
				# If a command category wasn't registered, the command won't get
				# rendered below, so we raise an AssertionError.
				raise AssertionError(
				"The following command did not register its (category) in "
				"help.CATEGORY_ORDER: %s (%s)" % (cmd, helpcategory(cmd))
				)
				martinvonzUnsubmitted Done No need to use exceptions here (`KeyError` could potentially be raised by something we didn't expect and then we'd misdiagnose it here). I think it's clearer like this: category = helpcategory(cmd) if category in cmdsbycategory: raise ... cmdsbycategory[category].append(cmd) martinvonz: No need to use exceptions here (`KeyError` could potentially be raised by something we didn't…

				# 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:			for category in help.CATEGORY_ORDER:
	categorycmds = cmdsbycategory[category]			categorycmds = cmdsbycategory[category]
	if not categorycmds:			if not categorycmds:
	continue # Skip empty categories			continue # Skip empty categories
	# Print a section header for the category.			# Print a section header for the category.
	# For now, the category header is at the same level as the headers for			# 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.			# the commands in the category; this is fixed in the next commit.
	ui.write(sectionfunc(help.CATEGORY_NAMES[category]))			ui.write(sectionfunc(help.CATEGORY_NAMES[category]))
	if multioccur:			if multioccur:
	ui.write(_(b"\n[+] marked option can be specified"			ui.write(_(b"\n[+] marked option can be specified"
	b" multiple times\n"))			b" multiple times\n"))
	ui.write(b"\n")			ui.write(b"\n")
	# aliases			# aliases
	if d[b'aliases']:			if d[b'aliases']:
	ui.write(_(b" aliases: %s\n\n") % b" ".join(d[b'aliases']))			ui.write(_(b" aliases: %s\n\n") % b" ".join(d[b'aliases']))


	def allextensionnames():			def allextensionnames():
	return set(extensions.enabled().keys()) \| set(extensions.disabled().keys())			return set(extensions.enabled().keys()) \| set(extensions.disabled().keys())

	if __name__ == "__main__":			if __name__ == "__main__":
	doc = b'hg.1.gendoc'			doc = b'hg.1.gendoc'
	if len(sys.argv) > 1:			if len(sys.argv) > 1:
	doc = encoding.strtolocal(sys.argv[1])			doc = encoding.strtolocal(sys.argv[1])

	ui = uimod.ui.load()			ui = uimod.ui.load()
	if doc == b'hg.1.gendoc':			if doc == b'hg.1.gendoc':
	showdoc(ui)			showdoc(ui)
	else:			else:
	showtopic(ui, encoding.strtolocal(sys.argv[1]))			showtopic(ui, encoding.strtolocal(sys.argv[1]))