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>
diff --git a/subcmds/diff.py b/subcmds/diff.py
index 4966bb1..b400ccf 100644
--- a/subcmds/diff.py
+++ b/subcmds/diff.py
@@ -33,7 +33,7 @@
def _Options(self, p):
p.add_option('-u', '--absolute',
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):
"""Obtains the diff for a specific project.
diff --git a/subcmds/diffmanifests.py b/subcmds/diffmanifests.py
index 392e597..6f23b34 100644
--- a/subcmds/diffmanifests.py
+++ b/subcmds/diffmanifests.py
@@ -68,10 +68,10 @@
def _Options(self, p):
p.add_option('--raw',
dest='raw', action='store_true',
- help='Display raw diff.')
+ help='display raw diff')
p.add_option('--no-color',
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',
dest='pretty_format', action='store',
metavar='<FORMAT>',
diff --git a/subcmds/forall.py b/subcmds/forall.py
index 4a631fb..0cf3b6a 100644
--- a/subcmds/forall.py
+++ b/subcmds/forall.py
@@ -131,30 +131,30 @@
def _Options(self, p):
p.add_option('-r', '--regex',
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',
dest='inverse_regex', action='store_true',
- help="Execute the command only on projects not matching regex or "
- "wildcard expression")
+ help='execute the command only on projects not matching regex or '
+ 'wildcard expression')
p.add_option('-g', '--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',
- help='Command (and arguments) to execute',
+ help='command (and arguments) to execute',
dest='command',
action='callback',
callback=self._cmd_option)
p.add_option('-e', '--abort-on-errors',
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',
- help='Silently skip & do not exit non-zero due missing '
+ help='silently skip & do not exit non-zero due missing '
'checkouts')
g = p.get_option_group('--quiet')
g.add_option('-p',
dest='project_header', action='store_true',
- help='Show project headers before output')
+ help='show project headers before output')
p.add_option('--interactive',
action='store_true',
help='force interactive usage')
diff --git a/subcmds/gitc_delete.py b/subcmds/gitc_delete.py
index 56e0eab..54b956f 100644
--- a/subcmds/gitc_delete.py
+++ b/subcmds/gitc_delete.py
@@ -33,7 +33,7 @@
def _Options(self, p):
p.add_option('-f', '--force',
dest='force', action='store_true',
- help='Force the deletion (no prompt).')
+ help='force the deletion (no prompt)')
def Execute(self, opt, args):
if not opt.force:
diff --git a/subcmds/info.py b/subcmds/info.py
index 2be5610..f7cf60f 100644
--- a/subcmds/info.py
+++ b/subcmds/info.py
@@ -48,7 +48,7 @@
help=optparse.SUPPRESS_HELP)
p.add_option('-l', '--local-only',
dest="local", action="store_true",
- help="Disable all remote operations")
+ help="disable all remote operations")
def Execute(self, opt, args):
self.out = _Coloring(self.client.globalConfig)
diff --git a/subcmds/list.py b/subcmds/list.py
index 5cbc0c2..68bcd5e 100644
--- a/subcmds/list.py
+++ b/subcmds/list.py
@@ -36,22 +36,22 @@
def _Options(self, p):
p.add_option('-r', '--regex',
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',
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',
action='store_true',
- help='Show projects regardless of checkout state')
+ help='show projects regardless of checkout state')
p.add_option('-f', '--fullpath',
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',
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',
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):
if opt.fullpath and opt.name_only:
diff --git a/subcmds/manifest.py b/subcmds/manifest.py
index e33e683..965c36e 100644
--- a/subcmds/manifest.py
+++ b/subcmds/manifest.py
@@ -53,27 +53,27 @@
def _Options(self, p):
p.add_option('-r', '--revision-as-HEAD',
dest='peg_rev', action='store_true',
- help='Save revisions as current HEAD')
+ help='save revisions as current HEAD')
p.add_option('-m', '--manifest-name',
help='temporary manifest to use for this sync', metavar='NAME.xml')
p.add_option('--suppress-upstream-revision', dest='peg_rev_upstream',
default=True, action='store_false',
- help='If in -r mode, do not write the upstream field. '
- 'Only of use if the branch names for a sha1 manifest are '
- 'sensitive.')
+ help='if in -r mode, do not write the upstream field '
+ '(only of use if the branch names for a sha1 manifest are '
+ 'sensitive)')
p.add_option('--suppress-dest-branch', dest='peg_rev_dest_branch',
default=True, action='store_false',
- help='If in -r mode, do not write the dest-branch field. '
- 'Only of use if the branch names for a sha1 manifest are '
- 'sensitive.')
+ help='if in -r mode, do not write the dest-branch field '
+ '(only of use if the branch names for a sha1 manifest are '
+ 'sensitive)')
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',
- help='Format output for humans to read.')
+ help='format output for humans to read')
p.add_option('-o', '--output-file',
dest='output_file',
default='-',
- help='File to save the manifest to',
+ help='file to save the manifest to',
metavar='-|NAME.xml')
def _Output(self, opt):
diff --git a/subcmds/overview.py b/subcmds/overview.py
index 202a5eb..b28367b 100644
--- a/subcmds/overview.py
+++ b/subcmds/overview.py
@@ -36,7 +36,7 @@
def _Options(self, p):
p.add_option('-c', '--current-branch',
dest="current_branch", action="store_true",
- help="Consider only checked out branches")
+ help="consider only checked out branches")
p.add_option('--no-current-branch',
dest='current_branch', action='store_false',
help='consider all local branches')
diff --git a/subcmds/rebase.py b/subcmds/rebase.py
index e0186d4..9ce4ecb 100644
--- a/subcmds/rebase.py
+++ b/subcmds/rebase.py
@@ -46,27 +46,27 @@
p.add_option('--fail-fast',
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',
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',
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',
dest='autosquash', action='store_true',
- help='Pass --autosquash to git rebase')
+ help='pass --autosquash to git rebase')
p.add_option('--whitespace',
dest='whitespace', action='store', metavar='WS',
- help='Pass --whitespace to git rebase')
+ help='pass --whitespace to git rebase')
p.add_option('--auto-stash',
dest='auto_stash', action='store_true',
- help='Stash local modifications before starting')
+ help='stash local modifications before starting')
p.add_option('-m', '--onto-manifest',
dest='onto_manifest', action='store_true',
- help='Rebase onto the manifest version instead of upstream '
- 'HEAD. This helps to make sure the local tree stays '
- 'consistent if you previously synced to a manifest.')
+ help='rebase onto the manifest version instead of upstream '
+ 'HEAD (this helps to make sure the local tree stays '
+ 'consistent if you previously synced to a manifest)')
def Execute(self, opt, args):
all_projects = self.GetProjects(args)
diff --git a/subcmds/sync.py b/subcmds/sync.py
index e528086..31760e5 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -169,10 +169,11 @@
PARALLEL_JOBS = 1
def _CommonOptions(self, p):
- try:
- self.PARALLEL_JOBS = self.manifest.default.sync_j
- except ManifestParseError:
- pass
+ if self.manifest:
+ try:
+ self.PARALLEL_JOBS = self.manifest.default.sync_j
+ except ManifestParseError:
+ pass
super()._CommonOptions(p)
def _Options(self, p, show_smart=True):
diff --git a/subcmds/upload.py b/subcmds/upload.py
index c497d87..fc389e4 100644
--- a/subcmds/upload.py
+++ b/subcmds/upload.py
@@ -152,61 +152,61 @@
def _Options(self, p):
p.add_option('-t',
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',
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',
action='store_true',
- help='Add local branch name as a hashtag.')
+ help='add local branch name as a hashtag')
p.add_option('-l', '--label',
dest='labels', action='append', default=[],
- help='Add a label when uploading.')
+ help='add a label when uploading')
p.add_option('--re', '--reviewers',
type='string', action='append', dest='reviewers',
- help='Request reviews from these people.')
+ help='request reviews from these people')
p.add_option('--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',
type='string', action='store', dest='branch',
- help='(Local) branch to upload.')
+ help='(local) branch to upload')
p.add_option('-c', '--current-branch',
dest='current_branch', action='store_true',
- help='Upload current git branch.')
+ help='upload current git branch')
p.add_option('--no-current-branch',
dest='current_branch', action='store_false',
- help='Upload all git branches.')
+ help='upload all git branches')
# Turn this into a warning & remove this someday.
p.add_option('--cbr',
dest='current_branch', action='store_true',
help=optparse.SUPPRESS_HELP)
p.add_option('--ne', '--no-emails',
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',
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',
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',
type='string', action='append', dest='push_options',
default=[],
- help='Additional push options to transmit')
+ help='additional push options to transmit')
p.add_option('-D', '--destination', '--dest',
type='string', action='store', dest='dest_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',
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',
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',
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')
def _SingleBranch(self, opt, branch, people):
diff --git a/tests/test_subcmds.py b/tests/test_subcmds.py
index 2234e64..bc53051 100644
--- a/tests/test_subcmds.py
+++ b/tests/test_subcmds.py
@@ -14,6 +14,7 @@
"""Unittests for the subcmds module (mostly __init__.py than subcommands)."""
+import optparse
import unittest
import subcmds
@@ -41,3 +42,32 @@
# Reject internal python paths like "__init__".
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}"')
