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
200–209	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)
202	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
202	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
202	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 (19 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		Apr 26 2019, 11:30 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 14960

doc/gendoc.py

	"""			"""
	fullname = h[cmd]			fullname = h[cmd]
	details = cmdtable[fullname]			details = cmdtable[fullname]
	helpcategory = details[0].helpcategory			helpcategory = details[0].helpcategory
	if helpcategory is None:			if helpcategory is None:
	return help.registrar.command.CATEGORY_NONE			return help.registrar.command.CATEGORY_NONE
	return helpcategory			return helpcategory

				# Keep a log of processed commands. If a command category isn't registered
				# in help.CATEGORY_ORDER, the command doesn't get processed by
				# commandprinter. By comparing the list of processed commands to the
				# original list of commands, we can detect unregistered categories.
				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…
				processedcmds = []

	# Print the help for each command. We present the commands grouped by			# 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			# category, and we use help.CATEGORY_ORDER as a guide for a helpful order
	# in which to present the categories.			# in which to present the categories.
	for category in help.CATEGORY_ORDER:			for category in help.CATEGORY_ORDER:
	# Make a list of the commands in this category.			# Make a list of the commands in this category.
				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…
	# Rescanning the list of all commands for each category is quadratic;			# Rescanning the list of all commands for each category is quadratic;
	# this is justified because N is low, and repeating the lookup keeps			# this is justified because N is low, and repeating the lookup keeps
	# our data structure simpler.			# our data structure simpler.
	categorycmds = sorted([			categorycmds = sorted([
	cmd for cmd in cmds			cmd for cmd in cmds
	if helpcategory(cmd) == category])			if helpcategory(cmd) == category])
	# Print a section header for the category. Skip empty categories. For			# 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			# 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.			# commands in the category; this is fixed in the next commit.
	if categorycmds:			if categorycmds:
	ui.write(sectionfunc(help.CATEGORY_NAMES[category]))			ui.write(sectionfunc(help.CATEGORY_NAMES[category]))
	# Print each command in the category			# Print each command in the category
	for f in sorted(categorycmds):			for f in sorted(categorycmds):
	if f.startswith(b"debug"):			if f.startswith(b"debug"):
	continue			continue
				processedcmds.append(f) # log command
	d = get_cmd(h[f], cmdtable)			d = get_cmd(h[f], cmdtable)
	ui.write(sectionfunc(d[b'cmd']))			ui.write(sectionfunc(d[b'cmd']))
	# short description			# short description
	ui.write(d[b'desc'][0])			ui.write(d[b'desc'][0])
	# synopsis			# synopsis
	ui.write(b"::\n\n")			ui.write(b"::\n\n")
	synopsislines = d[b'synopsis'].splitlines()			synopsislines = d[b'synopsis'].splitlines()
	for line in synopsislines:			for line in synopsislines:
	multioccur = True			multioccur = True
	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']))
				# Make sure that we left no commands unprocessed.
				expected = sorted(filter(lambda c: not c.startswith('debug'), cmds))
				observed = sorted(processedcmds)
				if not observed == expected:
				unprocessed = sorted(set(expected) - set(observed))
				unprocessedinfo = ', '.join(
				"%s (%s)" % (cmd, helpcategory(cmd))
				for cmd in unprocessed
				)
				raise AssertionError("The following commands did not register their"
				" (category) in help.CATEGORY_ORDER: %s" % unprocessedinfo)

	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]))