subcmds: force consistent help text format

We're inconsistent with help text as to whether it uses title case and
whether it ends in a period.  Add a test to enforce a standard, and use
the style that Python optparse & argparse use themselves (e.g. with the
--help option): always lowercase, and never trailing period.

Change-Id: Ic1defae23daeac0ac9116aaf487427f50b34050d
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/305144
Reviewed-by: Raman Tenneti <rtenneti@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
This commit is contained in:
Mike Frysinger 2021-05-04 08:06:36 -04:00
parent aedd1e5ef0
commit c177f944d9
12 changed files with 91 additions and 60 deletions

View File

@ -33,7 +33,7 @@ to the Unix 'patch' command.
def _Options(self, p): def _Options(self, p):
p.add_option('-u', '--absolute', p.add_option('-u', '--absolute',
dest='absolute', action='store_true', dest='absolute', action='store_true',
help='Paths are relative to the repository root') help='paths are relative to the repository root')
def _ExecuteOne(self, absolute, project): def _ExecuteOne(self, absolute, project):
"""Obtains the diff for a specific project. """Obtains the diff for a specific project.

View File

@ -68,10 +68,10 @@ synced and their revisions won't be found.
def _Options(self, p): def _Options(self, p):
p.add_option('--raw', p.add_option('--raw',
dest='raw', action='store_true', dest='raw', action='store_true',
help='Display raw diff.') help='display raw diff')
p.add_option('--no-color', p.add_option('--no-color',
dest='color', action='store_false', default=True, dest='color', action='store_false', default=True,
help='does not display the diff in color.') help='does not display the diff in color')
p.add_option('--pretty-format', p.add_option('--pretty-format',
dest='pretty_format', action='store', dest='pretty_format', action='store',
metavar='<FORMAT>', metavar='<FORMAT>',

View File

@ -131,30 +131,30 @@ without iterating through the remaining projects.
def _Options(self, p): def _Options(self, p):
p.add_option('-r', '--regex', p.add_option('-r', '--regex',
dest='regex', action='store_true', dest='regex', action='store_true',
help="Execute the command only on projects matching regex or wildcard expression") help='execute the command only on projects matching regex or wildcard expression')
p.add_option('-i', '--inverse-regex', p.add_option('-i', '--inverse-regex',
dest='inverse_regex', action='store_true', dest='inverse_regex', action='store_true',
help="Execute the command only on projects not matching regex or " help='execute the command only on projects not matching regex or '
"wildcard expression") 'wildcard expression')
p.add_option('-g', '--groups', p.add_option('-g', '--groups',
dest='groups', dest='groups',
help="Execute the command only on projects matching the specified groups") help='execute the command only on projects matching the specified groups')
p.add_option('-c', '--command', p.add_option('-c', '--command',
help='Command (and arguments) to execute', help='command (and arguments) to execute',
dest='command', dest='command',
action='callback', action='callback',
callback=self._cmd_option) callback=self._cmd_option)
p.add_option('-e', '--abort-on-errors', p.add_option('-e', '--abort-on-errors',
dest='abort_on_errors', action='store_true', dest='abort_on_errors', action='store_true',
help='Abort if a command exits unsuccessfully') help='abort if a command exits unsuccessfully')
p.add_option('--ignore-missing', action='store_true', p.add_option('--ignore-missing', action='store_true',
help='Silently skip & do not exit non-zero due missing ' help='silently skip & do not exit non-zero due missing '
'checkouts') 'checkouts')
g = p.get_option_group('--quiet') g = p.get_option_group('--quiet')
g.add_option('-p', g.add_option('-p',
dest='project_header', action='store_true', dest='project_header', action='store_true',
help='Show project headers before output') help='show project headers before output')
p.add_option('--interactive', p.add_option('--interactive',
action='store_true', action='store_true',
help='force interactive usage') help='force interactive usage')

View File

@ -33,7 +33,7 @@ and all locally downloaded sources.
def _Options(self, p): def _Options(self, p):
p.add_option('-f', '--force', p.add_option('-f', '--force',
dest='force', action='store_true', dest='force', action='store_true',
help='Force the deletion (no prompt).') help='force the deletion (no prompt)')
def Execute(self, opt, args): def Execute(self, opt, args):
if not opt.force: if not opt.force:

View File

@ -48,7 +48,7 @@ class Info(PagedCommand):
help=optparse.SUPPRESS_HELP) help=optparse.SUPPRESS_HELP)
p.add_option('-l', '--local-only', p.add_option('-l', '--local-only',
dest="local", action="store_true", dest="local", action="store_true",
help="Disable all remote operations") help="disable all remote operations")
def Execute(self, opt, args): def Execute(self, opt, args):
self.out = _Coloring(self.client.globalConfig) self.out = _Coloring(self.client.globalConfig)

View File

@ -36,22 +36,22 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
def _Options(self, p): def _Options(self, p):
p.add_option('-r', '--regex', p.add_option('-r', '--regex',
dest='regex', action='store_true', dest='regex', action='store_true',
help="Filter the project list based on regex or wildcard matching of strings") help='filter the project list based on regex or wildcard matching of strings')
p.add_option('-g', '--groups', p.add_option('-g', '--groups',
dest='groups', dest='groups',
help="Filter the project list based on the groups the project is in") help='filter the project list based on the groups the project is in')
p.add_option('-a', '--all', p.add_option('-a', '--all',
action='store_true', action='store_true',
help='Show projects regardless of checkout state') help='show projects regardless of checkout state')
p.add_option('-f', '--fullpath', p.add_option('-f', '--fullpath',
dest='fullpath', action='store_true', dest='fullpath', action='store_true',
help="Display the full work tree path instead of the relative path") help='display the full work tree path instead of the relative path')
p.add_option('-n', '--name-only', p.add_option('-n', '--name-only',
dest='name_only', action='store_true', dest='name_only', action='store_true',
help="Display only the name of the repository") help='display only the name of the repository')
p.add_option('-p', '--path-only', p.add_option('-p', '--path-only',
dest='path_only', action='store_true', dest='path_only', action='store_true',
help="Display only the path of the repository") help='display only the path of the repository')
def ValidateOptions(self, opt, args): def ValidateOptions(self, opt, args):
if opt.fullpath and opt.name_only: if opt.fullpath and opt.name_only:

View File

@ -53,27 +53,27 @@ to indicate the remote ref to push changes to via 'repo upload'.
def _Options(self, p): def _Options(self, p):
p.add_option('-r', '--revision-as-HEAD', p.add_option('-r', '--revision-as-HEAD',
dest='peg_rev', action='store_true', dest='peg_rev', action='store_true',
help='Save revisions as current HEAD') help='save revisions as current HEAD')
p.add_option('-m', '--manifest-name', p.add_option('-m', '--manifest-name',
help='temporary manifest to use for this sync', metavar='NAME.xml') help='temporary manifest to use for this sync', metavar='NAME.xml')
p.add_option('--suppress-upstream-revision', dest='peg_rev_upstream', p.add_option('--suppress-upstream-revision', dest='peg_rev_upstream',
default=True, action='store_false', default=True, action='store_false',
help='If in -r mode, do not write the upstream field. ' help='if in -r mode, do not write the upstream field '
'Only of use if the branch names for a sha1 manifest are ' '(only of use if the branch names for a sha1 manifest are '
'sensitive.') 'sensitive)')
p.add_option('--suppress-dest-branch', dest='peg_rev_dest_branch', p.add_option('--suppress-dest-branch', dest='peg_rev_dest_branch',
default=True, action='store_false', default=True, action='store_false',
help='If in -r mode, do not write the dest-branch field. ' help='if in -r mode, do not write the dest-branch field '
'Only of use if the branch names for a sha1 manifest are ' '(only of use if the branch names for a sha1 manifest are '
'sensitive.') 'sensitive)')
p.add_option('--json', default=False, action='store_true', p.add_option('--json', default=False, action='store_true',
help='Output manifest in JSON format (experimental).') help='output manifest in JSON format (experimental)')
p.add_option('--pretty', default=False, action='store_true', p.add_option('--pretty', default=False, action='store_true',
help='Format output for humans to read.') help='format output for humans to read')
p.add_option('-o', '--output-file', p.add_option('-o', '--output-file',
dest='output_file', dest='output_file',
default='-', default='-',
help='File to save the manifest to', help='file to save the manifest to',
metavar='-|NAME.xml') metavar='-|NAME.xml')
def _Output(self, opt): def _Output(self, opt):

View File

@ -36,7 +36,7 @@ are displayed.
def _Options(self, p): def _Options(self, p):
p.add_option('-c', '--current-branch', p.add_option('-c', '--current-branch',
dest="current_branch", action="store_true", dest="current_branch", action="store_true",
help="Consider only checked out branches") help="consider only checked out branches")
p.add_option('--no-current-branch', p.add_option('--no-current-branch',
dest='current_branch', action='store_false', dest='current_branch', action='store_false',
help='consider all local branches') help='consider all local branches')

View File

@ -46,27 +46,27 @@ branch but need to incorporate new upstream changes "underneath" them.
p.add_option('--fail-fast', p.add_option('--fail-fast',
dest='fail_fast', action='store_true', dest='fail_fast', action='store_true',
help='Stop rebasing after first error is hit') help='stop rebasing after first error is hit')
p.add_option('-f', '--force-rebase', p.add_option('-f', '--force-rebase',
dest='force_rebase', action='store_true', dest='force_rebase', action='store_true',
help='Pass --force-rebase to git rebase') help='pass --force-rebase to git rebase')
p.add_option('--no-ff', p.add_option('--no-ff',
dest='ff', default=True, action='store_false', dest='ff', default=True, action='store_false',
help='Pass --no-ff to git rebase') help='pass --no-ff to git rebase')
p.add_option('--autosquash', p.add_option('--autosquash',
dest='autosquash', action='store_true', dest='autosquash', action='store_true',
help='Pass --autosquash to git rebase') help='pass --autosquash to git rebase')
p.add_option('--whitespace', p.add_option('--whitespace',
dest='whitespace', action='store', metavar='WS', dest='whitespace', action='store', metavar='WS',
help='Pass --whitespace to git rebase') help='pass --whitespace to git rebase')
p.add_option('--auto-stash', p.add_option('--auto-stash',
dest='auto_stash', action='store_true', dest='auto_stash', action='store_true',
help='Stash local modifications before starting') help='stash local modifications before starting')
p.add_option('-m', '--onto-manifest', p.add_option('-m', '--onto-manifest',
dest='onto_manifest', action='store_true', dest='onto_manifest', action='store_true',
help='Rebase onto the manifest version instead of upstream ' help='rebase onto the manifest version instead of upstream '
'HEAD. This helps to make sure the local tree stays ' 'HEAD (this helps to make sure the local tree stays '
'consistent if you previously synced to a manifest.') 'consistent if you previously synced to a manifest)')
def Execute(self, opt, args): def Execute(self, opt, args):
all_projects = self.GetProjects(args) all_projects = self.GetProjects(args)

View File

@ -169,6 +169,7 @@ later is required to fix a server side protocol bug.
PARALLEL_JOBS = 1 PARALLEL_JOBS = 1
def _CommonOptions(self, p): def _CommonOptions(self, p):
if self.manifest:
try: try:
self.PARALLEL_JOBS = self.manifest.default.sync_j self.PARALLEL_JOBS = self.manifest.default.sync_j
except ManifestParseError: except ManifestParseError:

View File

@ -152,61 +152,61 @@ Gerrit Code Review: https://www.gerritcodereview.com/
def _Options(self, p): def _Options(self, p):
p.add_option('-t', p.add_option('-t',
dest='auto_topic', action='store_true', dest='auto_topic', action='store_true',
help='Send local branch name to Gerrit Code Review') help='send local branch name to Gerrit Code Review')
p.add_option('--hashtag', '--ht', p.add_option('--hashtag', '--ht',
dest='hashtags', action='append', default=[], dest='hashtags', action='append', default=[],
help='Add hashtags (comma delimited) to the review.') help='add hashtags (comma delimited) to the review')
p.add_option('--hashtag-branch', '--htb', p.add_option('--hashtag-branch', '--htb',
action='store_true', action='store_true',
help='Add local branch name as a hashtag.') help='add local branch name as a hashtag')
p.add_option('-l', '--label', p.add_option('-l', '--label',
dest='labels', action='append', default=[], dest='labels', action='append', default=[],
help='Add a label when uploading.') help='add a label when uploading')
p.add_option('--re', '--reviewers', p.add_option('--re', '--reviewers',
type='string', action='append', dest='reviewers', type='string', action='append', dest='reviewers',
help='Request reviews from these people.') help='request reviews from these people')
p.add_option('--cc', p.add_option('--cc',
type='string', action='append', dest='cc', type='string', action='append', dest='cc',
help='Also send email to these email addresses.') help='also send email to these email addresses')
p.add_option('--br', '--branch', p.add_option('--br', '--branch',
type='string', action='store', dest='branch', type='string', action='store', dest='branch',
help='(Local) branch to upload.') help='(local) branch to upload')
p.add_option('-c', '--current-branch', p.add_option('-c', '--current-branch',
dest='current_branch', action='store_true', dest='current_branch', action='store_true',
help='Upload current git branch.') help='upload current git branch')
p.add_option('--no-current-branch', p.add_option('--no-current-branch',
dest='current_branch', action='store_false', dest='current_branch', action='store_false',
help='Upload all git branches.') help='upload all git branches')
# Turn this into a warning & remove this someday. # Turn this into a warning & remove this someday.
p.add_option('--cbr', p.add_option('--cbr',
dest='current_branch', action='store_true', dest='current_branch', action='store_true',
help=optparse.SUPPRESS_HELP) help=optparse.SUPPRESS_HELP)
p.add_option('--ne', '--no-emails', p.add_option('--ne', '--no-emails',
action='store_false', dest='notify', default=True, action='store_false', dest='notify', default=True,
help='If specified, do not send emails on upload.') help='do not send e-mails on upload')
p.add_option('-p', '--private', p.add_option('-p', '--private',
action='store_true', dest='private', default=False, action='store_true', dest='private', default=False,
help='If specified, upload as a private change.') help='upload as a private change (deprecated; use --wip)')
p.add_option('-w', '--wip', p.add_option('-w', '--wip',
action='store_true', dest='wip', default=False, action='store_true', dest='wip', default=False,
help='If specified, upload as a work-in-progress change.') help='upload as a work-in-progress change')
p.add_option('-o', '--push-option', p.add_option('-o', '--push-option',
type='string', action='append', dest='push_options', type='string', action='append', dest='push_options',
default=[], default=[],
help='Additional push options to transmit') help='additional push options to transmit')
p.add_option('-D', '--destination', '--dest', p.add_option('-D', '--destination', '--dest',
type='string', action='store', dest='dest_branch', type='string', action='store', dest='dest_branch',
metavar='BRANCH', metavar='BRANCH',
help='Submit for review on this target branch.') help='submit for review on this target branch')
p.add_option('-n', '--dry-run', p.add_option('-n', '--dry-run',
dest='dryrun', default=False, action='store_true', dest='dryrun', default=False, action='store_true',
help='Do everything except actually upload the CL.') help='do everything except actually upload the CL')
p.add_option('-y', '--yes', p.add_option('-y', '--yes',
default=False, action='store_true', default=False, action='store_true',
help='Answer yes to all safe prompts.') help='answer yes to all safe prompts')
p.add_option('--no-cert-checks', p.add_option('--no-cert-checks',
dest='validate_certs', action='store_false', default=True, dest='validate_certs', action='store_false', default=True,
help='Disable verifying ssl certs (unsafe).') help='disable verifying ssl certs (unsafe)')
RepoHook.AddOptionGroup(p, 'pre-upload') RepoHook.AddOptionGroup(p, 'pre-upload')
def _SingleBranch(self, opt, branch, people): def _SingleBranch(self, opt, branch, people):

View File

@ -14,6 +14,7 @@
"""Unittests for the subcmds module (mostly __init__.py than subcommands).""" """Unittests for the subcmds module (mostly __init__.py than subcommands)."""
import optparse
import unittest import unittest
import subcmds import subcmds
@ -41,3 +42,32 @@ class AllCommands(unittest.TestCase):
# Reject internal python paths like "__init__". # Reject internal python paths like "__init__".
self.assertFalse(cmd.startswith('__')) self.assertFalse(cmd.startswith('__'))
def test_help_desc_style(self):
"""Force some consistency in option descriptions.
Python's optparse & argparse has a few default options like --help. Their
option description text uses lowercase sentence fragments, so enforce our
options follow the same style so UI is consistent.
We enforce:
* Text starts with lowercase.
* Text doesn't end with period.
"""
for name, cls in subcmds.all_commands.items():
cmd = cls()
parser = cmd.OptionParser
for option in parser.option_list:
if option.help == optparse.SUPPRESS_HELP:
continue
c = option.help[0]
self.assertEqual(
c.lower(), c,
msg=f'subcmds/{name}.py: {option.get_opt_string()}: help text '
f'should start with lowercase: "{option.help}"')
self.assertNotEqual(
option.help[-1], '.',
msg=f'subcmds/{name}.py: {option.get_opt_string()}: help text '
f'should not end in a period: "{option.help}"')